New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 764439 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: ----



Sign in to add a comment

composition event should be triggered on setComposingRegion

Project Member Reported by changwan@chromium.org, Sep 12 2017

Issue description

Steps to reproduce:
(1) Use GBoard English keyboard.
(2) Go to https://jsfiddle.net/galmacky/moL0k72d/embedded/result/
(3) Type "ab cd" and check that "cd" is underlined.
(4) Go to "ab".

Expected result:
Step (4) should trigger composition event.

Actual result:
Step (4) does not produce any composition event.

Note that the current web standards do not mention the above use case, where Android's InputConnection#setComposingRegion() changes composition offsets:
https://www.w3.org/TR/uievents/#event-type-compositionupdate

But I think it should trigger compositionstart and compositionupdate if we were not previously composing text. It should only trigger compositionupdate if we were previously composing text.

And it would make sense to keep composition offsets in the compositionevent, which may require web standards change.

This can also be reproduced with the following demo URL - composition line does not change on the virtual editor when you touch outside the current composition while using GBoard app.

http://jsbin.com/betafu/57/

rlanday@, could you take a look?
 
I think there might be some complexity here; see crbug.com/714791
Owner: aelias@chromium.org
aelias@: does this change make sense? Or would it be controversial?
Cc: rlanday@chromium.org

Comment 4 by aelias@chromium.org, Sep 12 2017

Owner: rlanday@chromium.org
Yes, in general any change in composition underline should produce an event so that JS can simulate a textbox as needed.

I don't think there's any problem relating to http://crbug.com/714791.  What we can't support in that bug is cancelable input events.  In general, there's no problem with generating more noncancelable events.  Also, this bug is not about input events but composition events (the way I think of it is, input events say when text has been mutated, whereas composition events say when document markers change).

Comment 5 by yosin@chromium.org, Jan 10 2018

Labels: Pri-3
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d20346715860fa7c0453cbcf8e4b371560e7dca5

commit d20346715860fa7c0453cbcf8e4b371560e7dca5
Author: Ryan Landay <rlanday@chromium.org>
Date: Fri Jan 12 22:22:32 2018

Add JS event test for InputMethodController::SetCompositionFromExistingText()

changwan@ reported a bug ( crbug.com/764439 ) that
InputMethodController::SetCompositionFromExistingText (used by Android IMEs to
mark the current word with a composition range when you move the cursor) does
not fire the compositionstart or compositionupdate events. This CL adds a layout
test for the current behavior. I will write a separate CL to fix the bug and
update the test with the new behavior.

Bug:  764439 
Change-Id: I17fae44210cea9383b41cea2bd7974c1db52bea5
Reviewed-on: https://chromium-review.googlesource.com/862621
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529084}
[modify] https://crrev.com/d20346715860fa7c0453cbcf8e4b371560e7dca5/content/shell/test_runner/text_input_controller.cc
[modify] https://crrev.com/d20346715860fa7c0453cbcf8e4b371560e7dca5/content/shell/test_runner/text_input_controller.h
[modify] https://crrev.com/d20346715860fa7c0453cbcf8e4b371560e7dca5/third_party/WebKit/LayoutTests/fast/events/inputevents/inputevent-ime.html

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/852ab8bfa27da53f0790a230bbabeac5f4175729

commit 852ab8bfa27da53f0790a230bbabeac5f4175729
Author: Ryan Landay <rlanday@chromium.org>
Date: Wed Jan 17 08:06:57 2018

Fire composition{start,update} events for SetCompositionFromExistingText()

Currently, an IME calling
InputMethodController::SetCompositionFromExistingText() (e.g. an Android IME
calling InputConnection#setComposingRegion()) does not fire the compositionstart
and compositionupdate events. This CL fixes this method to fire these two
events.

Bug:  764439 
Change-Id: Ib15f0355b32d75379021d1a8d32e4cc7cdfced8a
Reviewed-on: https://chromium-review.googlesource.com/865825
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529672}
[modify] https://crrev.com/852ab8bfa27da53f0790a230bbabeac5f4175729/third_party/WebKit/LayoutTests/fast/events/inputevents/inputevent-ime.html
[modify] https://crrev.com/852ab8bfa27da53f0790a230bbabeac5f4175729/third_party/WebKit/Source/core/editing/ime/InputMethodController.cpp
[modify] https://crrev.com/852ab8bfa27da53f0790a230bbabeac5f4175729/third_party/WebKit/Source/core/editing/ime/InputMethodController.h

