Fix isComposing to be correct on IME commands |
||
Issue descriptionThis test case records the JavaScript events we fire for most IME commands: https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/events/inputevents/inputevent-ime.html The isComposing field is defined as follows: "true if the input event occurs as part of a composition session, i.e., after a compositionstart event and before the corresponding compositionend event. The un-initialized value of this attribute MUST be false." https://www.w3.org/TR/uievents/#interface-inputevent Several of the test cases have behavior that appears to be incorrect. "setComposition with open composition, empty text" sets isComposing to true for beforeinput, but false for input. It should be true for both. "insertText with open composition, empty text" does the same thing and should also have isComposing=true for both. I suspect that InputMethodController::DeleteSurroundingText() and ExtendSelectionAndDelete() may also need fixes.
,
Jan 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d2e83f778ee04311b18b6a5ee5f5f5786d6d51ea commit d2e83f778ee04311b18b6a5ee5f5f5786d6d51ea Author: Ryan Landay <rlanday@chromium.org> Date: Thu Jan 18 02:52:04 2018 Fix bug applying text formatting with open IME composition In r489705, we changed a piece of code in InputMethodController::SelectComposition() from: GetFrame().Selection().SetSelection( SelectionInDOMTree::Builder().SetBaseAndExtent(range).Build(), 0); to GetFrame().Selection().SetSelection( SelectionInDOMTree::Builder().SetBaseAndExtent(range).Build()); This is not correct, because calling FrameSelection::SetSelection() with no options param is not the same as calling it with no options: void FrameSelection::SetSelection(const SelectionInDOMTree& selection) { SetSelection(selection, SetSelectionOptions::Builder() .SetShouldCloseTyping(true) .SetShouldClearTypingStyle(true) .Build()); } The fix is to explicitly pass SetSelectionOptions() to SetSelection(), which does have the same behavior as the old code that passed 0. I've filed https://crbug.com/803278 to clean up SetSelection() to not set these flags, since this behavior seems to be a potential source of additional bugs. Bug: 801388 ,802916 Change-Id: I84ae2f0a19839da2b9d42cd589325c3ee098ae00 Reviewed-on: https://chromium-review.googlesource.com/871184 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#530030} [modify] https://crrev.com/d2e83f778ee04311b18b6a5ee5f5f5786d6d51ea/third_party/WebKit/LayoutTests/fast/events/inputevents/inputevent-ime.html [modify] https://crrev.com/d2e83f778ee04311b18b6a5ee5f5f5786d6d51ea/third_party/WebKit/Source/core/editing/ime/InputMethodController.cpp [modify] https://crrev.com/d2e83f778ee04311b18b6a5ee5f5f5786d6d51ea/third_party/WebKit/Source/core/editing/ime/InputMethodControllerTest.cpp
,
Feb 1 2018
I think the commands this is still wrong for (see inputevent-ime.html) are: - extendSelectionAndDelete: isComposing is false for beforeinput and input even if there's an open composition - deleteSurroundingText: isComposing is false for beforeinput and input even if there's an open composition |
||
►
Sign in to add a comment |
||
Comment 1 by rlanday@chromium.org
, Jan 17 2018