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

Issue 787598 link

Starred by 3 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 3
Type: Bug



Sign in to add a comment

Undo is super-wonky when typing with IME (e.g. most typing on Android)

Project Member Reported by rlanday@chromium.org, Nov 21 2017

Issue description

Chrome Version: 64.0.3274.0 (Developer Build) unknown (32-bit)
OS: Android N

What steps will reproduce the problem?
(1) Set up an Android device with Gboard and also a physical keyboard. Enable showing the on-screen keyboard while the physical keyboard is connected.
(2) Go to editpad.org and type a few words (on either keyboard).
(3) Repeatedly press Ctrl-Z on the physical keyboard to undo the typing.

What is the expected result?

Undo should work as it does on other platforms, where undo removes several words (e.g. a sentence) at a time.

What happens instead?

Undo undoes one character at a time, and causes the word with a character being removed from it to become selected.

This happens because InputMethodController::SetComposition() works by selecting the composition range (which closes out the previous TypingCommand, putting it on the undo stack; this is why undo happens one character at a time), replacing the selection, and then converting the selection back to a composition range (without updating the selection in the open TypingCommand; this is why undo selects the words).

I tested typing in Japanese on macOS and similar behavior resulted.

This bug would affect Android particularly badly (since pretty much all on-screen keyboards use composition ranges for composing text, even for alphabetic languages), except the only way I know of triggering undo on Android is pressing Ctrl-Z on a physical keyboard. If we ever want to expose undo in the UI, fixing this bug will become high-pri.
 
Owner: rlanday@chromium.org
Status: Started (was: Untriaged)
android undo.mp4
297 KB View Download
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 1 2017

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

commit 812d7fc095e9093848a6207eaa635772a7bb52f4
Author: Ryan Landay <rlanday@chromium.org>
Date: Fri Dec 01 05:04:21 2017

Fix incorrect ending selection after InputMethodController::SetComposition()

InputMethodController::SetComposition() works as follows:

1. The current composition range is selected (we don't show this to users).
2. The selection is replaced with the new text, which is left selected.
3. The resulting selection is converted to a composition range, and we change
   the selection to whatever the IME requested be selected afterwards.

Step 2 causes the open TypingCommand's ending selection to be set to the
resulting composition range. We need to update it after step 3 to fix two
problems:

1. Pressing enter on a physical keyboard on Android with an open composition
   range causes the text in the composition range to be incorrectly deleted.

2. Pressing Ctrl-Z to undo causes each word to be selected in turn (see video on
   https://crbug.com/787598).

There's another problem with undo on Android, which is that the TypingCommand is
closed out after every call to SetComposition() when we select the composition,
which means undo only undoes one character at a time. I might fix this in a
separate CL.

Bug: 787598,  772565 ,  772584 
Change-Id: I29f09e3c0bd97c6c8e17e5455c5579b53aa34c1b
Reviewed-on: https://chromium-review.googlesource.com/783770
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520857}
[modify] https://crrev.com/812d7fc095e9093848a6207eaa635772a7bb52f4/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/812d7fc095e9093848a6207eaa635772a7bb52f4/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnection.java
[modify] https://crrev.com/812d7fc095e9093848a6207eaa635772a7bb52f4/third_party/WebKit/Source/core/editing/commands/TypingCommand.h
[modify] https://crrev.com/812d7fc095e9093848a6207eaa635772a7bb52f4/third_party/WebKit/Source/core/editing/ime/InputMethodController.cpp
[modify] https://crrev.com/812d7fc095e9093848a6207eaa635772a7bb52f4/third_party/WebKit/Source/core/editing/ime/InputMethodControllerTest.cpp

Cc: rlanday@chromium.org
Owner: ----
Status: Available (was: Started)

Sign in to add a comment