Attachment 335937 Details for Bug 183658 – Patch

[patch] Patch

bug-183658-20180316161044.patch (text/plain), 6.06 KB, created by Frédéric Wang Nélar on 2018-03-16 08:10:46 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Frédéric Wang Nélar

Created: 2018-03-16 08:10:46 PDT

Size: 6.06 KB

patch

obsolete

>Subversion Revision: 229622
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index f1a84d5590b03474d3e89b9d9bd48f2d0ce91b8e..333f91e41ae5ddd20f3f35c3e29e44cb5b91c5ea 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-03-16  Frederic Wang  <fwang@igalia.com>
>+
>+        [iOS] Text highlighted by the Find UI overlaps with NBC news header on google.com
>+        https://bugs.webkit.org/show_bug.cgi?id=183658
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        On Mobile, many sites have overlay headers or footers that may overlap with the highlighted
>+        text inside a scrollable overflow node. To workaround that issue, this commit changes the
>+        behavior so that the text is revealed at the center of the overflow node.
>+
>+        * WebProcess/WebPage/ios/FindControllerIOS.mm:
>+        (WebKit::FindController::didFindString): Use alignCenterAlways instead of alignToEdgeIfNeeded.
>+
> 2018-03-15  Zan Dobersek  <zdobersek@igalia.com>
> 
>         [TexMap] Remove TextureMapperLayer::texture()
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm
>index f493d6ce092911e69b22ae90b8f7e60ca00a3a20..f2c19844759566a4866c6d4419a8c4b5736db398 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm
>@@ -160,7 +160,9 @@ void FindController::didFindString()
> 
>     // Scrolling the main frame is handled by the SmartMagnificationController class but we still
>     // need to consider overflow nodes and subframes here.
>-    frame.selection().revealSelection(SelectionRevealMode::RevealUpToMainFrame, ScrollAlignment::alignToEdgeIfNeeded, WebCore::DoNotRevealExtent);
>+    // On Mobile, many sites have overlay headers or footers that may overlap with the highlighted
>+    // text, so we reveal the text at the center of the viewport. See https://webkit.org/b/183658
>+    frame.selection().revealSelection(SelectionRevealMode::RevealUpToMainFrame, ScrollAlignment::alignCenterAlways, WebCore::DoNotRevealExtent);
> }
> 
> void FindController::didFailToFindString()
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index a03aa5b8386e25a0d7c01901c2684153e7083d36..c33b458f9e865f5c0cae8f8f00f5fbb19b99b958 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-03-16  Frederic Wang  <fwang@igalia.com>
>+
>+        [iOS] Text highlighted by the Find UI overlaps with NBC news header on google.com
>+        https://bugs.webkit.org/show_bug.cgi?id=183658
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Improve the test added in r224284, so that we test when a result is revealed at
>+        the center of the overflow node.
>+
>+        * platform/ios/fast/scrolling/find-text-in-overflow-node.html: Put some content after
>+        'match 4' so that the overflow node can scroll and make the text appear at the center.
>+        Perform more accurate testing of the scrollTop of the overflow node.
>+        * platform/ios/fast/scrolling/find-text-in-overflow-node-expected.txt: Update expectation.
>+
> 2018-03-14  Devin Rousso  <webkit@devinrousso.com>
> 
>         Web Inspector: Canvas: a recording initiated by the user should be shown immediately on completion
>diff --git a/LayoutTests/platform/ios/fast/scrolling/find-text-in-overflow-node-expected.txt b/LayoutTests/platform/ios/fast/scrolling/find-text-in-overflow-node-expected.txt
>index bebb71c97cdba7605421136d3891c1b030b3e688..5a6c19adee1cd7c772a03a4e74302b726ab4f3dd 100644
>--- a/LayoutTests/platform/ios/fast/scrolling/find-text-in-overflow-node-expected.txt
>+++ b/LayoutTests/platform/ios/fast/scrolling/find-text-in-overflow-node-expected.txt
>@@ -1,4 +1,4 @@
>-Use iOS Find UI to search for the text "match". The following overflow node should scroll to show the fourth result.
>+Use iOS Find UI to search for the text "match". The following overflow node should scroll to show the fourth result at its center.
> 
> match 2
> 
>@@ -6,7 +6,6 @@ match 3
> 
> match 4
> 
>-
> PASS Initial position of overflow node 
> PASS Position of overflow node after the three first results 
> PASS Position of overflow node after the fourth result 
>diff --git a/LayoutTests/platform/ios/fast/scrolling/find-text-in-overflow-node.html b/LayoutTests/platform/ios/fast/scrolling/find-text-in-overflow-node.html
>index 5e8af35e7bb4bec4bd45873ab678d55e140e85ac..9365f1789fa2698bf88b2918b45e21e8e778b17c 100644
>--- a/LayoutTests/platform/ios/fast/scrolling/find-text-in-overflow-node.html
>+++ b/LayoutTests/platform/ios/fast/scrolling/find-text-in-overflow-node.html
>@@ -7,6 +7,9 @@
>     <script src="../../../../resources/testharnessreport.js"></script>
>     <script type="text/javascript">
>       setup({ "explicit_done": true });
>+      function height(id) {
>+        return document.getElementById(id).getBoundingClientRect().height;
>+      }
>       function run() {
>         if (!window.testRunner || !testRunner.runUIScript)
>           return;
>@@ -29,6 +32,8 @@
>             uiController.uiScriptComplete("Done");
>           `, afterMatch4.step_func_done(function() {
>               assert_greater_than(node.scrollTop, 0);
>+              var expectedScrollTop = height("divBefore") + height("match4") / 2 - height("scrollable") / 2;
>+              assert_approx_equals(node.scrollTop, expectedScrollTop, 2);
>           }));
>         }));
> 
>@@ -47,12 +52,13 @@
>     </style>
>   </head>
>   <body onload="run()">
>-    <p>Use iOS Find UI to search for the text "match". The following overflow node should scroll to show the fourth result.</p>
>+    <p>Use iOS Find UI to search for the text "match". The following overflow node should scroll to show the fourth result at its center.</p>
>     <p>match 2</p>
>     <p>match 3</p>
>     <div id="scrollable">
>-      <div style="height: 200px; background: linear-gradient(135deg, blue, cyan);"></div>
>-      <p>match 4</p>
>+      <div id="divBefore" style="height: 300px; background: linear-gradient(135deg, blue, cyan);"></div>
>+      <div id="match4">match 4</div>
>+      <div style="height: 300px; background: linear-gradient(135deg, blue, cyan);"></div>
>     </div>
>   </body>
> </html>

Flags:

thorton: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 183658: 335845 | 335937