Title: [235834] trunk
Revision
235834
Author
wenson_hs...@apple.com
Date
2018-09-08 20:25:05 -0700 (Sat, 08 Sep 2018)

Log Message

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.

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (235833 => 235834)


--- trunk/LayoutTests/ChangeLog	2018-09-09 02:03:52 UTC (rev 235833)
+++ trunk/LayoutTests/ChangeLog	2018-09-09 03:25:05 UTC (rev 235834)
@@ -1,3 +1,18 @@
+2018-09-08  Wenson Hsieh  <wenson_hs...@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  <aes...@apple.com>
 
         [Apple Pay] Dispatch a paymentmethodchange event when the payment method changes

Added: trunk/LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe-expected.txt (0 => 235834)


--- trunk/LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe-expected.txt	2018-09-09 03:25:05 UTC (rev 235834)
@@ -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.

Added: trunk/LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe.html (0 => 235834)


--- trunk/LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe.html	2018-09-09 03:25:05 UTC (rev 235834)
@@ -0,0 +1,76 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<script src=""
+<script src=""
+<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=""
+    <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

Modified: trunk/Source/WebKit/ChangeLog (235833 => 235834)


--- trunk/Source/WebKit/ChangeLog	2018-09-09 02:03:52 UTC (rev 235833)
+++ trunk/Source/WebKit/ChangeLog	2018-09-09 03:25:05 UTC (rev 235834)
@@ -1,3 +1,32 @@
+2018-09-08  Wenson Hsieh  <wenson_hs...@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_hor...@apple.com>
 
         Unify most of the WebKit Objective-C API sources

Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (235833 => 235834)


--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2018-09-09 02:03:52 UTC (rev 235833)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2018-09-09 03:25:05 UTC (rev 235834)
@@ -1221,22 +1221,19 @@
     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;
     
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to