Changeset 228714 in webkit


Ignore:
Timestamp:
Feb 19, 2018 3:30:27 PM (6 years ago)
Author:
timothy_horton@apple.com
Message:

REGRESSION (r219342): Touch event coordinates and elementFromPoint coordinates differ
​https://bugs.webkit.org/show_bug.cgi?id=182910
<rdar://problem/37533950>

Reviewed by Simon Fraser.

Source/WebCore:

We reverted other changes to the definition of client coordinates
in r219829 due to compatibility concerns. However, we failed to revert
r219342 on trunk, leaving elementFromPoint() using coordinates relative
to the layout viewport.

Add a currently off-by-default setting to switch on layout-viewport-relative
client coordinates and guard the elementFromPoint changes behind it.
A future patch should roll r219829 back in also behind this setting, so
that everything remains consistent regardless of which coordinate space we choose.

  • dom/TreeScope.cpp:

(WebCore::absolutePointIfNotClipped):

  • page/Settings.yaml:
  • rendering/RenderLayer.cpp:

(WebCore::RenderLayer::hitTest):

LayoutTests:

  • fast/dom/elementFromPoint-scaled-scrolled.html:

Revert changes to this test made in r219342.

  • fast/dom/elementFromPoint-scaled-scrolled-layout-viewport.html:
  • fast/dom/elementFromPoint-scaled-scrolled-layout-viewport-expected.txt:

Add a test that is equivalent to elementFromPoint-scaled-scrolled.html after r219342,
which turns on the new setting. This test is disabled on iOS (like it was
in r219342) because it needs window.scrollTo.

  • platform/ios-wk2/fast/dom/elementFromPoint-relative-to-viewport-expected.txt:

This now passes.

  • platform/ios/TestExpectations:

Re-mark-failing a test that was un-marked-failing by r219342.

