[WK2] Cannot select text on nytimes.com when the selection granularity is WKSelection...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Apr 2017 23:12:30 +0000 (23:12 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Apr 2017 23:12:30 +0000 (23:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170968
<rdar://problem/31692560>

Reviewed by Dan Bernstein.

Source/WebKit2:

Makes a small adjustment to textInteractionGesture:shouldBeginAtPoint:. When determining whether to allow the
text interaction assistant to recognize at a given point, instead of depending on whether or not the hit node is
the same as the assisted node, only enforce this restriction when editing an assisted node. Otherwise, default
to allowing the selection gesture.

Note that character granularity selection was working on most pages before, due to the fact that
nodeAtPositionIsAssistedNode was true in many cases when there is no assisted node at all. This is because, in
WebPage.mm, we compute the hit-tested node responding to click events to be null, and then set
nodeAtPositionIsAssistedNode to be equal to hitNode == m_assistedNode, which ends up being true because both of
these values are null.

This allowed text selection to work in the simple case when selection granularity character is used, but not
when the node containing the selected point actually does respond to click events, since the comparison returns
false.

New layout test: LayoutTests/editing/selection/character-granularity-select-text-with-click-handler.html.

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

LayoutTests:

Adds a new layout test checking that text within a node with a click handler can be selected when using
character selection granularity.

* editing/selection/character-granularity-select-text-with-click-handler-expected.txt: Added.
* editing/selection/character-granularity-select-text-with-click-handler.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/editing/selection/character-granularity-select-text-with-click-handler-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/character-granularity-select-text-with-click-handler.html [new file with mode: 0644]
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm

index 82f9409..12c7771 100644 (file)
@@ -1,3 +1,17 @@
+2017-04-18  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [WK2] Cannot select text on nytimes.com when the selection granularity is WKSelectionGranularityCharacter
+        https://bugs.webkit.org/show_bug.cgi?id=170968
+        <rdar://problem/31692560>
+
+        Reviewed by Dan Bernstein.
+
+        Adds a new layout test checking that text within a node with a click handler can be selected when using
+        character selection granularity.
+
+        * editing/selection/character-granularity-select-text-with-click-handler-expected.txt: Added.
+        * editing/selection/character-granularity-select-text-with-click-handler.html: Added.
+
 2017-04-18  Joseph Pecoraro  <pecoraro@apple.com>
 
         [mac-wk1 Debug] LayoutTest http/tests/inspector/network/resource-sizes-network.html is a flaky failure
index b4aa128..93762e3 100644 (file)
@@ -80,6 +80,7 @@ fast/attachment/attachment-subtitle-resize.html
 # This test only makes sense on iOS
 fast/attachment/attachment-wrapping-action.html
 editing/selection/character-granularity-selected-range-after-dismissing-selection.html [ Skip ]
+editing/selection/character-granularity-select-text-with-click-handler.html [ Skip ]
 editing/selection/caret-after-tap-in-editable-selection.html [ Skip ]
 
 # Only iOS has selection UI drawn by UIKit
diff --git a/LayoutTests/editing/selection/character-granularity-select-text-with-click-handler-expected.txt b/LayoutTests/editing/selection/character-granularity-select-text-with-click-handler-expected.txt
new file mode 100644 (file)
index 0000000..3aecd63
--- /dev/null
@@ -0,0 +1,2 @@
+WEBKIT
+PASS: Successfully selected text.
diff --git a/LayoutTests/editing/selection/character-granularity-select-text-with-click-handler.html b/LayoutTests/editing/selection/character-granularity-select-text-with-click-handler.html
new file mode 100644 (file)
index 0000000..96fb312
--- /dev/null
@@ -0,0 +1,29 @@
+<!-- webkit-test-runner [ useFlexibleViewport=true, useCharacterSelectionGranularity=true ] -->
+<meta name=viewport content="width=device-width, initial-scale=1">
+<div id="target" style="font-size: 75px;">WEBKIT</div>
+<script>
+    function selectTextScript()
+    {
+        return `
+        (() => {
+            uiController.longPressAtPoint(100, 50, () => {
+                uiController.uiScriptComplete();
+            });
+        })();`
+    }
+
+    if (!window.testRunner)
+        document.body.appendChild(document.createTextNode("To test manually, enable character selection granularity and select WEB by long pressing."));
+
+    target.addEventListener("click", () => { });
+    document.addEventListener("selectionchange", () => {
+        if (window.testRunner && getSelection().toString() === "WEBKIT") {
+            document.body.appendChild(document.createTextNode("PASS: Successfully selected text."));
+            testRunner.notifyDone();
+        }
+    });
+
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+    testRunner.runUIScript(selectTextScript(), () => { });
+</script>
index 0172c58..8b2e08f 100644 (file)
@@ -1,3 +1,31 @@
+2017-04-18  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [WK2] Cannot select text on nytimes.com when the selection granularity is WKSelectionGranularityCharacter
+        https://bugs.webkit.org/show_bug.cgi?id=170968
+        <rdar://problem/31692560>
+
+        Reviewed by Dan Bernstein.
+
+        Makes a small adjustment to textInteractionGesture:shouldBeginAtPoint:. When determining whether to allow the
+        text interaction assistant to recognize at a given point, instead of depending on whether or not the hit node is
+        the same as the assisted node, only enforce this restriction when editing an assisted node. Otherwise, default
+        to allowing the selection gesture.
+
+        Note that character granularity selection was working on most pages before, due to the fact that
+        nodeAtPositionIsAssistedNode was true in many cases when there is no assisted node at all. This is because, in
+        WebPage.mm, we compute the hit-tested node responding to click events to be null, and then set
+        nodeAtPositionIsAssistedNode to be equal to hitNode == m_assistedNode, which ends up being true because both of
+        these values are null.
+
+        This allowed text selection to work in the simple case when selection granularity character is used, but not
+        when the node containing the selected point actually does respond to click events, since the comparison returns
+        false.
+
+        New layout test: LayoutTests/editing/selection/character-granularity-select-text-with-click-handler.html.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView textInteractionGesture:shouldBeginAtPoint:]):
+
 2017-04-18  Keith Rollin  <krollin@apple.com>
 
         Add additional information when logging URL disposition in WebLoaderStrategy::scheduleLoad
index 823f082..3257b15 100644 (file)
@@ -1529,7 +1529,13 @@ static inline bool isSamePair(UIGestureRecognizer *a, UIGestureRecognizer *b, UI
     }
 #endif
 
-    return _positionInformation.nodeAtPositionIsAssistedNode;
+    // If we're currently editing an assisted node, only allow the selection to move within that assisted node.
+    if (self.isAssistingNode)
+        return _positionInformation.nodeAtPositionIsAssistedNode;
+
+    // Otherwise, if we're using a text interaction assistant outside of editing purposes (e.g. the selection mode
+    // is character granularity) then allow text selection.
+    return YES;
 }
 
 - (NSArray *)webSelectionRectsForSelectionRects:(const Vector<WebCore::SelectionRect>&)selectionRects