- Revision
- 247100
- Author
- [email protected]
- Date
- 2019-07-03 12:59:02 -0700 (Wed, 03 Jul 2019)
Log Message
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 Tim Horton.
Source/WebCore:
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):
LayoutTests:
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.
Modified Paths
Added Paths
Removed Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (247099 => 247100)
--- trunk/LayoutTests/ChangeLog 2019-07-03 19:53:30 UTC (rev 247099)
+++ trunk/LayoutTests/ChangeLog 2019-07-03 19:59:02 UTC (rev 247100)
@@ -1,3 +1,22 @@
+2019-07-03 Wenson Hsieh <[email protected]>
+
+ 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 Tim Horton.
+
+ 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-03 Russell Epstein <[email protected]>
Rebaseline fast/events/ios/keydown-keyup-special-keys-in-non-editable-element.html
Added: trunk/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler-expected.txt (0 => 247100)
--- trunk/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler-expected.txt 2019-07-03 19:59:02 UTC (rev 247100)
@@ -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: ""
Added: trunk/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler.html (0 => 247100)
--- trunk/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler.html (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-click-handler.html 2019-07-03 19:59:02 UTC (rev 247100)
@@ -0,0 +1,53 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+<head>
+ <script src=""
+ <script src=""
+ <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>
Deleted: trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler-expected.txt (247099 => 247100)
--- trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler-expected.txt 2019-07-03 19:53:30 UTC (rev 247099)
+++ trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler-expected.txt 2019-07-03 19:59:02 UTC (rev 247100)
@@ -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.
Deleted: trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html (247099 => 247100)
--- trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html 2019-07-03 19:53:30 UTC (rev 247099)
+++ trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html 2019-07-03 19:59:02 UTC (rev 247100)
@@ -1,60 +0,0 @@
-<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
-<html>
-<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
-<head>
- <script src=""
- <script src=""
- <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>
Copied: trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler-expected.txt (from rev 247099, trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler-expected.txt) (0 => 247100)
--- trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler-expected.txt 2019-07-03 19:59:02 UTC (rev 247100)
@@ -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.
Copied: trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html (from rev 247099, trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html) (0 => 247100)
--- trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-mousedown-handler.html 2019-07-03 19:59:02 UTC (rev 247100)
@@ -0,0 +1,60 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+<head>
+ <script src=""
+ <script src=""
+ <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>
Modified: trunk/Source/WebCore/ChangeLog (247099 => 247100)
--- trunk/Source/WebCore/ChangeLog 2019-07-03 19:53:30 UTC (rev 247099)
+++ trunk/Source/WebCore/ChangeLog 2019-07-03 19:59:02 UTC (rev 247100)
@@ -1,3 +1,35 @@
+2019-07-03 Wenson Hsieh <[email protected]>
+
+ 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 Tim Horton.
+
+ 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-03 Ryan Haddad <[email protected]>
Unreviewed, rolling out r246616.
Modified: trunk/Source/WebCore/page/EventHandler.cpp (247099 => 247100)
--- trunk/Source/WebCore/page/EventHandler.cpp 2019-07-03 19:53:30 UTC (rev 247099)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2019-07-03 19:59:02 UTC (rev 247100)
@@ -692,7 +692,8 @@
#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