Location:
trunk
Files:
9 edited
2 copied

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r228703 r228714  
     12018-02-19  Tim Horton  <timothy_horton@apple.com>
     2
     3        REGRESSION (r219342): Touch event coordinates and elementFromPoint coordinates differ
     4        https://bugs.webkit.org/show_bug.cgi?id=182910
     5        <rdar://problem/37533950>
     6
     7        Reviewed by Simon Fraser.
     8
     9        * fast/dom/elementFromPoint-scaled-scrolled.html:
     10        Revert changes to this test made in r219342.
     11
     12        * fast/dom/elementFromPoint-scaled-scrolled-layout-viewport.html:
     13        * fast/dom/elementFromPoint-scaled-scrolled-layout-viewport-expected.txt:
     14        Add a test that is equivalent to elementFromPoint-scaled-scrolled.html after r219342,
     15        which turns on the new setting. This test is disabled on iOS (like it was
     16        in r219342) because it needs window.scrollTo.
     17
     18        * platform/ios-wk2/fast/dom/elementFromPoint-relative-to-viewport-expected.txt:
     19        This now passes.
     20
     21        * platform/ios/TestExpectations:
     22        Re-mark-failing a test that was un-marked-failing by r219342.
     23
    1242018-02-19  Daniel Bates  <dabates@apple.com>
    225

  • trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-expected.txt

    r227974 r228714  
    1 This test applies page scale and scrolls the page, and checks that elementFromPoint returns the correct element.
    2 
    3 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
    4 
    5 
    6 PASS successfullyParsed is true
    7 
    8 TEST COMPLETE
    9 PASS document.elementFromPoint(225, 125) is b1
    10 PASS document.elementFromPoint(525, 425) is b2
    11 PASS document.elementFromPoint(225, 125) is b1
    12 PASS document.elementFromPoint(525, 425) is b2
    13 PASS document.elementFromPoint(115, 15) is b1
    14 PASS document.elementFromPoint(415, 315) is b2
    15 PASS document.elementFromPoint(-85, 15) is null
    16 PASS document.elementFromPoint(215, 315) is b2
    17 PASS document.elementFromPoint(525, 425) is b2
    18 PASS successfullyParsed is true
    19 
    20 TEST COMPLETE
    21 
     1B1B2B3
     2This test is successful if elementFromPoint returns the correct element.
     3B3

  • trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-layout-viewport-expected.txt

    r228713 r228714  
    1515PASS document.elementFromPoint(-85, 15) is null
    1616PASS document.elementFromPoint(215, 315) is b2
    17 PASS document.elementFromPoint(525, 425) is b2
    1817PASS successfullyParsed is true
    1918

  • trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-layout-viewport.html

    r228713 r228714  
    3131    if (window.internals) {
    3232        window.internals.settings.setVisualViewportEnabled(true);
     33        window.internals.settings.setClientCoordinatesRelativeToLayoutViewport(true);
    3334        window.internals.setPageScaleFactor(2, 0, 0);
    3435    }
    … …  
    5354    shouldBe("document.elementFromPoint(215, 315)", "b2");
    5455
    55     window.scrollTo(0, 0);
    56     if (window.internals)
    57         window.internals.setPageScaleFactor(0.5, 0, 0);
    58     // b2 is now technically outside the 800x600 scaled viewport rect,
    59     // but should still be found.
    60 
    61     shouldBe("document.elementFromPoint(525, 425)", "b2");
    62 
    6356    finishJSTest();
    6457}

  • trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled.html

    r227974 r228714  
    22<html>
    33<head>
    4 <style>
    5     body {
    6         width: 2000px;
    7         height: 2000px;
    8     }
    9     button {
    10         position: absolute;
    11         width: 50px;
    12         height: 50px;
    13     }
    14     #b1 {
    15         left: 200px;
    16         top: 100px;
    17     }
    18     #b2 {
    19         left: 500px;
    20         top: 400px;
    21     }
    22 </style>
    23 <script src="../../resources/js-test-pre.js"></script>
     4<script src="../../../../resources/js-test-pre.js"></script>
    245</head>
    256<body onload="runTest();" style="width:2000px;height:2000px;margin:0px;padding:100px">
    26 <button id="b1"></button>
    27 <button id="b2"></button>
     7<button style="width:50px;height:50px;">B1</button><button style="width:50px;height:50px;">B2</button><button style="width:50px;height:50px;">B3</button>
     8<div>This test is successful if elementFromPoint returns the correct element.</div>
     9<div id="result"></div>
    2810<script>
    2911function runTest() {
    30     description("This test applies page scale and scrolls the page, and checks that elementFromPoint returns the correct element.");
    31     if (window.internals) {
    32         window.internals.settings.setVisualViewportEnabled(true);
     12    if (window.internals)
    3313        window.internals.setPageScaleFactor(2, 0, 0);
    34     }
    35     window.scrollTo(100, 100);
     14    window.scrollTo(100,100);
    3615
    37     // The layout viewport hasn't been scrolled.
    38     shouldBe("document.elementFromPoint(225, 125)", "b1");
    39     shouldBe("document.elementFromPoint(525, 425)", "b2");
     16    if (window.testRunner)
     17        testRunner.dumpAsText();
    4018
    41     window.scrollTo(200, 200);
    42 
    43     // b1 is now offscreen, but still within the layout viewport.
    44     shouldBe("document.elementFromPoint(225, 125)", "b1");
    45     shouldBe("document.elementFromPoint(525, 425)", "b2");
    46 
    47     window.scrollTo(500, 400);
    48     shouldBe("document.elementFromPoint(115, 15)", "b1");
    49     shouldBe("document.elementFromPoint(415, 315)", "b2");
    50 
    51     window.scrollTo(700, 400);
    52     shouldBeNull("document.elementFromPoint(-85, 15)");
    53     shouldBe("document.elementFromPoint(215, 315)", "b2");
    54 
    55     window.scrollTo(0, 0);
    56     if (window.internals)
    57         window.internals.setPageScaleFactor(0.5, 0, 0);
    58     // b2 is now technically outside the 800x600 scaled viewport rect,
    59     // but should still be found.
    60 
    61     shouldBe("document.elementFromPoint(525, 425)", "b2");
    62 
    63     finishJSTest();
     19    document.getElementById("result").innerText = document.elementFromPoint(125, 25).innerText;
    6420}
    6521</script>
    66 <script src="../../resources/js-test-post.js"></script>
     22</script>
    6723</body>
    6824</html>

  • trunk/LayoutTests/platform/ios-wk2/fast/dom/elementFromPoint-relative-to-viewport-expected.txt

    r219668 r228714  
    2121PASS unscrolledBoxInitial is '0'
    2222PASS scrolledDownBoxInitial is '5'
    23 FAIL scrolledRightBoxInitial should be 3. Was 2.
    24 FAIL scrolledDownAndRightBoxInitial should be 8. Was 7.
     23PASS scrolledRightBoxInitial is '3'
     24PASS scrolledDownAndRightBoxInitial is '8'
    2525PASS successfullyParsed is true
    2626

  • trunk/LayoutTests/platform/ios/TestExpectations

    r228699 r228714  
    28582858
    28592859# Test relies on window.scrollTo
    2860 fast/dom/elementFromPoint-scaled-scrolled.html [ Skip ]
     2860fast/dom/elementFromPoint-scaled-scrolled-layout-viewport.html [ Skip ]
    28612861fast/zooming/client-rect-in-fixed-zoomed.html [ Skip ]
    28622862fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed.html [ Skip ]
    … …  
    29532953webkit.org/b/171760 imported/w3c/i18n/bidi/bidi-plaintext-011.html [ ImageOnlyFailure ]
    29542954webkit.org/b/171760 imported/w3c/i18n/bidi/block-plaintext-005.html [ ImageOnlyFailure ]
     2955
     2956webkit.org/b/172019 imported/w3c/web-platform-tests/cssom-view/elementFromPoint.html [ Failure ]
    29552957
    29562958webkit.org/b/172547 http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch.html [ Failure ]

  • trunk/Source/WebCore/ChangeLog

    r228712 r228714  
     12018-02-19  Tim Horton  <timothy_horton@apple.com>
     2
     3        REGRESSION (r219342): Touch event coordinates and elementFromPoint coordinates differ
     4        https://bugs.webkit.org/show_bug.cgi?id=182910
     5        <rdar://problem/37533950>
     6
     7        Reviewed by Simon Fraser.
     8
     9        We reverted other changes to the definition of client coordinates
     10        in r219829 due to compatibility concerns. However, we failed to revert
     11        r219342 on trunk, leaving elementFromPoint() using coordinates relative
     12        to the layout viewport.
     13
     14        Add a currently off-by-default setting to switch on layout-viewport-relative
     15        client coordinates and guard the elementFromPoint changes behind it.
     16        A future patch should roll r219829 back in also behind this setting, so
     17        that everything remains consistent regardless of which coordinate space we choose.
     18
     19        * dom/TreeScope.cpp:
     20        (WebCore::absolutePointIfNotClipped):
     21        * page/Settings.yaml:
     22        * rendering/RenderLayer.cpp:
     23        (WebCore::RenderLayer::hitTest):
     24
    1252018-02-19  Eric Carlson  <eric.carlson@apple.com>
    226

  • trunk/Source/WebCore/dom/TreeScope.cpp

    r225719 r228714  
    300300        return std::nullopt;
    301301
    302     if (document.frame()->settings().visualViewportEnabled()) {
     302    const auto& settings = document.frame()->settings();
     303    if (settings.visualViewportEnabled() && settings.clientCoordinatesRelativeToLayoutViewport()) {
    303304        document.updateLayout();
    304305        if (!document.view() || !document.hasLivingRenderTree())

  • trunk/Source/WebCore/page/Settings.yaml

    r228697 r228714  
    732732resourceLoadStatisticsDebugMode:
    733733  initial: false
     734
     735clientCoordinatesRelativeToLayoutViewport:
     736  initial: false
     737  onChange: setNeedsRecalcStyleInAllFrames

  • trunk/Source/WebCore/rendering/RenderLayer.cpp

    r227974 r228714  
    49324932    LayoutRect hitTestArea = renderer().view().documentRect();
    49334933    if (!request.ignoreClipping()) {
    4934         if (renderer().settings().visualViewportEnabled()) {
     4934        const auto& settings = renderer().settings();
     4935        if (settings.visualViewportEnabled() && settings.clientCoordinatesRelativeToLayoutViewport()) {
    49354936            auto& frameView = renderer().view().frameView();
    49364937            LayoutRect absoluteLayoutViewportRect = frameView.layoutViewportRect();
Note: See TracChangeset for help on using the changeset viewer.