Attachment 346813 Details for Bug 188370 – Fixes the bug

[patch] Fixes the bug

bug-188370-20180808195459.patch (text/plain), 8.97 KB, created by Ryosuke Niwa on 2018-08-08 19:54:59 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-08-08 19:54:59 PDT

Size: 8.97 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 234710)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,29 @@
>+2018-08-08  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        REGRESSION (r228260): Events handled by input method invoke default event handler
>+        https://bugs.webkit.org/show_bug.cgi?id=188370
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The bug was caused by the default event handler being invoked for the "virtual" keydown events
>+        generated for input methods. Namely, when Japanese or Chinese input methods swallows TAB key,
>+        WebKit should not invoke the default event handler and move the focus during input composition.
>+
>+        Fixed the bug by adding a new boolean on Event indicating whether the default event handler
>+        should be invoked, and restoring the behavior prior to r228260 for these virtual keydown events.
>+
>+        Test: editing/input/press-tab-during-ime-composition.html
>+
>+        * dom/Event.h:
>+        (WebCore::Event::ignoreDefaultEventHandler const): Added.
>+        (WebCore::Event::setIgnoreDefaultEventHandler): Added.
>+        * dom/EventDispatcher.cpp:
>+        (WebCore::EventDispatcher::dispatchEvent): Don't invoke the default event handler if
>+        ignoreDefaultEventHandler is set.
>+        * page/EventHandler.cpp:
>+        (WebCore::EventHandler::internalKeyEvent): Set ignoreDefaultEventHandler. This restores WebKit's
>+        behavior prior to r228260.
>+
> 2018-08-08  Don Olmstead  <don.olmstead@sony.com>
> 
>         [Nicosia] Add the Nicosia-specific PlatformLayer type alias
>Index: Source/WebCore/dom/Event.h
>===================================================================
>--- Source/WebCore/dom/Event.h	(revision 234710)
>+++ Source/WebCore/dom/Event.h	(working copy)
>@@ -119,6 +119,9 @@ public:
>     bool defaultHandled() const { return m_defaultHandled; }
>     void setDefaultHandled() { m_defaultHandled = true; }
> 
>+    bool ignoreDefaultEventHandler() const { return m_ignoreDefaultEventHandler; }
>+    void setIgnoreDefaultEventHandler() { m_ignoreDefaultEventHandler = true; }
>+
>     void setInPassiveListener(bool value) { m_isExecutingPassiveEventListener = value; }
> 
>     bool cancelBubble() const { return propagationStopped(); }
>@@ -156,6 +159,7 @@ private:
>     bool m_immediatePropagationStopped { false };
>     bool m_wasCanceled { false };
>     bool m_defaultHandled { false };
>+    bool m_ignoreDefaultEventHandler { false };
>     bool m_isTrusted { false };
>     bool m_isExecutingPassiveEventListener { false };
> 
>Index: Source/WebCore/dom/EventDispatcher.cpp
>===================================================================
>--- Source/WebCore/dom/EventDispatcher.cpp	(revision 234710)
>+++ Source/WebCore/dom/EventDispatcher.cpp	(working copy)
>@@ -167,7 +167,7 @@ void EventDispatcher::dispatchEvent(Node
>     // Call default event handlers. While the DOM does have a concept of preventing
>     // default handling, the detail of which handlers are called is an internal
>     // implementation detail and not part of the DOM.
>-    if (!event.defaultPrevented() && !event.defaultHandled()) {
>+    if (!event.defaultPrevented() && !event.defaultHandled() && !event.ignoreDefaultEventHandler()) {
>         // FIXME: Not clear why we need to reset the target for the default event handlers.
>         // We should research this, and remove this code if possible.
>         auto* finalTarget = event.target();
>Index: Source/WebCore/page/EventHandler.cpp
>===================================================================
>--- Source/WebCore/page/EventHandler.cpp	(revision 234710)
>+++ Source/WebCore/page/EventHandler.cpp	(working copy)
>@@ -3288,7 +3288,7 @@ bool EventHandler::internalKeyEvent(cons
>         keyDownEvent.setWindowsVirtualKeyCode(CompositionEventKeyCode);
>         keydown = KeyboardEvent::create(keyDownEvent, &m_frame.windowProxy());
>         keydown->setTarget(element);
>-        keydown->setDefaultHandled();
>+        keydown->setIgnoreDefaultEventHandler();
>     }
>     
>     if (accessibilityPreventsEventPropagation(keydown))
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 234710)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>+2018-08-08  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        REGRESSION (r228260): Events handled by input method invoke default event handler
>+        https://bugs.webkit.org/show_bug.cgi?id=188370
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added a regression test. Due to the lack of adaquete support of textInputController in WebKitTestRunner,
>+        the test is only enabled in WebKit1. The WTR fix is tracked by https://webkit.org/b/188428.
>+
>+        * editing/input/press-tab-during-ime-composition-expected.txt: Added.
>+        * editing/input/press-tab-during-ime-composition.html: Added.
>+        * platform/ios/TestExpectations:
>+        * platform/wk2/TestExpectations:
>+
> 2018-08-08  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Web Inspector: XHR content sometimes shows as error even though load succeeded
>Index: LayoutTests/editing/input/press-tab-during-ime-composition-expected.txt
>===================================================================
>--- LayoutTests/editing/input/press-tab-during-ime-composition-expected.txt	(nonexistent)
>+++ LayoutTests/editing/input/press-tab-during-ime-composition-expected.txt	(working copy)
>@@ -0,0 +1,6 @@
>+This test ensures WebKit does not execute the default event handler of keydown when the user presses TAB key while IME composition is taking place.
>+To test manually, type "kyo" using Japanese hiragana keyboard in the first text field, and press TAB key.
>+WebKit should not move the focus to the second text field.
>+
>+PASS
>+
>Index: LayoutTests/editing/input/press-tab-during-ime-composition.html
>===================================================================
>--- LayoutTests/editing/input/press-tab-during-ime-composition.html	(nonexistent)
>+++ LayoutTests/editing/input/press-tab-during-ime-composition.html	(working copy)
>@@ -0,0 +1,48 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<p>This test ensures WebKit does not execute the default event handler of keydown when the user presses TAB key while IME composition is taking place.<br>
>+To test manually, type "kyo" using Japanese hiragana keyboard in the first text field, and press TAB key.<br>
>+WebKit should not move the focus to the second text field.</p>
>+<div id="container">
>+<label>First text field:<input id="first" type="text"></label><br>
>+<label>Second text field:<input id="second" type="text" onfocus="log('FAIL: the second text field got focused')"></label><br>
>+</div>
>+<pre id="result"></pre>
>+<script>
>+
>+const first = document.getElementById('first');
>+function runTest() {
>+    first.focus();
>+    first.selectionStart = 0;
>+    first.selectionEnd = 0;
>+    textInputController.setInputMethodHandler(function (event) {
>+        if (event.characters[0] == '\t') {
>+            textInputController.setMarkedText("\u4eca\u65e5", 0, 2);
>+            return true;
>+        }
>+        return false;
>+    })
>+    textInputController.setMarkedText("kyo", 0, 3);
>+    eventSender.keyDown('\t');
>+
>+    if (document.activeElement != first)
>+        return log('FAIL: the focus did not remain in the fisrt text field');
>+
>+    log('PASS');
>+
>+    document.getElementById('container').style.display = 'none';
>+}
>+
>+function log(text) {
>+    document.getElementById('result').textContent += text + '\n';
>+}
>+
>+if (window.testRunner) {
>+    testRunner.dumpAsText();
>+    runTest();
>+}
>+
>+</script>
>+</body>
>+</html>
>Index: LayoutTests/platform/ios/TestExpectations
>===================================================================
>--- LayoutTests/platform/ios/TestExpectations	(revision 234710)
>+++ LayoutTests/platform/ios/TestExpectations	(working copy)
>@@ -810,6 +810,7 @@ editing/input/page-up-down-scrolls.html 
> editing/input/password-echo-passnode.html [ Skip ]
> editing/input/password-echo-passnode2.html [ Skip ]
> editing/input/password-echo-passnode3.html [ Skip ]
>+editing/input/press-tab-during-ime-composition.html [ Skip ]
> editing/input/reveal-caret-of-transformed-multiline-input.html [ Skip ]
> editing/input/reveal-edit-on-input-vertically.html [ Skip ]
> editing/input/reveal-contenteditable-on-input-vertically.html [ Skip ]
>Index: LayoutTests/platform/wk2/TestExpectations
>===================================================================
>--- LayoutTests/platform/wk2/TestExpectations	(revision 234710)
>+++ LayoutTests/platform/wk2/TestExpectations	(working copy)
>@@ -88,6 +88,9 @@ platform/mac/fast/text/attributed-substr
> platform/mac/fast/text/attributed-substring-from-range.html
> platform/mac/fast/text/justified-text-rect.html
> 
>+# [WebKit2] WebKitTestRunner doesn't support emulating pressing a tab key during IME composition
>+webkit.org/b/188428 editing/input/press-tab-during-ime-composition.html
>+
> # WebKitTestRunner needs testRunner.setMainFrameIsFirstResponder
> # <https://bugs.webkit.org/show_bug.cgi?id=42685>
> editing/selection/caret-and-focus-ring.html

Flags:

wenson_hsieh: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 188370: 346676 | 346801 | 346813