Changeset 229801 in webkit


Ignore:
Timestamp:
Mar 21, 2018 2:46:12 AM (6 years ago)
Author:
fred.wang@free.fr
Message:

[iOS] Text highlighted by the Find UI overlaps with NBC news header on google.com
​https://bugs.webkit.org/show_bug.cgi?id=183658

Patch by Frederic Wang <​fwang@igalia.com> on 2018-03-21
Reviewed by Tim Horton.

Source/WebKit:

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.

LayoutTests:

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.
Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r229782 r229801  
     12018-03-21  Frederic Wang  <fwang@igalia.com>
     2
     3        [iOS] Text highlighted by the Find UI overlaps with NBC news header on google.com
     4        https://bugs.webkit.org/show_bug.cgi?id=183658
     5
     6        Reviewed by Tim Horton.
     7
     8        Improve the test added in r224284, so that we test when a result is revealed at
     9        the center of the overflow node.
     10
     11        * platform/ios/fast/scrolling/find-text-in-overflow-node.html: Put some content after
     12        'match 4' so that the overflow node can scroll and make the text appear at the center.
     13        Perform more accurate testing of the scrollTop of the overflow node.
     14        * platform/ios/fast/scrolling/find-text-in-overflow-node-expected.txt: Update expectation.
     15
    1162018-03-20  Zalan Bujtas  <zalan@apple.com>
    217

  • trunk/LayoutTests/platform/ios/fast/scrolling/find-text-in-overflow-node-expected.txt

    r224284 r229801  
    1 Use iOS Find UI to search for the text "match". The following overflow node should scroll to show the fourth result.
     1Use iOS Find UI to search for the text "match". The following overflow node should scroll to show the fourth result at its center.
    22
    33match 2
    … …  
    77match 4
    88
    9 
    109PASS Initial position of overflow node
    1110PASS Position of overflow node after the three first results

  • trunk/LayoutTests/platform/ios/fast/scrolling/find-text-in-overflow-node.html

    r224284 r229801  
    88    <script type="text/javascript">
    99      setup({ "explicit_done": true });
     10      function height(id) {
     11        return document.getElementById(id).getBoundingClientRect().height;
     12      }
    1013      function run() {
    1114        if (!window.testRunner || !testRunner.runUIScript)
    … …  
    3033          `, afterMatch4.step_func_done(function() {
    3134              assert_greater_than(node.scrollTop, 0);
     35              var expectedScrollTop = height("divBefore") + height("match4") / 2 - height("scrollable") / 2;
     36              assert_approx_equals(node.scrollTop, expectedScrollTop, 2);
    3237          }));
    3338        }));
    … …  
    4853  </head>
    4954  <body onload="run()">
    50     <p>Use iOS Find UI to search for the text "match". The following overflow node should scroll to show the fourth result.</p>
     55    <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>
    5156    <p>match 2</p>
    5257    <p>match 3</p>
    5358    <div id="scrollable">
    54       <div style="height: 200px; background: linear-gradient(135deg, blue, cyan);"></div>
    55       <p>match 4</p>
     59      <div id="divBefore" style="height: 300px; background: linear-gradient(135deg, blue, cyan);"></div>
     60      <div id="match4">match 4</div>
     61      <div style="height: 300px; background: linear-gradient(135deg, blue, cyan);"></div>
    5662    </div>
    5763  </body>

  • trunk/Source/WebKit/ChangeLog

    r229799 r229801  
     12018-03-21  Frederic Wang  <fwang@igalia.com>
     2
     3        [iOS] Text highlighted by the Find UI overlaps with NBC news header on google.com
     4        https://bugs.webkit.org/show_bug.cgi?id=183658
     5
     6        Reviewed by Tim Horton.
     7
     8        On Mobile, many sites have overlay headers or footers that may overlap with the highlighted
     9        text inside a scrollable overflow node. To workaround that issue, this commit changes the
     10        behavior so that the text is revealed at the center of the overflow node.
     11
     12        * WebProcess/WebPage/ios/FindControllerIOS.mm:
     13        (WebKit::FindController::didFindString): Use alignCenterAlways instead of alignToEdgeIfNeeded.
     14
    1152018-03-21  Carlos Garcia Campos  <cgarcia@igalia.com>
    216

  • trunk/Source/WebKit/WebProcess/WebPage/ios/FindControllerIOS.mm

    r224284 r229801  
    161161    // Scrolling the main frame is handled by the SmartMagnificationController class but we still
    162162    // need to consider overflow nodes and subframes here.
    163     frame.selection().revealSelection(SelectionRevealMode::RevealUpToMainFrame, ScrollAlignment::alignToEdgeIfNeeded, WebCore::DoNotRevealExtent);
     163    // On Mobile, many sites have overlay headers or footers that may overlap with the highlighted
     164    // text, so we reveal the text at the center of the viewport. See https://webkit.org/b/183658
     165    frame.selection().revealSelection(SelectionRevealMode::RevealUpToMainFrame, ScrollAlignment::alignCenterAlways, WebCore::DoNotRevealExtent);
    164166}
    165167
Note: See TracChangeset for help on using the changeset viewer.