Title: [250392] trunk
Revision
250392
Author
wenson_hs...@apple.com
Date
2019-09-26 12:03:47 -0700 (Thu, 26 Sep 2019)

Log Message

[iOS 13] Tapping on a non-editable text selection should toggle callout bar visibility instead of clearing selection
https://bugs.webkit.org/show_bug.cgi?id=202254
<rdar://problem/54410263>

Reviewed by Megan Gardner.

Source/WebKit:

In iOS 13, tapping a text selection should toggle callout bar visibility (i.e. "selection commands" in UIKit).
This currently does not work for non-editable text, since the synthetic click gesture simultaneously fires
alongside the text interaction assistant's non-editable tap gesture, which dispatches a click to the page which
then clears the selection.

To remedy this and match platform behavior, we avoid recognizing clicks that occur over the text selection, but
only in the case where the bounding rect of the text selection doesn't cover a large portion of the visible
content rect of the web view. This ensures that the user doesn't get stuck in a state where it's impossible to
dismiss a very large text selection (e.g. after selecting all the content on the page).

Tests:  editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text.html
        editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text.html

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _shouldToggleSelectionCommandsAfterTapAt:]):

Check the last known selection rects (on _lastSelectionDrawingInfo) to see if the tapped point lies within at
least one of the selection rects.

(-[WKContentView gestureRecognizerShouldBegin:]):

LayoutTests:

* editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text-expected.txt: Added.
* editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text.html: Added.

Add a new layout test to verify that when tapping in a text selection that encompasses the entire page, we allow
the tap to dismiss the selection instead of toggling callout bar visibility.

* editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text-expected.txt: Added.
* editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text.html: Added.

Add another layout test to verify that when tapping inside a text selection, the callout bar is toggled, and
when tapping outside the selected text, the selection is dismissed.

* resources/ui-helper.js:
(window.UIHelper.async.waitForSelectionToAppear):
(window.UIHelper.async.waitForSelectionToDisappear):

New helper methods to wait for selection rects to appear or disappear.

(window.UIHelper):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (250391 => 250392)


--- trunk/LayoutTests/ChangeLog	2019-09-26 18:52:13 UTC (rev 250391)
+++ trunk/LayoutTests/ChangeLog	2019-09-26 19:03:47 UTC (rev 250392)
@@ -1,3 +1,31 @@
+2019-09-26  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [iOS 13] Tapping on a non-editable text selection should toggle callout bar visibility instead of clearing selection
+        https://bugs.webkit.org/show_bug.cgi?id=202254
+        <rdar://problem/54410263>
+
+        Reviewed by Megan Gardner.
+
+        * editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text-expected.txt: Added.
+        * editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text.html: Added.
+
+        Add a new layout test to verify that when tapping in a text selection that encompasses the entire page, we allow
+        the tap to dismiss the selection instead of toggling callout bar visibility.
+
+        * editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text-expected.txt: Added.
+        * editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text.html: Added.
+
+        Add another layout test to verify that when tapping inside a text selection, the callout bar is toggled, and
+        when tapping outside the selected text, the selection is dismissed.
+
+        * resources/ui-helper.js:
+        (window.UIHelper.async.waitForSelectionToAppear):
+        (window.UIHelper.async.waitForSelectionToDisappear):
+
+        New helper methods to wait for selection rects to appear or disappear.
+
+        (window.UIHelper):
+
 2019-09-26  Alexey Shvayka  <shvaikal...@gmail.com>
 
         toExponential, toFixed, and toPrecision should allow arguments up to 100

Added: trunk/LayoutTests/editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text-expected.txt (0 => 250392)


--- trunk/LayoutTests/editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text-expected.txt	2019-09-26 19:03:47 UTC (rev 250392)
@@ -0,0 +1,11 @@
+This test verifies that tapping selected non-editable text clears the text selection in the case where the selected text covers the vast majority of visible content. To manually test, tap the button to select the text above, and then tap inside the selection to clear it.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Observed selection.
+PASS Dismissed selection.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text.html (0 => 250392)


