[iOS 13] Tapping on a non-editable text selection should toggle callout bar visibilit...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Sep 2019 19:03:47 +0000 (19:03 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Sep 2019 19:03:47 +0000 (19:03 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250392 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text.html [new file with mode: 0644]
LayoutTests/editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text.html [new file with mode: 0644]
LayoutTests/resources/ui-helper.js
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

index a788cf1..7a3f9f2 100644 (file)
@@ -1,3 +1,31 @@
+2019-09-26  Wenson Hsieh  <wenson_hsieh@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  <shvaikalesh@gmail.com>
 
         toExponential, toFixed, and toPrecision should allow arguments up to 100
diff --git a/LayoutTests/editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text-expected.txt b/LayoutTests/editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text-expected.txt
new file mode 100644 (file)
index 0000000..40fefbb
--- /dev/null
@@ -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
+
diff --git a/LayoutTests/editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text.html b/LayoutTests/editing/selection/ios/clear-selection-after-tap-in-large-selected-non-editable-text.html
new file mode 100644 (file)
index 0000000..0179dc8
--- /dev/null
@@ -0,0 +1,54 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<script src="../../../resources/js-test.js"></script>
+<script src="../../../resources/ui-helper.js"></script>
+<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>
diff --git a/LayoutTests/editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text-expected.txt b/LayoutTests/editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text-expected.txt
new file mode 100644 (file)
index 0000000..33a499a
--- /dev/null
@@ -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
+
diff --git a/LayoutTests/editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text.html b/LayoutTests/editing/selection/ios/toggle-callout-bar-after-tap-in-selected-non-editable-text.html
new file mode 100644 (file)
index 0000000..32a1625
--- /dev/null
@@ -0,0 +1,60 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<script src="../../../resources/js-test.js"></script>
+<script src="../../../resources/ui-helper.js"></script>
+<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>
index ad36dbb..a481de8 100644 (file)
@@ -1018,4 +1018,18 @@ window.UIHelper = class UIHelper {
         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;
+        }
+    }
 }
index 9d1fff3..c03a0f0 100644 (file)
@@ -1,3 +1,32 @@
+2019-09-26  Wenson Hsieh  <wenson_hsieh@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  <pgriffis@igalia.com>
 
         [GTK] Fix logic of dark theme detection
index f8d076a..eb827b6 100644 (file)
@@ -2165,6 +2165,33 @@ static inline bool isSamePair(UIGestureRecognizer *a, UIGestureRecognizer *b, UI
     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 @@ static inline bool isSamePair(UIGestureRecognizer *a, UIGestureRecognizer *b, UI
         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.