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

Issue 663944 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
inactive
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocked on:
issue 570920
issue 671595
issue 674295



Sign in to add a comment

Should we set caret position before or after 'compositionend'?

Project Member Reported by chongz@chromium.org, Nov 9 2016

Issue description

Background:
```Android’s InputConnection API specifies a way to commit (confirm) the current composition and move the cursor at the same time[1].```
 Issue 570920  added support by setting caret position after 'compositionend'.

Potential issue:
JS may want to listen to 'compositionend' and mutate DOM & move focus after IME finishes, however the above approach will reset caret position after 'compositionend' event listeners.
(But I don't find any broken websites yet)

Possible solution:
According to spec[2] I think we should fire 'compositionend' after all IME controls such as inserting text and moving caret.
e.g. Considering moving caret as part of composition.

I think this meets the original design[3] better as |setComposingText(text, newCursorPosition)| would move caret during composition.

[1] https://developer.android.com/reference/android/view/inputmethod/InputConnection.html#commitText(java.lang.CharSequence, int)

[2] https://www.w3.org/TR/uievents/#event-type-compositionend
```
A user agent MUST dispatch this event when a text composition system completes or cancels the current composition session, and the compositionend event MUST be dispatched after the control is updated.
```

[3] Same link as [1]
```
This behaves like calling setComposingText(text, newCursorPosition) then finishComposingText().
```
 

Comment 1 by yosin@chromium.org, Nov 10 2016

I support "before" == setting caret before dispatching "compositionend". Moving caret is one step of composition completing/canceling.

Since there is no spec or implementation dependent where caret during IME session, it is safe to set caret position before dispatching "compositionend".

Comment 2 by yabinh@chromium.org, Nov 10 2016

Owner: yabinh@chromium.org
Status: Started (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 18 2016

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

commit df699ce3ffd28396d0447fbc8072ebe7199857ca
Author: yabinh <yabinh@chromium.org>
Date: Fri Nov 18 22:03:34 2016

Make "compositionend" event fired after setting caret position

JS may want to listen to 'compositionend' and mutate DOM & move focus after IME finishes.
Besides, according to the spec (https://www.w3.org/TR/uievents/#event-type-compositionend),
we should fire 'compositionend' after all IME controls such as inserting text and moving
caret. But we are resetting caret position after 'compositionend' event listeners now.
This CL will make the order correct.

BUG= 663944 

TEST=run_webkit_layout_test --gtest_filter=InputMethodControllerTest.*

Review-Url: https://codereview.chromium.org/2493703002
Cr-Commit-Position: refs/heads/master@{#433308}

[modify] https://crrev.com/df699ce3ffd28396d0447fbc8072ebe7199857ca/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/df699ce3ffd28396d0447fbc8072ebe7199857ca/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp

Comment 4 by yabinh@chromium.org, Nov 25 2016

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 19 2017

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

commit 53003b5bb24d266ac78adf0623b2dd07c1ac6e9f
Author: aelias <aelias@chromium.org>
Date: Thu Jan 19 01:38:43 2017

Revert 'Make "compositionend" event fired after setting caret position'

This reverts http://crrev.com/2493703002, which was bisected to cause Google
Docs Korean IME  issue 674295 .  Note the revert is unclean in that the
structural changes to InputMethodControllerTest are preserved (only the
new tests are deleted) and a new updateStyleAndLayoutIgnorePendingStylesheets
was needed because of changes since the original landing.

TBR=yosin@chromium.org
BUG= 674295 , 663944 

Review-Url: https://codereview.chromium.org/2644623003
Cr-Commit-Position: refs/heads/master@{#444593}

[modify] https://crrev.com/53003b5bb24d266ac78adf0623b2dd07c1ac6e9f/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/53003b5bb24d266ac78adf0623b2dd07c1ac6e9f/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp

Comment 6 by yosin@chromium.org, Jan 19 2017

Status: Assigned (was: Fixed)
Unfortunately, the change was reverted, http://crrev.com/2644623003, due by Korean IME regression  issue 674295 

Comment 7 by yosin@chromium.org, Jan 19 2017

Blockedon: 674295

Comment 8 by yabinh@chromium.org, Jan 23 2017

Blockedon: 671595

Comment 9 by yabinh@chromium.org, Feb 15 2017

Owner: aelias@chromium.org
Reassigned to aelias@.
Status: Archived (was: Assigned)
I'm not planning to take further action on this since it seems like a risky change and the correct interpretation of the spec isn't that obvious in any case.  I think we should alter event order when the either the spec is clear or there are concrete use cases.

Sign in to add a comment