--- trunk/LayoutTests/editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text.html	2019-09-26 19:03:47 UTC (rev 250392)
@@ -0,0 +1,54 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<script src=""
+<script src=""
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+<meta charset="utf8">
+<style>
+body, html {
+    width: 100%;
+    height: 100%;
+    margin: 0;
+}
+
+#text {
+    font-size: 54px;
+    margin-top: 0;
+}
+</style>
+<script>
+jsTestIsAsync = true;
+
+addEventListener("load", async () => {
+    description("This test verifies that tapping selected non-editable text clears the text selection in the case where the selected text covers the vast majority of visible content. To manually test, tap the button to select the text above, and then tap inside the selection to clear it.");
+
+    const text = document.getElementById("text");
+    const button = document.querySelector("button");
+
+    button.addEventListener("mousedown", event => {
+        getSelection().selectAllChildren(text);
+        event.preventDefault();
+        button.remove();
+    });
+
+    await UIHelper.activateElement(button);
+    await UIHelper.waitForSelectionToAppear();
+    testPassed("Observed selection.");
+
+    await UIHelper.activateAt(150, 250);
+    await UIHelper.waitForSelectionToDisappear();
+    testPassed("Dismissed selection.");
+
+    text.remove();
+    finishJSTest();
+});
+</script>
+</head>
+<body>
+<button>Click to select text</button>
+<p id="text">Here’s to the crazy ones, the misfits, the rebels, the trouble makers, the round pegs in the square holes, the ones who see things differently. There not fond of rules, and they have no respect for the status quo, you can quote then, disagree with them, glorify or vilify them, about the only thing you can’t do is ignore them.  Because they change things. They push the human race forward. And while some may see them as the crazy ones, we see genius. Because the people who are crazy enough to think they can change the world are the ones who do.</p>
+<p id="description"></p>
+<p id="console"></p>
+</body>
+</html>

Added: trunk/LayoutTests/editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text-expected.txt (0 => 250392)


--- trunk/LayoutTests/editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text-expected.txt	2019-09-26 19:03:47 UTC (rev 250392)
@@ -0,0 +1,13 @@
+This test verifies that tapping selected non-editable text toggles callout bar visibility. To manually test, tap the button to select the text above, and then tap inside the selection to show the callout bar; tap inside the selection again to dismiss the callout bar, and finally tap outside of the selected text to clear the selection.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Observed selection.
+PASS Showed callout bar after first tap in selection.
+PASS Dismissed callout bar after second tap in selection.
+PASS Dismissed selection after tap outside of selection.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text.html (0 => 250392)


--- trunk/LayoutTests/editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text.html	2019-09-26 19:03:47 UTC (rev 250392)
@@ -0,0 +1,60 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<script src=""
+<script src=""
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+<meta charset="utf8">
+<style>
+body, html {
+    width: 100%;
+    height: 100%;
+    margin: 0;
+}
+</style>
+<script>
+jsTestIsAsync = true;
+
+addEventListener("load", async () => {
+    description("This test verifies that tapping selected non-editable text toggles callout bar visibility. To manually test, tap the button to select the text above, and then tap inside the selection to show the callout bar; tap inside the selection again to dismiss the callout bar, and finally tap outside of the selected text to clear the selection.");
+
+    const text = document.getElementById("text");
+    const button = document.querySelector("button");
+    const target = document.getElementById("target");
+
+    button.addEventListener("mousedown", event => {
+        getSelection().selectAllChildren(text);
+        event.preventDefault();
+        button.remove();
+    });
+
+    await UIHelper.activateElement(button);
+    await UIHelper.waitForSelectionToAppear();
+    testPassed("Observed selection.");
+
+    await UIHelper.activateElement(text);
+    await UIHelper.waitForMenuToShow();
+    testPassed("Showed callout bar after first tap in selection.");
+
+    await UIHelper.activateElement(text);
+    await UIHelper.waitForMenuToHide();
+    testPassed("Dismissed callout bar after second tap in selection.");
+
+    await UIHelper.activateElement(target);
+    await UIHelper.waitForSelectionToDisappear();
+    testPassed("Dismissed selection after tap outside of selection.");
+
+    text.remove();
+    target.remove();
+    finishJSTest();
+});
+</script>
+</head>
+<body>
+<button>Click to select text</button>
+<p id="text">Here’s to the crazy ones, the misfits, the rebels, the trouble makers, the round pegs in the square holes, the ones who see things differently. There not fond of rules, and they have no respect for the status quo, you can quote then, disagree with them, glorify or vilify them, about the only thing you can’t do is ignore them.  Because they change things. They push the human race forward. And while some may see them as the crazy ones, we see genius. Because the people who are crazy enough to think they can change the world are the ones who do.</p>
+<div id="target">Then click here</div>
+<p id="description"></p>
+<p id="console"></p>
+</body>
+</html>

Modified: trunk/LayoutTests/resources/ui-helper.js (250391 => 250392)


--- trunk/LayoutTests/resources/ui-helper.js	2019-09-26 18:52:13 UTC (rev 250391)
+++ trunk/LayoutTests/resources/ui-helper.js	2019-09-26 19:03:47 UTC (rev 250392)
@@ -1018,4 +1018,18 @@
         const uiScript = `uiController.doAfterDoubleTapDelay(() => uiController.uiScriptComplete(""))`;
         return new Promise(resolve => testRunner.runUIScript(uiScript, resolve));
     }
