Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Japanese Hiragana Windows IME ignores first letter in editor #1453

Closed
saw opened this issue May 10, 2017 · 35 comments
Closed

Japanese Hiragana Windows IME ignores first letter in editor #1453

saw opened this issue May 10, 2017 · 35 comments

Comments

@saw
Copy link

saw commented May 10, 2017

Typing with the IME enabled in windows 8/IE 11 the very first character seems to be ignored by the IME

  1. Enable Japanese Hiragana IME
  2. Visit quilljs.com/playground
  3. type "tesuto" + enter

Expected behavior:
result: てすと
Actual behavior:
result: t えすと

screen shot 2017-05-09 at 10 08 10 pm

Everything is fine after that

Platforms:
Windows 8.1/ IE 11

Version:

1.2.4

@saw
Copy link
Author

saw commented May 10, 2017

Update: removing ql-blank from the editor fixes the problem, so it must be caused by the :before content

@azam
Copy link

azam commented May 10, 2017

Same as #1437

@azam
Copy link

azam commented May 11, 2017

Misread the actual behavior, while not exactly the same, using a Japanese IME on osx produces similar result.

Expected behavior:
result: てすと
Actual behavior:
result: tてすと

I suspect the text node value on the first keypress is added to the line and current index of the line is moved by 1, which is why on Japanese IME pressing te results in the first t is outputted, and when composition ends, the composed value is added next/right to t.

This happens on the dom mutation event callback observed by Parchment.Scroll (and extended by blots/scroll.js), where it creates a text blot & text node on the first keypress on a new line. Adding the subsequent keypresses properly appends, because probably Scroll/Container found a parent node to append to instead of creating a new text node.

But I still cannot find the exact code that consumes the first keypress and move current index forward 😿

@saw
Copy link
Author

saw commented May 11, 2017

@azam removing the :before rule absolutely fixes this issue, but I have no idea why. Trying to re-create this in a code pen without quill I couldn't create the same bug: http://codepen.io/saw-1473371988/pen/EmQMNq

@azam
Copy link

azam commented May 11, 2017

I tried deleting these lines, just to see if the ql-blank class change is making any difference, but it didn't, so I don't see how removing it will resolve the issue:

https://github.com/quilljs/quill/blob/master/core/quill.js#L73
https://github.com/quilljs/quill/blob/master/core/quill.js#L90
https://github.com/quilljs/quill/blob/master/assets/core.styl#L170-L175