Labels: M-65
Status: Fixed (was: Assigned)

Comment 9 by ja...@javan.us, Feb 16 2018

Hello,

I’m one of the maintainers of Trix (https://github.com/basecamp/trix), a rich text editor for the web. Trix, like many modern editors, listens for events to record input, and then converts that input into an editing operation on its internal document model. This process has always been challenging on Android due to its lack of key codes on keyboard events (this ol’ issue https://bugs.chromium.org/p/chromium/issues/detail?id=118639), but we’ve managed well enough by handling composition events.

The change that fixed this issue is a significant departure from the current behavior of composition events, and problematic in a number ways:

1. Composition events are dispatched every time you move the cursor but aren’t typing. 

This alone quite a stretch from from the spec’s definition: “Composition Events provide a means for inputing text in a supplementary or alternate manner than by Keyboard Events” (https://www.w3.org/TR/uievents/#events-compositionevents). These composition events while moving the cursor also have no associated keyboard or inputs events, another deviation from the spec: “During the composition session, keydown and keyup events MUST still be sent” (https://www.w3.org/TR/uievents/#events-composition-key-events).

2. The DOM selection during compositions now varies depending on the context.

When moving the cursor, the selection during compositionupdate is always collapsed at the cursor’s position. When typing, the selection is the expanded range around the composed characters. This means we can’t reliably determine if the input is new or if it’s replacing a range of existing content.

Description of the attached screenshots:

cursor-movement.png: Shows the composition events dispatched after moving the cursor from left to right four times. Note how the compositionupdate data is identical, but the DOM selection is not.

movement-then-typing.png: Shows the events dispatched when placing the cursor before "The", "quick", and "brown", and then typing "fox " at the end. This illustrates how the selection expands while typing, and how difficult it is to distinguish input from cursor movement generally.

These screenshots come from http://jsfiddle.net/javan/mj13jj7b/embedded/result,js/ - a small test bed I made.

Related issue: https://bugs.chromium.org/p/chromium/issues/detail?id=812674

Thanks for your time!
cursor-movement.png
69.4 KB View Download
movement-then-typing.png
217 KB View Download
> 1. Composition events are dispatched every time you move the cursor but aren’t typing. 

> This alone quite a stretch from from the spec’s definition: “Composition Events provide a means for inputing text in a supplementary or alternate manner than by Keyboard Events” (https://www.w3.org/TR/uievents/#events-compositionevents).

We're just passing along information about what the keyboard is doing. Pretty much all input from Android on-screen keyboards comes as a sequence of composition-related commands. Before this change, I think there were situations where we were firing compositionupdate events without ever firing a compositionstart event. Doesn't that seem kind of wonky?



> These composition events while moving the cursor also have no associated keyboard or inputs events, another deviation from the spec: “During the composition session, keydown and keyup events MUST still be sent” (https://www.w3.org/TR/uievents/#events-composition-key-events).

I'm not 100% how to interpret this, but it's not clear to me that the spec is saying we should emit keydown/keyup events for composition events that don't actually correspond to pressing a key on a (virtual or physical) keyboard. Would it actually be helpful for you if we did this? Or would it just be one more event for stuff to get confused by?


> 2. The DOM selection during compositions now varies depending on the context.

> When moving the cursor, the selection during compositionupdate is always collapsed at the cursor’s position. When typing, the selection is the expanded range around the composed characters. This means we can’t reliably determine if the input is new or if it’s replacing a range of existing content.

I think this is because the editing code selects the composition before replacing it with the updated composition text. I think this selection is supposed to be an implementation detail, so I'm not sure it's intentional that we're actually reporting it in the compositionupdate event at all. The newly-added events are correctly reporting that no text is selected.

Comment 11 by ja...@javan.us, Feb 16 2018

> We're just passing along information about what the keyboard is doing. Pretty much all input from Android on-screen keyboards comes as a sequence of composition-related commands.

In this case there is no input. Touching the screen to move the cursor isn't the same as composing, and the keyboard isn't *doing* anything.

> Before this change, I think there were situations where we were firing compositionupdate events without ever firing a compositionstart event. Doesn't that seem kind of wonky?

It is! We currently ignore updates that aren't preceded by a start.

> but it's not clear to me that the spec is saying we should emit keydown/keyup events for composition events that don't actually correspond to pressing a key

I'm not 100% either, but another way of reading it is that you *shouldn't* emit composition events when there's no corresponding key press.

> I think this selection is supposed to be an implementation detail, so I'm not sure it's intentional that we're actually reporting it in the compositionupdate event at all

To be clear, I'm not getting the selection from the event. It's what the DOM reports from an input element's selectionStart/selectionEnd properties or from window.getSelection().
> Touching the screen to move the cursor isn't the same as composing, and the keyboard isn't *doing* anything.

From both the keyboard's perspective and Chrome's perspective, there's an active composition range open. You're asking us if we can add extra logic to suppress composition updates that don't care about for your use case. I don't think we want to do that because one, it's more complexity for us to deal with, and two, we're not sure all developers will be interested in the same subset of composition events. 

> I'm not 100% either, but another way of reading it is that you *shouldn't* emit composition events when there's no corresponding key press.

That idea doesn't make sense though because not all composition events come from on-screen keyboards. Voice input IMEs also use composition commands to input text.

Comment 13 by ja...@javan.us, Feb 16 2018

> You're asking us if we can add extra logic to suppress composition updates

I'm definitely not asking for that, and not really trying to debate how the spec should be interpreted either. I'm just asking you to consider the changes you made "because it seemed more correct to me" from the perspective of those who use them.
I still think it’s more correct. I agree with aelias@’s comment:

https://bugs.chromium.org/p/chromium/issues/detail?id=764439#c4
> the way I think of it is, input events say when text has been mutated, whereas composition events say when document markers change

I.e. the composition{start,update,end} events tell you what’s going with the keyboard’s composition underline. When using Gboard on Android, the composition underline moves when you move the cursor, so that’s why we’re firing the events. I think it’s unlikely that we’ll make any large changes to our general strategy of how we interact with IMEs in the near future.

The argument I’m most sympathetic to is that different versions of Chrome are firing events differently and it’s difficult to properly support them all. My concern is that we may have told developers at one point “diff the compositionupdate strings against each other to determine when virtual keys are being pressed” and now there’s a bunch of code out there misusing these events to listen for DOM updates.

Comment 15 by ja...@javan.us, Feb 16 2018

Except that's not really true. The DOM is mutated throughout the composition process. The keyboard's underlining is an OS-specific UI detail.

https://jsfiddle.net/javan/3akghfq5/
composing.png
209 KB View Download
I agree that we update the DOM if you‘re actually typing or otherwise changing the text using an IME. That’s why we fire the input event when this happens.

Comment 17 by ja...@javan.us, Feb 18 2018

> My concern is that we may have told developers at one point “diff the compositionupdate strings against each other to determine when virtual keys are being pressed” and now there’s a bunch of code out there misusing these events to listen for DOM updates.

That seems likely since hundreds of developers have asked this question since 2012 (https://bugs.chromium.org/p/chromium/issues/detail?id=118639). What is the correct, Chrome-approved way to determine what virtual keys are being pressed?

Android is by far the most difficult platform for rich text editors to support. I encourage you to look through just a few related issues:

- https://github.com/facebook/draft-js/issues/1224
- https://github.com/facebook/draft-js/labels/android
- https://github.com/ProseMirror/prosemirror/issues/576
- https://github.com/basecamp/trix/issues/471#issuecomment-357272814

This change to composition events makes supporting Android even more difficult by giving them two different meanings. Quietly introducing it in the next Chrome release will break text editing on a countless number of web applications.

Sign in to add a comment