+
+    static async waitForSelectionToAppear() {
+        while (true) {
+            if ((await this.getUISelectionViewRects()).length > 0)
+                break;
+        }
+    }
+
+    static async waitForSelectionToDisappear() {
+        while (true) {
+            if (!(await this.getUISelectionViewRects()).length)
+                break;
+        }
+    }
 }

Modified: trunk/Source/WebKit/ChangeLog (250391 => 250392)


--- trunk/Source/WebKit/ChangeLog	2019-09-26 18:52:13 UTC (rev 250391)
+++ trunk/Source/WebKit/ChangeLog	2019-09-26 19:03:47 UTC (rev 250392)
@@ -1,3 +1,32 @@
+2019-09-26  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [iOS 13] Tapping on a non-editable text selection should toggle callout bar visibility instead of clearing selection
+        https://bugs.webkit.org/show_bug.cgi?id=202254
+        <rdar://problem/54410263>
+
+        Reviewed by Megan Gardner.
+
+        In iOS 13, tapping a text selection should toggle callout bar visibility (i.e. "selection commands" in UIKit).
+        This currently does not work for non-editable text, since the synthetic click gesture simultaneously fires
+        alongside the text interaction assistant's non-editable tap gesture, which dispatches a click to the page which
+        then clears the selection.
+
+        To remedy this and match platform behavior, we avoid recognizing clicks that occur over the text selection, but
+        only in the case where the bounding rect of the text selection doesn't cover a large portion of the visible
+        content rect of the web view. This ensures that the user doesn't get stuck in a state where it's impossible to
+        dismiss a very large text selection (e.g. after selecting all the content on the page).
+
+        Tests:  editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text.html
+                editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text.html
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _shouldToggleSelectionCommandsAfterTapAt:]):
+
+        Check the last known selection rects (on _lastSelectionDrawingInfo) to see if the tapped point lies within at
+        least one of the selection rects.
+
+        (-[WKContentView gestureRecognizerShouldBegin:]):
+
 2019-09-26  Patrick Griffis  <pgrif...@igalia.com>
 
         [GTK] Fix logic of dark theme detection

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (250391 => 250392)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2019-09-26 18:52:13 UTC (rev 250391)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2019-09-26 19:03:47 UTC (rev 250392)
@@ -2165,6 +2165,33 @@
     return textSelectionRects;
 }
 
+- (BOOL)_shouldToggleSelectionCommandsAfterTapAt:(CGPoint)point
+{
+    if (_lastSelectionDrawingInfo.selectionRects.isEmpty())
+        return NO;
+
+    WebCore::FloatRect selectionBoundingRect;
+    BOOL pointIsInSelectionRect = NO;
+    for (auto& rectInfo : _lastSelectionDrawingInfo.selectionRects) {
+        auto rect = rectInfo.rect();
+        if (rect.isEmpty())
+            continue;
+
+        pointIsInSelectionRect |= rect.contains(WebCore::roundedIntPoint(point));
+        selectionBoundingRect.unite(rect);
+    }
+
+    WebCore::FloatRect unobscuredContentRect = self.unobscuredContentRect;
+    selectionBoundingRect.intersect(unobscuredContentRect);
+
+    float unobscuredArea = unobscuredContentRect.area();
+    float ratioForConsideringSelectionRectToCoverVastMajorityOfContent = 0.75;
+    if (!unobscuredArea || selectionBoundingRect.area() / unobscuredArea > ratioForConsideringSelectionRectToCoverVastMajorityOfContent)
+        return NO;
+
+    return pointIsInSelectionRect;
+}
+
 - (BOOL)gestureRecognizerShouldBegin:(UIGestureRecognizer *)gestureRecognizer
 {
     CGPoint point = [gestureRecognizer locationInView:self];
@@ -2187,8 +2214,11 @@
         return NO;
     };
 
-    if (gestureRecognizer == _singleTapGestureRecognizer)
+    if (gestureRecognizer == _singleTapGestureRecognizer) {
+        if ([self _shouldToggleSelectionCommandsAfterTapAt:point])
+            return NO;
         return !isInterruptingDecelerationForScrollViewOrAncestor([_singleTapGestureRecognizer lastTouchedScrollView]);
+    }
 
     if (gestureRecognizer == _doubleTapGestureRecognizerForDoubleClick) {
         // Do not start the double-tap-for-double-click gesture recognizer unless we've got a dblclick event handler on the node at the tap location.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to