REGRESSION (r235153): [iOS] Can't move selection start grabber when selecting text...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 9 Sep 2018 03:25:05 +0000 (03:25 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 9 Sep 2018 03:25:05 +0000 (03:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189454
<rdar://problem/44265956>

Reviewed by Darin Adler.

Source/WebKit:

rangeForPointInRootViewCoordinates is responsible for taking a user gesture location representing the location
of the selection start or end handle (given in root view coordinates) and computing a Range representing an
updated selection. r235153 introduced a mechanism here to clamp the y offset of this user gesture location to
a max or min value determined by computing the bounds of the other selection handle, which more closely matches
platform behavior elsewhere in iOS.

However, this clamping logic would cause the user gesture location in root view coordinates to incorrectly clamp
in cases where the user selects text within an iframe that is offset from the top of the main document, since it
compares content coordinates (i.e. the caret bounds) against root view coordinates (i.e. the gesture location).
This makes it impossible to use selection handles to select text in some iframes.

We fix this by first converting the gesture location to document coordinates, and then clamping.

Test: editing/selection/ios/selection-handle-clamping-in-iframe.html

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::rangeForPointInRootViewCoordinates):

Also reuse `selectionStart` and `selectionEnd` when computing absolute caret bounds, instead of creating new
VisiblePositions.

LayoutTests:

Adds a test that selects a word inside an iframe, moves the selection start handle down past the selection end,
and then moves the selection end handle up above the selection start. The test verifies that the entire word
remains selected.

* editing/selection/ios/selection-handle-clamping-in-iframe-expected.txt: Added.
* editing/selection/ios/selection-handle-clamping-in-iframe.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe.html [new file with mode: 0644]
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

index 9caf072..1247196 100644 (file)
@@ -1,3 +1,18 @@
+2018-09-08  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        REGRESSION (r235153): [iOS] Can't move selection start grabber when selecting text in a subframe
+        https://bugs.webkit.org/show_bug.cgi?id=189454
+        <rdar://problem/44265956>
+
+        Reviewed by Darin Adler.
+
+        Adds a test that selects a word inside an iframe, moves the selection start handle down past the selection end,
+        and then moves the selection end handle up above the selection start. The test verifies that the entire word
+        remains selected.
+
+        * editing/selection/ios/selection-handle-clamping-in-iframe-expected.txt: Added.
+        * editing/selection/ios/selection-handle-clamping-in-iframe.html: Added.
+
 2018-09-08  Andy Estes  <aestes@apple.com>
 
         [Apple Pay] Dispatch a paymentmethodchange event when the payment method changes
