Attachment 349268 Details for Bug 189454 – Patch

[patch] Patch

bug-189454-20180908180451.patch (text/plain), 8.24 KB, created by Wenson Hsieh on 2018-09-08 18:04:51 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2018-09-08 18:04:51 PDT

Size: 8.24 KB

patch

obsolete

>Subversion Revision: 235819
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 77cce9454dfedeb8ffede17a17161e0f1934a6b9..ebc6f2d4cdeacde4b13e01ee28f3390997941c3e 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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-07  Brent Fulgham  <bfulgham@apple.com>
> 
>         Allow WebContent access to AVCSupported IOKit property in sandbox
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index f74c9a597433857a385fef64d0b31d44f92a870e..d0d3b20d241f57c62911c3d2daf3444e3247a51c 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -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;
>     
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 7819ddd72747569b0a48e919b5dc70373f579979..b41c5e383a241bf4d1246bda0492db24567b6431 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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-07  Youenn Fablet  <youenn@apple.com>
> 
>         Tests checking document GC in case of ActiveDOMObjects are flaky
>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
>index 0000000000000000000000000000000000000000..73a23f4e20d38f0c4bd4a1caea0582d7ff4d2efd
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..9bf9677e4a3647bac01216b01167a9a2fb92203e
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/selection-handle-clamping-in-iframe.html
>@@ -0,0 +1,76 @@
>+<!DOCTYPE html> 
>+<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

Actions: View | Formatted Diff | Diff

Attachments on bug 189454: 349268