Issue metadata
Sign in to add a comment
|
Rich text editors exhibit strange auto-correct insertion/correction behaviors without user input
Reported by
d...@basecamp.com,
Feb 15 2018
|
||||||||||||||||||||||
Issue descriptionDevice name: Nexus 5x From "Settings > About Chrome" Application version: Chrome Beta 65.0.3325.74 Operating system: Android 8.1.0; Nexus 5X Build/OPM3.171019.014 Keyboard: Gboard 6.9.8.183626756-release-arm64-v8a URLs: 1. https://trix-editor.org/ 2. https://draftjs.org/ === Case 1 - Steps to reproduce: (1) Open https://trix-editor.org/ (2) Tap the rich text edit field at the very first character position (the bolded "Trix" text) Expected result: Normal typing and text input is available Actual result: Autocorrect appears to automatically fire repeatedly without user input and enters incorrect text into the rich text field. === Case 2 - Steps to reproduce: (1) Open https://draftjs.org/ (2) Tap the rich text edit field (3) Type any word, then tap spacebar Expected result: Normal typing and text displayed on screen Actual result: Word types OK, but after hitting spacebar, the word disappears === Other relevant notes: - The behavior in either case does not happen with Chrome 64.0.3282.137 on the same device. - Two alternate keyboards, Swiftkey 6.7.7.27 and Flesky 9.3.0 do not appear to have the same issue on either Chrome 64 nor Chrome 65. - Videos of the behavior on both sites are attached. Thanks for your help!
,
Feb 16 2018
,
Feb 16 2018
Tested the issue in Android and able to reproduce the issue. Steps Followed: 1. Launched the Chrome 2. Navigated to https://trix-editor.org/ 3. Hovered on trix 4. Observed the weird behavior Chrome versions tested: 65.0.3325.74 OS: Android 8.1.0 Android Devices: Pixel XL 8.1.0 Using the per-revision bisect providing the bisect results, Good build: 65.0.3323.0 Bad build: 65.0.3324.0 You are looking for a change made after 529671(GOOD), but before 529672(BAD). CHANGELOG URL: The script might not always return single CL as suspect as some perf builds might get missing due to failure. https://chromium.googlesource.com/chromium/src/+/852ab8bfa27da53f0790a230bbabeac5f4175729 From the CL's above, assigning the issue to the owner concerned. @rlanday: Could you please look into the issue, and assign it to concerned owner if this is not related your change. Please navigate to below link for log's and video-- go/chrome-androidlogs/812674 Regarding case #2: Steps Followed (1) Open https://draftjs.org/ (2) Tap the rich text edit field (3) Type any word, then tap spacebar Observing the same behaviour since older builds (M60). Hence considering Non-Regression issue. Note: This issue is not observed in Desktop. Thanks!!
,
Feb 16 2018
dan@: It looks like this is the result of a change we made to fire additional compositionstart and compositionupdate events. Many Android keyboards, including Gboard, open an IME composition when you move the insertion point onto a new word (this is how they draw the black underline). We were previously not firing compositionstart and compositionupdate events for these IME commands. After the change we made, moving the insertion point onto a new word results in compositionstart being fired, and then a compositionupdate event that provides the text actually in the composition range. The compositionupdate event is necessary because, by our reading of the spec for compositionstart, we are not allowed to provide the composition text in the data field unless it's selected: https://www.w3.org/TR/uievents/#event-type-compositionstart > Some implementations MAY populate the data attribute of the compositionstart event with the text currently selected in the document (for editing and replacement). Otherwise, the value of the data attribute MUST be the empty string. We do not, however, fire an input event when this happens, since there's no actual DOM change. It seems like your code is thinking that the text is actually being changed when the first compositionupdate is received? I see elsewhere in the spec for compositionstart, it describes the data field as "the original string being edited, otherwise the empty string". changwan@ + yosin@: do you think should we make the data field for compositionstart contain the text the IME is setting the composition range on, even though there's no selection involved? Maybe I misinterpreted the spec.
,
Feb 16 2018
Hello, Dan's coworker here. I just shared some thoughts on the composition event change here: https://bugs.chromium.org/p/chromium/issues/detail?id=764439#c9
,
Feb 16 2018
> It seems like your code is thinking that the text is actually being changed when the first compositionupdate is received?
We apply changes on compositionend. And with this recent change, composition{start,update,end} are *all* fired just by moving the cursor.
,
Feb 16 2018
The compositionend event is actually not new. We were already firing it, without firing a compositionstart event first to open the selection. Doesn't that seem not quite right? Chrome 65 is currently out for beta and is targeted for stable release in March 6. Seems like there are a couple ways to proceed here: 1. We could leave the behavior as-is. I'm not convinced it's wrong or buggy. It seems like you should be able to use the beforeinput and/or input events to identify when the text is actually changing. 2. We could change the compositionstart event to include the initial composition string in the data field instead of an empty string. This way, if you're diffing between compositionstart and compositionupdate, your code won't get confused. However, the spec clearly says we're allowed to send an empty string, so you should probably try to handle this case. I don't think it's a good idea to make this change so soon before the M65 release. We would probably have to revert the behavior entirely and re-introduce it in a later milestone. 3. We could revert the CL. I changed the behavior because it seemed more correct to me. As far as I know, no one is actually depending on it yet. yosin@ + changwan@, what do you think?
,
Feb 16 2018
> It seems like you should be able to use the beforeinput and/or input events to identify when the text is actually changing. In our case, we need to support a whole range of browser and device versions. Many of which have inconsistent "input" event support and no "beforeinput" events. > We could change the compositionstart event to include the initial composition string in the data field instead of an empty string. This way, if you're diffing between compositionstart and compositionupdate, your code won't get confused. Is it necessary to fire compositionupdate and compositionend at all when there's no user input? Right now, moving your cursor through a word without making changes fires the entire suite of composition events. (see the screenshots in my other issue) --- We could easily adapt these changes, but doing so in a backwards compatible way with previous Chrome versions would be quite difficult.
,
Feb 16 2018
> Is it necessary to fire compositionupdate and compositionend at all when there's no user input? I think if we were to pass the composition text in the data field in the compositionstart event, we wouldn't need to send the compositionupdate events. The compositionupdate events are what's giving you trouble, right? The compositionend events are not new.
,
Feb 16 2018
> The compositionupdate events are what's giving you trouble, right? The compositionend events are not new. Here's our current list conditions for coping with Android's composition events so far: 1. Ignore compositionend when compositionstart data is not empty. Happens when pressing backspace at the end of word and updating it on older versions of System WebView. 2. Ignore compositionend when compositionupdate was received, but compositionstart was not. Can happen when backspacing through an entire word. 3. Ignore compositionend when no compositionstart or compositionupdate was received. Happens when moving the cursor in and out of a word. > I think if we were to pass the composition text in the data field in the compositionstart event, we wouldn't need to send the compositionupdate events So yes, sending non-empty data in compositionstart would happen to work because of #1.
,
Feb 16 2018
And I'd love to see those ^ bugs/quirks fixed! > I changed the behavior because it seemed more correct to me I understand your intention, but in practice it seems incorrect to me that cursor movement presents itself as a complete composition that's nearly indistinguishable from an actual composition of text.
,
Feb 20 2018
I discussed this change with two other engineers (changwan@ and yosin@) and we've decided not to revert it for the following reasons: - We believe the change brings us into better compliance with the spec. We were previously opening a composition range, which we would later send a compositionend event for, without ever sending a compositionstart event to open the range. This change fixes that behavior. - We don't think we're allowed to send the composition text in the data field of the compositionstart event (which would avoid having to send the compositionupdate events that don't actually correspond to an edit). The UI Events spec for compositionstart says: https://www.w3.org/TR/uievents/#event-type-compositionstart > Some implementations MAY populate the data attribute of the compositionstart event with the text currently selected in the document (for editing and replacement). Otherwise, the value of the data attribute MUST be the empty string. We believe that the "text currently selected in the document" is what you get when you call window.getSelection().toString(), which is distinct from the text in the composition range. Sending an empty string is clearly allowed for this event by the spec. - It seems your code is relying on buggy behavior in a way we don't want to continue supporting. It seems that the way you're currently detecting these compositionend events that don't actually correspond to an edit is to ignore them since we have a bug where we're not firing compositionstart. It therefore doesn't seem possible for us to fix this bug without breaking your code. We encourage you to use the beforeinput/input events to determine when DOM edits are actually occurring as a result of keyboard input. I will start a discussion on the W3C Editing Taskforce mailing list to see if it makes sense to change the spec to update/clarify what the behavior should be when an IME is setting composition on existing text. Maybe we can make these easier to distinguish somehow. Dan and Javan, I encourage you to provide your perspectives there.
,
Feb 20 2018
> We believe the change brings us into better compliance with the spec. We were previously opening a composition range, which we would later send a compositionend event for, without ever sending a compositionstart event to open the range. This change fixes that behavior. Another fix could have been removing that stray compositionend event and stop sending any composition events during cursor movement. That would be more consistent with all other browsers / platforms. > We believe that the "text currently selected in the document" is what you get when you call window.getSelection().toString(), which is distinct from the text in the composition range. Sending an empty string is clearly allowed for this event by the spec. Is there any way to determine the composition range then? Say I actually wanted to use these events and/or other DOM APIs to draw some kind of UI around the active word. > It seems your code is relying on buggy behavior in a way we don't want to continue supporting. Fortunately, we happened to test our editor on this beta Chrome release, and have time to update it before the next stable release. But it's not just *our code*. Many other editors are affected and will certainly be caught off guard. There's no mention of this change on https://www.chromestatus.com/features/schedule. Is it listed anywhere?
,
Feb 20 2018
Have you seen https://www.w3.org/TR/ime-api/#inputmethodcontext-interface ? The proposed candidatewindow* events and their composition*Offset properties encompass much of the behavior you're overloading composition events with.
,
Feb 20 2018
Here's the thread I started on the Editing Taskforce mailing list: https://lists.w3.org/Archives/Public/public-editing-tf/2018Feb/0000.html > Another fix could have been removing that stray compositionend event and stop sending any composition events during cursor movement. That would be more consistent with all other browsers / platforms. I think you're right that we could in principle pretend the composition doesn't exist until the IME actually changes the text. That might be better for your use case. However, I think cases might come up where developers actually do want to know about these compositions, especially if we add better APIs around IME. So I'm not convinced this makes sense. This also doesn't really seem like a general solution; it seems like either we're trying to paper over some other problem with the API, or you're trying to take a shortcut that might not work in all cases. > Is there any way to determine the composition range then? Say I actually wanted to use these events and/or other DOM APIs to draw some kind of UI around the active word. I don't think there is. This is something that has been identified as an area for future work, but unfortunately probably isn't going to be built any time soon: https://docs.google.com/document/d/10qltJUVg1-Rlnbjc6RH8WnngpJptMEj-tyrvIZBPSfY/ (see section "G. IME dropdown menu positioning") > Fortunately, we happened to test our editor on this beta Chrome release, and have time to update it before the next stable release. But it's not just *our code*. Many other editors are affected and will certainly be caught off guard. There's no mention of this change on https://www.chromestatus.com/features/schedule. Is it listed anywhere? We have not listed it as a new feature anywhere, since from our perspective, we were just fixing a bug with an API. We can't always foresee what sort of behavioral quirks people it's going to turn out that people are depending on. Testing on pre-release builds is generally going to be the most robust way to catch changes like this. I will follow up and see if it makes sense to get this listed. > Have you seen https://www.w3.org/TR/ime-api/#inputmethodcontext-interface ? I think I've seen this before, but unfortunately, I don't think anyone is currently working on implementing this in Chrome.
,
Feb 20 2018
> Here's the thread I started
Thanks for kicking that off!
> from our perspective, we were just fixing a bug with an API
From our perspective (and many others, from what I can gather), the bug was compositionend firing at all. Not that composition{start,update} *weren't* fired. This perspective is based on how/when all other platforms dispatch composition events.
So from the perspective of everyone using composition events in their application, this change is a new feature. One that only exists on Android.
,
Feb 21 2018
> That might be better for your use case. However, I think cases might come up where developers actually do want to know about these compositions Here's a recap of this change from my perspective: * It has a theoretical use case that isn't even possible to implement because the composition range can't be determined. * It concretely breaks several rich text editors in their current form. * It fixes a bug with lone compositionend events that applications have been ignoring all along by introducing even more events that can't be used in a meaningful way. * It's unclear if it conforms to the spec and you're asking the Editing Taskforce if it's proper. * It's going to be quietly introduced in the next Chrome release with no pressing need to do so.
,
Feb 21 2018
IMEs actually exist on every Chrome platform that I'm aware of (Mac, Windows, Linux, Chrome OS, Android, and iOS...note that iOS uses Apple's WebKit instead of Blink, but it still supports IMEs). What makes Android special is that most platforms only use IMEs for text input for certain languages, like Chinese or Japanese, or for certain input types, like handwriting recognition, but on Android, nearly all Android keyboards input text through the IME APIs. I think the idea was originally that it would encourage English-speaking developers to build apps that properly supported IMEs (since once you get your app working with English, Chinese and Japanese input uses the same APIs, so it automatically works with those).
,
Feb 21 2018
> IMEs actually exist on every Chrome platform that I’m aware of I never said IMEs only exist on Android. Android is the only platform that emits composition events when moving the cursor. - - - I’m happy that you started a thread to discuss this with the Editing Taskforce. The initial response (https://lists.w3.org/Archives/Public/public-editing-tf/2018Feb/0001.html) seems to share at least some of my feelings/concerns with this change: > Ok, but the composition is not actually changing? To me it sounds like this will just be adding to the confusion. > As an editor developer, I would initially say that if a composition opens and closes but nothing changes and there is nothing I really can do about the composition event that changed nothing — then why notify me about it all? > What seems like a bigger distraction is having an update event for the composition when really nothing has changed. With that in mind, why not hold off on this change for a release or two? Doesn’t it make more sense to have a discussion and then ship it? It’s only going to make matters worse for developers if you end up changing the behavior again.
,
Feb 22 2018
> It seems your code is relying on buggy behavior in a way we don’t want to continue supporting FYI, this change may impact Angular (*your* JavaScript framework) because it too has a “special case” for “this junk”: https://twitter.com/robwormald/status/966489183765151744
,
Feb 22 2018
I think the part of this bug report about Draft.js is invalid. I can reproduce the buggy behavior in Chrome 64.0.3282.137. As one of their developers mentioned on GitHub, they don't actually support Chrome for Android: https://github.com/facebook/draft-js/issues/1657#issuecomment-366999480
,
Feb 22 2018
For the record, we have no affiliation with Draft.js. And for posterity, here is the reason they don’t fully support Android: “we have been 100% blocked from supporting Android web on Draft.js due to the lack of consistency with other browsers and with the spec in how events are handled” The bug I noticed on Draft.js occurs when moving the cursor through existing text, and I’m unable to reproduce it in Chrome 64. See attached video. I would be happy to provide links to other editors if you’re interested in comparing.
,
Feb 23 2018
I’m aware that many web developers don’t like how the Android keyboard APIs work. This has more to do with how Android was designed than with anything we can do in Chrome. Have you compared Chrome’s behavior on Android with Firefox or Dolphin (a Gecko-based browser)? Firefox’s implementation (as of 58.0.2) seems particularly interesting, in that if I’m typing a word from start to end with Gboard, it fires composition events, but not keydown/keyup (Chrome fires “fake” keydown/keyup events with a 229 key code, but it does at least fire them), but if I type in the middle of a word, it does seem to fire keydown, keypress, and keyup events with the key code for the pressed key, but it doesn’t fire any composition-related events.
,
Feb 23 2018
I haven't tried those browsers on Android, but their different key event sequences highlight why your change to composition events is so problematic.
If it’s no longer safe to assume that compositions mean user input, then we need to determine that on our own by listening for some combination of key and input events.
Let’s look at the events dispatched while typing “fox” on Android:
keydown
compositionstart ""
beforeinput
compositionupdate "f"
input
keydown
beforeinput
compositionupdate "fo"
input
keydown
beforeinput
compositionupdate "fox"
input
keydown
beforeinput
compositionupdate "fox"
input
compositionend "fox"
Like you mentioned, “keydown” may not be fired in some browsers. “beforeinput” isn’t widely supported so we can’t rely on that. “input” is pretty reliable, although it isn’t fired after “compositionend” for some reason.
Basically, we have to track a bunch of state, and then make an educated guess when compositions end. Guessing wrong means we either fail to record input, or we duplicate existing input during cursor movement.
Prior to this change, a full sequence of composition{start,update,end} events always meant the user was typing. And it meant that in all browser / OS / keyboard combinations. We never had to guess. Now we do.
,
Feb 23 2018
I actually don’t think that was ever a safe assumption to make. Let’s say we hypothetically were to make a change to Chrome to not fire compositionstart or compositionupdate events for moving the cursor between words if we don’t actually receive an edit. Does that actually solve your problem? There’s another case where you’re backspacing through a word. E.g. you put your cursor on “fox” and hit backspace. Chrome 64 emits these composition events: compositionupdate: "fo" compositionend Chrome 65 emits these events: compositionstart compositionupdate: "fox" compositionupdate: "fo" compositionend You seem to have some code that assumes that whatever is sent in “compositionupdate” immediately before “compositionend” corresponds to newly-inserted text, except you’re cheating for this case and detecting it by taking advantage of the bug causing us to not emit compositionstart. The reason input is fired before compositionend and not after, by the way, is because updating the composition is actually what updates the DOM (and the spec says compositionupdate has to be fired before the DOM update, which triggers the input event). Ending the composition doesn’t change the text again.
,
Feb 23 2018
> I actually don’t think that was ever a safe assumption to make. It was. Well, maybe I should have said it meant the user was making an edit, not that they were typing. > Let’s say we hypothetically were to make a change to Chrome to not fire compositionstart or compositionupdate events for moving the cursor between words if we don’t actually receive an edit. Does that actually solve your problem? Yes, 100% solves it. > except you’re cheating for this case and detecting it by taking advantage of the bug causing us to not emit compositionstart. Our only "cheat" is that we skip handling compositionend when it wasn't preceded by a compositionstart. Your Chrome 65 backspace fix is great and fits our current expectations. > You seem to have some code that assumes that whatever is sent in “compositionupdate” immediately before “compositionend” corresponds to newly-inserted text No, nothing like that. Basically, we make a range from the composition's start and end positions, and then update our internal document model on compositionend by inserting the end data at that range. Works for inserting and removing. Our only assumption is that composition events happen when the user is making an edit.
,
Feb 26 2018
How are you getting the composition start and end positions? Are you getting the composition range based on the current selection when compositionupdate is fired? If so, I think there’s some risk of this breaking in the future, since as I already noted, I think the selection changing when we replace the composition text is more of an implementation detail than something we’re explicitly exposing. I think a workaround for now (if I understand what you’re doing well enough) is to set a boolean to false when compositionstart is fired (might also need to listen for compositionupdate to handle older versions of Chrome), flip it to true if an input event is fired, and then check the boolean when compositionend is fired to see if the text was actually changed.
,
Mar 1 2018
> set a boolean to false when compositionstart is fired (might also need to listen for compositionupdate to handle older versions of Chrome), flip it to true if an input event is fired, and then check the boolean when compositionend is fired So I did exactly that and in less than 24 hours a problem was reported. Firefox (on Linux only) doesn't dispatch any input events during a composition. It does dispatch "input" *after* compositionend, but, as we already discussed, Chrome does not. The attached screenshot shows the events from typing "ión". Like I said, we have to make an educated guess now and it's far too easy to get wrong.
,
Mar 1 2018
It seems that Firefox's behavior in this case is non-conformant to the UI Events spec: https://www.w3.org/TR/uievents/#events-composition-input-events "During the composition session, the compositionupdate MUST be dispatched after the beforeinput is sent, but before the input event is sent." ... "Since there are no DOM updates associated with the compositionend event, beforeinput and input events should not be sent at that time." I see that there's an annoying conflict here: Firefox: compositionupdate always means the DOM was updated (according to what you're saying). Without making this assumption, there's no way to tell that the input event fired after compositionend actually resulted from the composition. Chrome: compositionupdate doesn't necessarily mean the DOM was updated. You have to listen for an input event that Firefox doesn't fire in the same order. Unfortunately I can't do a whole lot about Firefox's behavior. They might fix it if you file a bug.
,
Mar 2 2018
Can you take a look at https://bugs.chromium.org/p/chromium/issues/detail?id=818332 please? It makes our educated guess about compositions even more difficult to do well.
,
Aug 16
We are looking at updating Firefox to match the spec here (this behavior was spec'ed after the Firefox implementation) but are currently trying to clarify the value of isComposing for these events: https://github.com/w3c/uievents/issues/202 |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by d...@basecamp.com
, Feb 15 2018