diff --git a/LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe-expected.txt b/LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe-expected.txt
new file mode 100644 (file)
index 0000000..73a23f4
--- /dev/null
@@ -0,0 +1,7 @@
+The final selection is: "iOS"
+
+Verifies that the selection remains the same when dragging the start selection handles below the end selection handle and vice versa.
+
+To manually run the test, select "iOS" in the iframe above, drag the start selection handle down, and then drag the end selection handle up.
+
+The text "iOS" should remain selected.
diff --git a/LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe.html b/LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe.html
new file mode 100644 (file)
index 0000000..9bf9677
--- /dev/null
@@ -0,0 +1,76 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<script src="../../../resources/ui-helper.js"></script>
+<script src="../../../resources/basic-gestures.js"></script>
+<meta name=viewport content="width=device-width">
+<style>
+body, html {
+    width: 100%;
+    height: 100%;
+    margin: 0;
+}
+
+pre {
+    width: 300px;
+    height: 160px;
+    overflow: scroll;
+    border: 1px green solid;
+}
+
+pre > #result {
+    color: green;
+}
+
+#target {
+    width: 300px;
+    height: 160px;
+}
+</style>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+function midPointOfRect(rect) {
+    return [rect.left + (rect.width / 2), rect.top + (rect.height / 2)];
+}
+
+async function runTest() {
+    // Wait for both the main frame and the subframe to finish loading.
+    loadCount = window.loadCount ? loadCount : 0;
+    if (++loadCount != 2)
+        return;
+
+    await longPressAtPoint(160, 240);
+    let startRect = { };
+    let endRect = { };
+    while (!startRect.width || !startRect.height || !endRect.width || !endRect.height) {
+        startRect = await UIHelper.getSelectionStartGrabberViewRect();
+        endRect = await UIHelper.getSelectionEndGrabberViewRect();
+    }
+
+    const [startX, startY] = midPointOfRect(startRect);
+    await touchAndDragFromPointToPoint(startX, startY, startX, startY + 100);
+    await liftUpAtPoint(startX, startY + 100);
+
+    const [endX, endY] = midPointOfRect(endRect);
+    await touchAndDragFromPointToPoint(endX, endY, endX, endY - 100);
+    await liftUpAtPoint(endX, endY - 100);
+
+    result.textContent = target.contentWindow.getSelection().toString();
+    testRunner.notifyDone();
+}
+</script>
+</head>
+
+<body onload="runTest()">
+<pre>The final selection is: "<span id="result"></span>"</pre>
+<iframe onload="runTest()" src="data:text/html,
+    <span id='text' style='font-size: 140px;'>iOS</span>" id="target"></iframe>
+<p>Verifies that the selection remains the same when dragging the start selection handles below the end selection handle and vice versa.</p>
+<p>To manually run the test, select "iOS" in the iframe above, drag the start selection handle down, and then drag the end selection handle up.</p>
+<p>The text "iOS" should remain selected.</p>
+</body>
+</html>
\ No newline at end of file
index 5e9e188..d9f188b 100644 (file)
@@ -1,3 +1,32 @@
+2018-09-08  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        REGRESSION (r235153): [iOS] Can't move selection start grabber when selecting text in a subframe
+        https://bugs.webkit.org/show_bug.cgi?id=189454
+        <rdar://problem/44265956>
+
+        Reviewed by Darin Adler.
+
+        rangeForPointInRootViewCoordinates is responsible for taking a user gesture location representing the location
+        of the selection start or end handle (given in root view coordinates) and computing a Range representing an
+        updated selection. r235153 introduced a mechanism here to clamp the y offset of this user gesture location to
+        a max or min value determined by computing the bounds of the other selection handle, which more closely matches
+        platform behavior elsewhere in iOS.
+
+        However, this clamping logic would cause the user gesture location in root view coordinates to incorrectly clamp
+        in cases where the user selects text within an iframe that is offset from the top of the main document, since it
+        compares content coordinates (i.e. the caret bounds) against root view coordinates (i.e. the gesture location).
+        This makes it impossible to use selection handles to select text in some iframes.
+
+        We fix this by first converting the gesture location to document coordinates, and then clamping.
+
+        Test: editing/selection/ios/selection-handle-clamping-in-iframe.html
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::rangeForPointInRootViewCoordinates):
+
+        Also reuse `selectionStart` and `selectionEnd` when computing absolute caret bounds, instead of creating new
+        VisiblePositions.
+
 2018-09-08  Tim Horton  <timothy_horton@apple.com>
 
         Unify most of the WebKit Objective-C API sources
index f74c9a5..d0d3b20 100644 (file)
@@ -1221,22 +1221,19 @@ static RefPtr<Range> rangeForPointInRootViewCoordinates(Frame& frame, const IntP
     VisibleSelection existingSelection = frame.selection().selection();
     VisiblePosition selectionStart = existingSelection.visibleStart();
     VisiblePosition selectionEnd = existingSelection.visibleEnd();
-    
-    IntPoint adjustedPoint = pointInRootViewCoordinates;
-    
+
+    auto pointInDocument = frame.view()->rootViewToContents(pointInRootViewCoordinates);
+
     if (baseIsStart) {
-        IntRect caret = existingSelection.visibleStart().absoluteCaretBounds();
-        int startY = caret.center().y();
-        if (adjustedPoint.y() < startY)
-            adjustedPoint.setY(startY);
+        int startY = selectionStart.absoluteCaretBounds().center().y();
+        if (pointInDocument.y() < startY)
+            pointInDocument.setY(startY);
     } else {
-        IntRect caret = existingSelection.visibleEnd().absoluteCaretBounds();
-        int endY = caret.center().y();
-        if (adjustedPoint.y() > endY)
-            adjustedPoint.setY(endY);
+        int endY = selectionEnd.absoluteCaretBounds().center().y();
+        if (pointInDocument.y() > endY)
+            pointInDocument.setY(endY);
     }
     
-    IntPoint pointInDocument = frame.view()->rootViewToContents(adjustedPoint);
     VisiblePosition result;
     RefPtr<Range> range;