Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I think this pull request should fix #548 and #696. I have tested on macOS Catalina with Brave, Chrome, Firefox, Opera, Safari, and Edge and on iOS with Safari. I would appreciate tests on other devices. I will explain my observations on why these bugs occur and how my solution addresses them.
Problem
There are two separate bugs. One is that IME inputs are duplicated when when pressing Enter (to terminate IME input) and the other is that the IME's input gets modified when inputting on a blank line. The first bug occurs because the Mobiledoc Editor interprets an Enter when using an IME as the same as inputing Enter. As the IME input is buffered, the Mobiledoc Editor cursor is at the position where the IME input begins and when Enter is inputted handleNewline is called (which duplicates the text on the new line). The second bug occurs because of the absence of a markup node in the editor on a blank line. Because of the absence of a markup node and the IME buffering, this causes the Mutation handler to modify the editor DOM which in turn modifies the initial IME input.
Solution
The first bug can be fixed by ignoring control keydowns (Arrows, Enter, Delete, Tab) when an IME is being used.
The second bug can be fixed by getting rid of the offending mutations. One way to do this is to prepend a "dummy" character (specifically a space) to blank lines when composestart is fired and deleting this character when composeend is fired. This is treated as a the user typing an input (creating a markup node) which prevents the mutation. I initially thought using the null control character "\0" may be good in order to hide what is going on from the user, but this prepending of \0 gets recorded in mobiledoc editor's edit history. If the user then performs an undo, the null character will be in the text again and invisible to the user. I feel a space character is not too distracting and is noticeable in case the user does perform an undo.
The other solution for the second bug is to stop listening for mutations when composestart is fired and restart listening when composeend is fired. For most browsers, the first solution works and for Chrome-based browsers the second solutions works. This is mostly due to how each browsers modifies the DOM when receiving updates to the input of an IME. Chrome doesn't modify the DOM too much while other browsers add some extra nodes (see addendum). Thus the solution I came up with is to use both depending on the browser being used.
Asides
When inputting accented characters on Safari via the IME, the Enter keypress event is interpreted as a carriage return "\r" and so I have made it so these keypresses are handled as editor newlines instead.
Tests
I thought maybe I can add a test for the Mac IME for special characters (see #696) via event triggers but it didn't work; Triggering the Option-E keypress/keydown event doesn't do anything since the desired input is a OS-level keyboard command. Instead I added some tests that roughly mimic how the IME modifies the DOM.
Addenda (Solving the root problem)
Each browser modifies the DOM differently when receiving updates from an IME. I will document the browser behaviors that I have observed that may be worth noting in the event that the renderer gets reworked someday. I also give a possible solution on dealing with the behaviors so that the Chrome solution can be used.
Safari
IME use on Safari has a strange behavior in that it tries to copy the style of some wrapping element. If I try to input "か" (ka) in Japanese with the mutation observer disabled I get:
<p>[cursor]</p>
(Blank line)<p>k</p>
(Type "k")<p>か</p>
(Type "a")<br>
(Type "Enter", triggers input event "deleteCompositionText" for some reason)<font style="color:#fff"><span>か</span></font>
(Occurs immediately after 4. Note that<p>
is replaced by<font>
)#fff
is the font color of<p>
. If I remove this CSS style then at step 5 I just get か with no wrapping element. In either case, this messes up the renderer because of the mismatching section nodes. Safari does not follow this behavior if there is an initial character in<p>
.Firefox
The Chrome fix actually works in some sense for Firefox, but unfortunately it also has a strange behavior. When inputting "か" with the mutation observer disabled I get:
<p>[cursor]</p>
(Blank line)<p><br></p>
(Type "k")<p><br></p>
(Type "a")<p>か</p>
(Type "Enter")The problem with this is that you can't see what you're typing between IME initialization and termination. This behavior also ceases when there's an initial character in
<p>
.How this can be dealt with
I don't know why Firefox/Safari tries to modify the DOM in this way, but this behavior can be dealt with if the
<br>
element introduced bycursorElement
is removed. Applying the following changes gets rid of the IME bug:p
,ul
, etc.) becontenteditable
rather than giving the editor divcontenteditable
.cursorElement
.I'm hesitant about applying this solution because I'm not too familiar with the code base nor with the UX intentions. But I would be more than happy to give it a try if I can get some background on some of the code (mostly the purpose of
cursorElement
)