Attachment 373390 Details for Bug 199430 – Patch

[patch] Patch

bug-199430-20190703082216.patch (text/plain), 13.81 KB, created by Wenson Hsieh on 2019-07-03 08:22:16 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-07-03 08:22:16 PDT

Size: 13.81 KB

patch

obsolete

>Subversion Revision: 247030
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index bd449d0600998620c1ea9cf8be30083af77dddc8..5a97549f475a9275450772820462c9db96ec3dc5 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,35 @@
>+2019-07-03  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        REGRESSION (iOS 13): Tapping an element with a click event handler no longer clears the selection
>+        https://bugs.webkit.org/show_bug.cgi?id=199430
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        After <trac.webkit.org/r245067>, we no longer immediately clear the text selection when recognizing a single tap
>+        in WKContentView, and instead only clear it out in the case where the single tap didn't result in a click event
>+        in the web process. This fixed an issue wherein the text selection would be prematurely cleared when tapping,
>+        but also made it such that tapping on an element with a click event handler would not cause the selection to
>+        change, even if preventDefault() is not called on mousedown. On web pages that add a click event listener to
>+        `document.body`, it's nearly impossible to dismiss text selections by tapping elsewhere in the body.
>+
>+        On macOS, this works because EventHandler::handleMousePressEventSingleClick contains logic to modify the
>+        selection when handling a mousedown, as a part of default behavior. However, there is platform-specific logic
>+        added in <trac.webkit.org/r233311> that avoids changing the selection when handling a synthetic mousedown on
>+        iOS; this is because we defer to the single tap text interaction gesture on iOS, which (among other things)
>+        provides additional support for moving the selection to word boundaries, instead of the editing position
>+        directly under the click.
>+
>+        However, no such platform-specific text interaction single tap gesture exists for non-editable text, so there's
>+        no reason we need to bail in the case where the root editable element is null. We can fix this bug without
>+        breaking the fix in r233311 by matching macOS behavior and not bailing via early return in the case where the
>+        single tap would move selection into non-editable text.
>+
>+        Tests: editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler.html
>+               editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html
>+
>+        * page/EventHandler.cpp:
>+        (WebCore::EventHandler::handleMousePressEventSingleClick):
>+
> 2019-07-01  Chris Dumez  <cdumez@apple.com>
> 
>         It should not be possible to trigger a load while in the middle of restoring a page in PageCache
>diff --git a/Source/WebCore/page/EventHandler.cpp b/Source/WebCore/page/EventHandler.cpp
>index a30c9e9437e75a0b25ee7d8530855b25a54a08be..2b48f4b02a5518e09474c98c203af539d355b72b 100644
>--- a/Source/WebCore/page/EventHandler.cpp
>+++ b/Source/WebCore/page/EventHandler.cpp
>@@ -692,7 +692,8 @@ bool EventHandler::handleMousePressEventSingleClick(const MouseEventWithHitTestR
> 
> #if PLATFORM(IOS_FAMILY)
>     // The text selection assistant will handle selection in the case where we are already editing the node
>-    if (newSelection.rootEditableElement() == targetNode->rootEditableElement())
>+    auto* editableRoot = newSelection.rootEditableElement();
>+    if (editableRoot && editableRoot == targetNode->rootEditableElement())
>         return true;
> #endif
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 5f75eda5a09cb27821e8674831a2d8eb96dc9788..439554a1246a8d0ca4d224742d7e5f9c9881b17f 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-07-03  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        REGRESSION (iOS 13): Tapping an element with a click event handler no longer clears the selection
>+        https://bugs.webkit.org/show_bug.cgi?id=199430
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add and adjust layout tests to verify that calling preventDefault() on mousedown on iOS causes an existing
>+        selection to not be cleared, and that tapping in an element with a click handler clears out the selection.
>+
>+        * editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler-expected.txt: Added.
>+        * editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler.html: Added.
>+        * editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler-expected.txt: Renamed.
>+        * editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html:
>+
>+        Renamed from LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html,
>+        and adjusted to call preventDefault() on mousedown events instead of click events. Also, remove a bit of
>+        trailing whitespace.
>+
> 2019-07-01  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [iOS 13] editing/selection/ios/dispatch-mouse-events-when-modifying-selection-quirk.html fails on trunk
>diff --git a/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler-expected.txt b/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..660a6ac2cea998a279ad2762c8ac983612e0d162
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler-expected.txt
>@@ -0,0 +1,12 @@
>+This test verifies that the DOM selection is dismissed when tapping on an element that listens to click events. To manually test, select 'WebKit' and tap on the red square. The selection should be dismissed, and the output area should indicate that no text is selected.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS getSelection().toString() is ""
>+PASS getSelection().type is "Caret"
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+WebKit
>+The selected text is: ""
>diff --git a/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler.html b/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..7356329853e42a377010f9116c7b1111b7a7d034
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler.html
>@@ -0,0 +1,53 @@
>+<!DOCTYPE html> 
>+<html>
>+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
>+<head>
>+    <script src="../../../resources/ui-helper.js"></script>
>+    <script src="../../../resources/js-test.js"></script>
>+    <style>
>+    body {
>+        margin: 0;
>+    }
>+
>+    #target {
>+        font-size: 100px;
>+    }
>+
>+    #clickTarget {
>+        width: 100px;
>+        height: 100px;
>+        background-color: tomato;
>+    }
>+    </style>
>+    <script>
>+    jsTestIsAsync = true;
>+    progress = 0;
>+
>+    function checkProgress() {
>+        if (++progress != 2)
>+            return;
>+
>+        shouldBeEqualToString("getSelection().toString()", "");
>+        shouldBeEqualToString("getSelection().type", "Caret");
>+        finishJSTest();
>+    }
>+
>+    function run() {
>+        description("This test verifies that the DOM selection is dismissed when tapping on an element that listens to click events. To manually test, select 'WebKit' and tap on the red square. The selection should be dismissed, and the output area should indicate that no text is selected.");
>+
>+        document.addEventListener("selectionchange", () => document.getElementById("result").textContent = getSelection().toString());
>+        const clickTarget = document.getElementById("clickTarget");
>+        clickTarget.addEventListener("click", checkProgress);
>+        getSelection().selectAllChildren(document.getElementById("selectionTarget"));
>+
>+        if (window.testRunner)
>+            UIHelper.activateElement(clickTarget).then(checkProgress);
>+    }
>+    </script>
>+</head>
>+<body onload="run()">
>+    <div id="selectionTarget">WebKit</div>
>+    <div id="clickTarget"></div>
>+    <pre>The selected text is: "<span id="result"></span>"</pre>
>+</body>
>+</html>
>diff --git a/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler-expected.txt b/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler-expected.txt
>deleted file mode 100644
>index e6d97ef4fdd88f241ebc862748ce1f29546a6ab9..0000000000000000000000000000000000000000
>--- a/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler-expected.txt
>+++ /dev/null
>@@ -1,3 +0,0 @@
>-WebKit
>-The selected text is: "WebKit"
>-This test verifies that the DOM selection is not dismissed when tapping on an element that preventDefault()s the click event.
>diff --git a/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html b/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html
>deleted file mode 100644
>index e38f4d30fc04ff431a713fbad6e152dc0a61e252..0000000000000000000000000000000000000000
>--- a/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html
>+++ /dev/null
>@@ -1,60 +0,0 @@
>-<!DOCTYPE html> 
>-<html>
>-<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
>-<head>
>-    <script src="../../../resources/basic-gestures.js"></script>
>-    <script src="../../../resources/ui-helper.js"></script>
>-    <style>
>-    body {
>-        margin: 0;
>-    }
>-
>-    #target {
>-        font-size: 100px;
>-    }
>-
>-    #clickTarget {
>-        width: 100px;
>-        height: 100px;
>-        background-color: green;
>-    }
>-    </style>
>-    <script>
>-    if (window.testRunner) {
>-        testRunner.dumpAsText();
>-        testRunner.waitUntilDone();
>-    }
>-
>-    async function run()
>-    {
>-        if (!window.testRunner)
>-            return;
>-
>-        function didChangeSelection()
>-        {
>-            result.textContent = window.getSelection().toString()
>-        }
>-
>-        document.addEventListener("selectionchange", didChangeSelection);
>-
>-        var clickTarget = document.getElementById("clickTarget");
>-
>-        clickTarget.addEventListener("click", event => {
>-            event.preventDefault();
>-            setTimeout(() => testRunner.notifyDone(), 0);
>-        });
>-
>-        var target = document.getElementById("target");        
>-        window.getSelection().setBaseAndExtent(target, 0, target, 6);
>-
>-        await UIHelper.activateElement(clickTarget);
>-    }
>-    </script>
>-</head>
>-<body onload="run()">
>-    <div id="target">WebKit</div>
>-    <div id="clickTarget"></div>
>-    <pre>The selected text is: "<span id="result"></span>"</pre>
>-    <p>This test verifies that the DOM selection is not dismissed when tapping on an element that preventDefault()s the click event.</p>
>-</body>
>-</html>
>diff --git a/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler-expected.txt b/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..611c694affb236c7a957105a969899521dbfaad2
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler-expected.txt
>@@ -0,0 +1,3 @@
>+WebKit
>+The selected text is: "WebKit"
>+This test verifies that the DOM selection is not dismissed when tapping on an element that preventDefault()s the mousedown event.
>diff --git a/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html b/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..89b76cd9245370459a7c4eeb009e0890d7ae13f2
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html
>@@ -0,0 +1,60 @@
>+<!DOCTYPE html> 
>+<html>
>+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
>+<head>
>+    <script src="../../../resources/basic-gestures.js"></script>
>+    <script src="../../../resources/ui-helper.js"></script>
>+    <style>
>+    body {
>+        margin: 0;
>+    }
>+
>+    #target {
>+        font-size: 100px;
>+    }
>+
>+    #clickTarget {
>+        width: 100px;
>+        height: 100px;
>+        background-color: green;
>+    }
>+    </style>
>+    <script>
>+    if (window.testRunner) {
>+        testRunner.dumpAsText();
>+        testRunner.waitUntilDone();
>+    }
>+
>+    async function run()
>+    {
>+        if (!window.testRunner)
>+            return;
>+
>+        function didChangeSelection()
>+        {
>+            result.textContent = window.getSelection().toString()
>+        }
>+
>+        document.addEventListener("selectionchange", didChangeSelection);
>+
>+        var clickTarget = document.getElementById("clickTarget");
>+
>+        clickTarget.addEventListener("mousedown", event => {
>+            event.preventDefault();
>+            setTimeout(() => testRunner.notifyDone(), 0);
>+        });
>+
>+        var target = document.getElementById("target");
>+        window.getSelection().setBaseAndExtent(target, 0, target, 6);
>+
>+        await UIHelper.activateElement(clickTarget);
>+    }
>+    </script>
>+</head>
>+<body onload="run()">
>+    <div id="target">WebKit</div>
>+    <div id="clickTarget"></div>
>+    <pre>The selected text is: "<span id="result"></span>"</pre>
>+    <p>This test verifies that the DOM selection is not dismissed when tapping on an element that preventDefault()s the mousedown event.</p>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 199430: 373390