I am testing it out using the local server (http://localhost:9000/standalone/snow/) on macOS Sierra, Chrome 58, Japanese IME (both default Apple & Google Japanese IME) if it helps.

Reason of me suspecting the mutation observer callback is because when I comment out the following line, the first key stopped finalizing as a text node, so I am guessing that the problem lies after this line, or any events emitted after this.

https://github.com/quilljs/parchment/blob/master/src/blot/scroll.ts#L134

Continuing to debug the issue ...

@azam
Copy link

azam commented May 11, 2017

(After some time trying to make the local dev server available to my IE VM) I tried dropping ql-blank, and as you said, it solves the problem, but only on IE. The problem persists on Chrome 58 even on Windows.

@saw
Copy link
Author

saw commented May 11, 2017

@azam ok I can repro on mac now, and it's intermittent. Tab A: no bug, Tab B (incognito): bug

@saw
Copy link
Author

saw commented May 11, 2017

I've tracked this down to insertBefore() in container.ts. I'm not entirely sure why it happens, but the first character entered in a descendant of container.ts results in that insertBefore method being called.

The problem is caused by inserting the textNode of the first text blot before the block:

parentBlot.domNode.insertBefore(this.domNode, (typeof refDomNode !== 'undefined') ? refDomNode : null);

@saw
Copy link
Author

saw commented May 11, 2017

How to fix it is a mystery at this point, especially considering it doesn't always happen.

@saw
Copy link
Author

saw commented May 11, 2017

I believe this may in fact be the same bug as reported in #1437 . That was closed when users disabled the evernote plugin, however discovered that the bug always happens for me in an incognito window in chrome. Also the bug happens if I disable 1password. Enable it: fixes the bug.

@saw
Copy link
Author

saw commented May 11, 2017

Update: 1password extension fixes the bug in safari & chrome. Nothing makes any sense anymore. Up is down.

@saw
Copy link
Author

saw commented May 12, 2017

I believe I have reproduced the root cause in this pen: https://codepen.io/saw-1473371988/pen/Wjzmay

Basically the IME seems to think the first character is after the insertion point, but the insertion point is corrected after the text node is appended to the block node (I think this happens in selection.js:setNativeRange()). So this is progress! Now to figure out how to fix this,

@saw
Copy link
Author

saw commented May 12, 2017

Here is what I think is happening:

When you type the first character in the editor quill creates a <p> to contain it then appends the text node you created to that node this puts the cursor before the character you just typed. Then quill uses the Selection and Range apis to move the cursor after the character. This creates a race condition because the js code is synchronous. The OS starts launching the IME thing (for Japanese typing) as soon as the first character is entered. If the cursor has been moved by the time the IME is getting characters you’re fine. If it hasn’t you aren’t, the IME thinks you are typing in front of where you think you're typing.

@azam
Copy link

azam commented May 13, 2017

Problem cannot be reproduced on develop. Suspecting 01e567e fixed the problem.

@saw
Copy link
Author

saw commented May 14, 2017

@azam I can reproduce in develop, try it in an incognito window.

@ykosaka
Copy link

ykosaka commented May 15, 2017

When I installed this add-on, IME works correctly.
After deleting the add-on, this problem is reproduced again.

https://chrome.google.com/webstore/detail/evernote-web-clipper/pioclpoplcdbaefihamjohnefbikjilc/related?hl=ja

environment:
macOS Sierra 10.12.3 Beta(16D25a)
Google Chrome 58.0.3029.110 (64-bit)
Google Japanese Input (2.20.2700.1)
quill version 1.2.4

@saw
Copy link
Author

saw commented May 15, 2017

I'm exploring solutions to this. The basic issue is we can't append the textNode to it's parent while the IME is open. I've "fixed" the problem by stopping update() in scroll.js while the IME is open and then applying them after, but I'm having issue right now with the model becoming out of sync with the contents of the editor when I do that. Another solution is to attach a text_node to each new block rather than a <br> but his is a huge architectural change

@saw
Copy link
Author

saw commented May 15, 2017

here is one possibly crazy option, change the default child of block from break to text, then modify text so it has a non-empty string (maybe zero-width space, U+200B) as it's default child, then in optimize remove the stray character in the beginning of the text blot. This does work, but it's a huge change because the empty state of the enter would no longer be <p><br></p>

@saw
Copy link
Author

saw commented May 15, 2017

The above change fixes the bug, but is a significant change and probable wrong. @jhchen do you have a suggestion for solving this that wouldn't be as sweeping and destructive?

@saw
Copy link
Author

saw commented May 15, 2017

Here is an override that fixes the issue completely, but you will need to scrub the '\u200B' from your output

@azam
Copy link

azam commented May 16, 2017

I changed ZWS to NUL on as shown on this branch: azam/quill@c7cc4ca199e6bdf9281c93a4907f581063576851 .

I agree that it is a quick workaround. If you want to go this way, there is a lot more to do. I think you might also want to override other inherited methods (length, value, etc) to absorb the change rather than let the user scrub the text themselves.

For example, one of the effects of not overriding these methods is, try writing something and then deleting all text, Editor#isBlank do not evaluate to true, so the place holder will not be shown afterwards.

Creating some sort of a new type of blank blot probably makes more sense.

@saw
Copy link
Author

saw commented May 16, 2017

For our internal use I think I have some overrides that work (I'll add you to the pull request @azam ) but I'm not sure the ideal solution for the library

@jhchen
Copy link
Member

jhchen commented May 18, 2017

I can confirm Windows + IE11/Edge is confirmed to not be working. Chrome/Safari should be working as of 0e7a10c

@410675629
Copy link

Do someone track this bug?@jhchen ,I have try your method on Chrome/Safari,but it doesn't work.

@jmzhang
Copy link

jmzhang commented Jun 23, 2017

Interesting find: change the ql-blank to use :after rather than :before, so toggling the class does not trigger the contents to reflow, thus making the IME happy.

But there's a catch, you need to manually position the :after pseudo element with top equal to the inner padding of the editor, about 12px for snow theme. I think this is acceptable 🤔

Update: The Chinese IME still doesn't work after this change, it seems the IMEs are extremely fragile and any updates to the containers can be devastating during compositions.

Update's Update: It turns out that the Chinese IME's bug is not relevant to this issue, the proposed fix should work.

@jhchen
Copy link
Member

jhchen commented Jul 17, 2017

The original steps for reproduction is now working on the latest version of Quill 1.2.6 on the latest IE 11.483.15063.0 and MS Edge 40.15063.0.0. @saw can you try with the latest and see if you are still having issues?

@saw
Copy link
Author

saw commented Jul 17, 2017 via email

@saw
Copy link
Author

saw commented Jul 27, 2017

@jhchen it does not seem to be happening anymore. Do you happen to know what changed to fix this? I'm really curious how you did it.

@jhchen
Copy link
Member

jhchen commented Aug 7, 2017

I can't recall exactly. Try git bisect?

@KaihatsuOnline
Copy link

worked before, but recently reappeared.

@fobazzz
Copy link

fobazzz commented Apr 6, 2018

I also again appeared a bug in window chrome

@smura
Copy link

smura commented Aug 9, 2018

I found a solution to this issue with IE11. The following CSS will fix this issue.

.ql-editor::before {
    visibility: hidden;
    color: rgba(0, 0, 0, .6);
    content: attr(data-placeholder);
    font-style: italic;
    left: 15px;
    pointer-events: none;
    position: absolute;
    right: 15px;
  }

  .ql-editor.ql-blank::before {
    visibility: visible;
  }

Try this with IE11 and IME : https://jsbin.com/kujavudoki/1/edit?html,css,js,output

I think this issue is caused by changing the DOM tree with psudo-element "::before". If psudo-element is always in the DOM tree, IME will be correct.

@SungwooJo
Copy link

It also reappeared in version 2.0.0-dev.2

smura pushed a commit to smura/quill that referenced this issue Aug 21, 2018
This problem occurs in Windows 7 and does not in Windows 10.
@kravda
Copy link

kravda commented Nov 24, 2020

Any news? problem is reproduced in 1.3.7
Typing 'sui' it should be すい
Firefox 83 - correct first and next attempts
изображение
IE11 - remain s and not correct first symbol, next attempts correct
изображение
Chrome 86 - remain s, next attempts correct
изображение

@quill-bot
Copy link

Quill 2.0 has been released (announcement post) with many changes and fixes. If this is still an issue please create a new issue after reviewing our updated Contributing guide 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests