Bug 153137 - Implement document.elementsFromPoint
Summary: Implement document.elementsFromPoint
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ali Juma
URL: https://drafts.csswg.org/cssom-view/#...
Keywords: InRadar
Depends on: 171215
Blocks:
  Show dependency treegraph
 
Reported: 2016-01-15 10:44 PST by Mike Taylor
Modified: 2017-07-26 13:51 PDT (History)
13 users (show)

See Also:


Attachments
WIP (89.01 KB, patch)
2017-06-01 11:48 PDT, Ali Juma
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-elcapitan (1.07 MB, application/zip)
2017-06-01 12:56 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (1.67 MB, application/zip)
2017-06-01 13:03 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-elcapitan (1.83 MB, application/zip)
2017-06-01 13:17 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (1.09 MB, application/zip)
2017-06-01 14:05 PDT, Build Bot
no flags Details
Patch (95.60 KB, patch)
2017-06-23 11:10 PDT, Ali Juma
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (929.83 KB, application/zip)
2017-06-23 12:47 PDT, Build Bot
no flags Details
Patch (94.58 KB, patch)
2017-06-26 13:42 PDT, Ali Juma
no flags Details | Formatted Diff | Diff
Patch (94.37 KB, patch)
2017-07-17 13:42 PDT, Ali Juma
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-elcapitan (1.17 MB, application/zip)
2017-07-17 15:28 PDT, Build Bot
no flags Details
Patch (84.75 KB, patch)
2017-07-20 11:34 PDT, Ali Juma
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (9.91 MB, application/zip)
2017-07-20 17:43 PDT, Build Bot
no flags Details
Patch for landing (90.56 KB, patch)
2017-07-21 10:31 PDT, Ali Juma
no flags Details | Formatted Diff | Diff
Patch for landing (81.46 KB, patch)
2017-07-24 11:54 PDT, Ali Juma
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Taylor 2016-01-15 10:44:55 PST
Spec: https://drafts.csswg.org/cssom-view/#dom-document-elementsfrompoint

Chrome, Opera and (soon) Firefox are shipping this. IE10+ and Edge have it prefixed.
Comment 1 Frédéric Wang (:fredw) 2017-04-26 23:13:10 PDT
We have one W3C test for this at imported/w3c/web-platform-tests/cssom-view/elementsFromPoint.html
Comment 2 Ali Juma 2017-06-01 11:48:15 PDT
Created attachment 311737 [details]
WIP
Comment 3 Build Bot 2017-06-01 12:56:50 PDT
Comment on attachment 311737 [details]
WIP

Attachment 311737 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3855164

New failing tests:
imported/w3c/web-platform-tests/cssom-view/negativeMargins.html
Comment 4 Build Bot 2017-06-01 12:56:51 PDT
Created attachment 311743 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 5 Build Bot 2017-06-01 13:03:30 PDT
Comment on attachment 311737 [details]
WIP

Attachment 311737 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3855172

New failing tests:
imported/w3c/web-platform-tests/cssom-view/negativeMargins.html
Comment 6 Build Bot 2017-06-01 13:03:32 PDT
Created attachment 311744 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 7 Simon Fraser (smfr) 2017-06-01 13:06:13 PDT
Comment on attachment 311737 [details]
WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=311737&action=review

> Source/WebCore/dom/TreeScope.cpp:306
> +    point.scale(scaleFactor);
> +    point.moveBy(view->contentsScrollPosition());

This code will change with bug 172019. It should use a new function called clientToDocumentPoint() which is the mirror image of FrameView::documentToClientPoint().

> Source/WebCore/rendering/HitTestRequest.h:45
> +        ListBased = 1 << 13,

The "-based" doesn't fit the rest of the names. Maybe CollectsMultipleElements or MultipleResults or something.

> Source/WebCore/rendering/HitTestRequest.h:47
> +        PenetratingList = 1 << 14

This is a terrible name. Penetrating what? Maybe "inclusive"?

> Source/WebCore/rendering/HitTestRequest.h:71
> +    bool listBased() const { return m_requestType & ListBased; }
> +    bool penetratingList() const { return m_requestType & PenetratingList; }

These need renaming.
Comment 8 Build Bot 2017-06-01 13:17:03 PDT
Comment on attachment 311737 [details]
WIP

Attachment 311737 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3855180

New failing tests:
imported/w3c/web-platform-tests/cssom-view/negativeMargins.html
Comment 9 Build Bot 2017-06-01 13:17:05 PDT
Created attachment 311749 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 10 Ryosuke Niwa 2017-06-01 13:41:08 PDT
Please make this a runtime-enabled feature.
Comment 11 Build Bot 2017-06-01 14:05:14 PDT
Comment on attachment 311737 [details]
WIP

Attachment 311737 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3855432

New failing tests:
imported/w3c/web-platform-tests/cssom-view/negativeMargins.html
fast/dom/elementsFromPoint/elementsFromPoint-iframes.html
fast/dom/elementsFromPoint/elementsFromPoint-invalid-cases.html
Comment 12 Build Bot 2017-06-01 14:05:15 PDT
Created attachment 311758 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 13 Ali Juma 2017-06-23 11:10:17 PDT
Created attachment 313729 [details]
Patch
Comment 14 Build Bot 2017-06-23 12:47:10 PDT
Comment on attachment 313729 [details]
Patch

Attachment 313729 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3985318

New failing tests:
fast/dom/elementFromPoint-scaled-scrolled.html
Comment 15 Build Bot 2017-06-23 12:47:11 PDT
Created attachment 313734 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 16 Simon Fraser (smfr) 2017-06-23 12:49:04 PDT
Comment on attachment 313729 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=313729&action=review

> Source/WebCore/ChangeLog:15
> +        This also makes elementFromPoint and elementsFromPoint treat their input
> +        point to be in client (layout viewport) coordinates.

Can you please do this change separately, via bug 172019?
Comment 17 Ali Juma 2017-06-26 13:42:55 PDT
Created attachment 313866 [details]
Patch
Comment 18 Ali Juma 2017-06-26 13:58:59 PDT
Comment on attachment 313866 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=313866&action=review

> Source/WebCore/dom/DocumentOrShadowRoot.idl:34
> +    [EnabledAtRuntime=ElementsFromPoint] sequence<Element> elementsFromPoint(double x, double y);

This is using a runtime-enabled feature rather than a Setting because of bug 173737.

> LayoutTests/imported/w3c/web-platform-tests/cssom-view/elementsFromPoint-expected.txt:15
> +FAIL transformed element at x,y assert_array_equals: Should have returned a sequence with [pink, body, html] lengths differ, expected 3 got 0
> +FAIL no hit target at x,y assert_array_equals: Should have returned the sequence [body, html] lengths differ, expected 2 got 0

These two failures are because the test requires a larger window than what we use in layout tests; they pass when run interactively in a larger window. I'll get the test fixed.

> LayoutTests/imported/w3c/web-platform-tests/cssom-view/elementsFromPoint-shadowroot-expected.txt:3
> +FAIL elementsFromPoint on a shadow root should only return elements in that shadow tree assert_equals: shadowRoot.elementsFromPoint(50,15) expected "DIV#box12" but got "DIV#box12, DIV#host, BODY, HTML"

This is bug 170743.

> LayoutTests/imported/w3c/web-platform-tests/cssom-view/negativeMargins-expected.txt:4
> +FAIL cssom-view - elementFromPoint and elementsFromPoint dealing with negative margins 1 assert_array_equals: elementsFromPoint should get sequence [inner, outer, body, html] lengths differ, expected 4 got 5

The expected result here seems wrong to me -- it requires elementsFromPoint()[0] (which is 'inner') to be different from elementFromPoint() (which is 'outer'). The test was added in https://bugzilla.mozilla.org/show_bug.cgi?id=1164427, and reading through the comments (e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1164427#c27, https://bugzilla.mozilla.org/show_bug.cgi?id=1164427#c31) it sounds like they matched buggy behavior from Chrome under the mistaken impression that the spec required that behavior.
Comment 19 Ali Juma 2017-06-26 14:03:31 PDT
(In reply to Simon Fraser (smfr) from comment #16)
> Comment on attachment 313729 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=313729&action=review
> 
> > Source/WebCore/ChangeLog:15
> > +        This also makes elementFromPoint and elementsFromPoint treat their input
> > +        point to be in client (layout viewport) coordinates.
> 
> Can you please do this change separately, via bug 172019?

I've removed this part of the change, and will do it in bug 172019.
Comment 20 Ali Juma 2017-07-17 13:42:24 PDT
Created attachment 315705 [details]
Patch
Comment 21 Ali Juma 2017-07-17 13:48:23 PDT
Comment on attachment 315705 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315705&action=review

> Source/WebCore/ChangeLog:14
> +        This is a runtime-enabled feature, enabled by default.

Just wanted to call out explicitly that this patch enables the feature by default. Let me know if there are other things that block enabling by default.

> LayoutTests/imported/w3c/ChangeLog:30
> +        * web-platform-tests/cssom-view/resources/iframe2.html: Added.

As recommended on webkit-dev (https://lists.webkit.org/pipermail/webkit-dev/2017-July/029265.html), I've created an upstream PR (https://github.com/w3c/web-platform-tests/pull/6568) for these tests in parallel with this review.
Comment 22 Build Bot 2017-07-17 15:28:48 PDT
Comment on attachment 315705 [details]
Patch

Attachment 315705 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4137629

New failing tests:
security/contentSecurityPolicy/video-with-data-url-allowed-by-media-src-star.html
Comment 23 Build Bot 2017-07-17 15:28:49 PDT
Created attachment 315722 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 24 Brian Burg 2017-07-19 11:19:45 PDT
(In reply to Ryosuke Niwa from comment #10)
> Please make this a runtime-enabled feature.

This doesn't seem necessary. Is there a reason right now? What's the timeline/criteria for not making it optional? It's in a spec...
Comment 25 Simon Fraser (smfr) 2017-07-19 11:21:15 PDT
Comment on attachment 315705 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315705&action=review

Generally looks good but I'd like to see the suggested cleanup happen.

> Source/WebCore/dom/TreeScope.cpp:299
> +static bool absolutePointIfNotClipped(Document& document, const LayoutPoint& clientPoint, LayoutPoint& absolutePoint)

Cool kids would return an optional<LayoutPoint>

> Source/WebCore/dom/TreeScope.cpp:367
> +Vector<Element*> TreeScope::elementsFromPoint(double x, double y)

Can we name these clientX, clientY (and rename args to elementFromPoint too)?

> Source/WebCore/dom/TreeScope.h:92
>      WEBCORE_EXPORT Element* elementFromPoint(double x, double y);
> +    WEBCORE_EXPORT Vector<Element*> elementsFromPoint(double x, double y);

I think elementFromPoint() should return a RefPtr<Element>, and elementsFromPoint a Vector<RefPtr<Element>>.

> Source/WebCore/page/RuntimeEnabledFeatures.h:70
> +    void setElementsFromPointEnabled(bool isEnabled) { m_elementsFromPointEnabled = isEnabled; }
> +    bool elementsFromPointEnabled() const { return m_elementsFromPointEnabled; }

I don't think you need to add a runtime-enabled feature for this now.

> Source/WebCore/rendering/HitTestRequest.h:47
> +        // Collect a list of nodes instead of just one. Used for elementsFromPoint and rect-based tests.
> +        CollectMultipleElements = 1 << 13,
> +        // When using list-based testing, continue hit testing even after a hit has been found.
> +        InclusiveList = 1 << 14

I don't like InclusiveList. How about "IncludeAllElementsUnderPoint".

> Source/WebCore/rendering/HitTestRequest.h:70
> +    bool collectsMultipleElements() const { return m_requestType & CollectMultipleElements; }

Maybe "resultIsElementList"? You call this a "list-based test" elsewhere.

> Source/WebCore/rendering/HitTestRequest.h:71
> +    bool inclusiveList() const { return m_requestType & InclusiveList; }

This also needs renaming.

> Source/WebCore/rendering/HitTestResult.cpp:678
> +        return true;

Really hard to know what this boolean result means. Would be clearer with an enum. Does it mean "continue testing"?

> Source/WebCore/rendering/HitTestResult.cpp:690
>          return false;

Same with this one.

> Source/WebCore/rendering/HitTestResult.cpp:725
> +        for (NodeSet::const_iterator it = other.m_listBasedTestResult->begin(), last = other.m_listBasedTestResult->end(); it != last; ++it)

auto. Can't we use a modern C++ loop with m_listBasedTestResult?

> Source/WebCore/rendering/HitTestResult.h:134
> +    // Returns true if the test is a list-based test and we should continue testing.

Please make an enum for this.

> Source/WebCore/rendering/RenderView.cpp:186
> +    ASSERT(!location.isRectBasedTest() || request.collectsMultipleElements());

Is it really RenderView's job to make this assertion?

> Source/WebCore/rendering/svg/RenderSVGShape.cpp:356
> +            LayoutPoint localLayoutPoint(localPoint);

Why is this local variable necessary?

> Source/WebKit/Shared/WebPreferencesDefinitions.h:294
> +    macro(ElementsFromPointEnabled, elementsFromPointEnabled, Bool, bool, true, "ElementsFromPoint", "Enable document.elementsFromPoint support") \

No need for this.

> Source/WebKit/UIProcess/API/C/WKPreferences.cpp:1534
> +void WKPreferencesSetElementsFromPointEnabled(WKPreferencesRef preferencesRef, bool enabled)
> +{
> +    toImpl(preferencesRef)->setElementsFromPointEnabled(enabled);
> +}
> +
> +bool WKPreferencesGetElementsFromPointEnabled(WKPreferencesRef preferencesRef)
> +{
> +    return toImpl(preferencesRef)->elementsFromPointEnabled();
> +}

No need for this.

> Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:510
> +// Defaults to true.
> +WK_EXPORT void WKPreferencesSetElementsFromPointEnabled(WKPreferencesRef, bool enabled);
> +WK_EXPORT bool WKPreferencesGetElementsFromPointEnabled(WKPreferencesRef);

No need for this.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:3355
> +    RuntimeEnabledFeatures::sharedFeatures().setElementsFromPointEnabled(store.getBoolValueForKey(WebPreferencesKey::elementsFromPointEnabledKey()));

No need for this.

> Source/WebKitLegacy/mac/WebView/WebPreferenceKeysPrivate.h:192
> +#define WebKitElementsFromPointEnabledPreferenceKey @"WebKitElementsFromPointEnabled"

No need for this.

> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:680
> +        @YES, WebKitElementsFromPointEnabledPreferenceKey,

No need for this.

> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:3197
> +- (BOOL)elementsFromPointEnabled
> +{
> +    return [self _boolValueForKey:WebKitElementsFromPointEnabledPreferenceKey];
> +}
> +
> +- (void)setElementsFromPointEnabled:(BOOL)flag
> +{
> +    [self _setBoolValue:flag forKey:WebKitElementsFromPointEnabledPreferenceKey];
> +}

No need for this.

> Source/WebKitLegacy/mac/WebView/WebPreferencesPrivate.h:566
> +- (void)setElementsFromPointEnabled:(BOOL)flag;
> +- (BOOL)elementsFromPointEnabled;

No need for this.

> Source/WebKitLegacy/mac/WebView/WebView.mm:2998
> +    RuntimeEnabledFeatures::sharedFeatures().setElementsFromPointEnabled([preferences elementsFromPointEnabled]);

No need for this.

> Tools/DumpRenderTree/mac/DumpRenderTree.mm:968
> +    [preferences setElementsFromPointEnabled:YES];

No need for this.

> Tools/WebKitTestRunner/TestController.cpp:669
> +    WKPreferencesSetElementsFromPointEnabled(preferences, true);

No need for this.
Comment 26 Radar WebKit Bug Importer 2017-07-19 11:24:40 PDT
<rdar://problem/33407048>
Comment 27 Ali Juma 2017-07-20 11:34:21 PDT
Created attachment 316006 [details]
Patch
Comment 28 Ali Juma 2017-07-20 11:35:54 PDT
Comment on attachment 315705 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315705&action=review

>> Source/WebCore/dom/TreeScope.cpp:299
>> +static bool absolutePointIfNotClipped(Document& document, const LayoutPoint& clientPoint, LayoutPoint& absolutePoint)
> 
> Cool kids would return an optional<LayoutPoint>

Done.

>> Source/WebCore/dom/TreeScope.cpp:367
>> +Vector<Element*> TreeScope::elementsFromPoint(double x, double y)
> 
> Can we name these clientX, clientY (and rename args to elementFromPoint too)?

Done.

>> Source/WebCore/dom/TreeScope.h:92
>> +    WEBCORE_EXPORT Vector<Element*> elementsFromPoint(double x, double y);
> 
> I think elementFromPoint() should return a RefPtr<Element>, and elementsFromPoint a Vector<RefPtr<Element>>.

Done.

>> Source/WebCore/page/RuntimeEnabledFeatures.h:70
>> +    bool elementsFromPointEnabled() const { return m_elementsFromPointEnabled; }
> 
> I don't think you need to add a runtime-enabled feature for this now.

Removed.

>> Source/WebCore/rendering/HitTestRequest.h:47
>> +        InclusiveList = 1 << 14
> 
> I don't like InclusiveList. How about "IncludeAllElementsUnderPoint".

Done.

>> Source/WebCore/rendering/HitTestRequest.h:70
>> +    bool collectsMultipleElements() const { return m_requestType & CollectMultipleElements; }
> 
> Maybe "resultIsElementList"? You call this a "list-based test" elsewhere.

Done.

>> Source/WebCore/rendering/HitTestRequest.h:71
>> +    bool inclusiveList() const { return m_requestType & InclusiveList; }
> 
> This also needs renaming.

Done.

>> Source/WebCore/rendering/HitTestResult.cpp:678
>> +        return true;
> 
> Really hard to know what this boolean result means. Would be clearer with an enum. Does it mean "continue testing"?

Changed to an enum.

>> Source/WebCore/rendering/HitTestResult.cpp:725
>> +        for (NodeSet::const_iterator it = other.m_listBasedTestResult->begin(), last = other.m_listBasedTestResult->end(); it != last; ++it)
> 
> auto. Can't we use a modern C++ loop with m_listBasedTestResult?

Done.

>> Source/WebCore/rendering/RenderView.cpp:186
>> +    ASSERT(!location.isRectBasedTest() || request.collectsMultipleElements());
> 
> Is it really RenderView's job to make this assertion?

Removed. The equivalent assertion in HitTestResult::addNodeToListBasedTestResult provides the same coverage.

>> Source/WebCore/rendering/svg/RenderSVGShape.cpp:356
>> +            LayoutPoint localLayoutPoint(localPoint);
> 
> Why is this local variable necessary?

Removed.
Comment 29 Simon Fraser (smfr) 2017-07-20 11:44:01 PDT
Comment on attachment 316006 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=316006&action=review

> Source/WebCore/dom/TreeScope.cpp:334
> +    std::optional<LayoutPoint> absolutePoint = absolutePointIfNotClipped(documentScope(), clientPoint);

auto absolutePoint

> Source/WebCore/dom/TreeScope.cpp:338
> +    HitTestResult result(*absolutePoint);

absolutePoint.value()

> Source/WebCore/dom/TreeScope.cpp:385
> +    HitTestResult result(*absolutePoint);

absolutePoint.value()

> Source/WebCore/rendering/HitTestResult.h:48
> +    enum HitTestProgress {
> +        StopHitTesting,
> +        ContinueHitTesting
> +    };

enum class HitTestProgress { Stop, Continue };

OK to move this outside the class.

> Source/WebCore/rendering/svg/RenderSVGContainer.cpp:186
> +            if (result.addNodeToListBasedTestResult(child->node(), request, localPoint) == HitTestResult::StopHitTesting)
> +                return true;

At some point we should convert nodeAtFloatPoint to return an enum as well.
Comment 30 Build Bot 2017-07-20 17:43:33 PDT
Comment on attachment 316006 [details]
Patch

Attachment 316006 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4158171

New failing tests:
imported/w3c/IndexedDB-private-browsing/idbfactory_open12.html
Comment 31 Build Bot 2017-07-20 17:43:35 PDT
Created attachment 316049 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 32 Ali Juma 2017-07-21 10:31:58 PDT
Created attachment 316099 [details]
Patch for landing
Comment 33 Ali Juma 2017-07-21 10:34:21 PDT
Comment on attachment 316099 [details]
Patch for landing

Marking cq- since the new web platform tests in this patch need to land upstream before this patch lands.
Comment 34 Ali Juma 2017-07-24 11:54:41 PDT
Created attachment 316304 [details]
Patch for landing
Comment 35 Ali Juma 2017-07-24 12:42:23 PDT
Comment on attachment 316304 [details]
Patch for landing

This is ready to land now (the web platform tests have landed upstream).
Comment 36 Simon Fraser (smfr) 2017-07-24 12:47:20 PDT
The test will probably fail after https://trac.webkit.org/changeset/219829/webkit. You should land failing results, since I'll probably revert https://trac.webkit.org/changeset/219829/webkit again soon (after it's been cherry-picked for a branch).
Comment 37 Ali Juma 2017-07-24 14:14:10 PDT
(In reply to Simon Fraser (smfr) from comment #36)
> The test will probably fail after
> https://trac.webkit.org/changeset/219829/webkit. You should land failing
> results, since I'll probably revert
> https://trac.webkit.org/changeset/219829/webkit again soon (after it's been
> cherry-picked for a branch).

I re-ran the tests after that change, and it turns out they still pass (the implementation of elementsFromPoint doesn't use documentToClientOffset; instead, it uses clientToLayoutViewportPoint and layoutViewportToAbsolutePoint).
Comment 38 Simon Fraser (smfr) 2017-07-24 14:25:23 PDT
That means that I failed to change those methods (I knew there was a reason I didn't want functions that didn't go through documentToClientOffset() :( )
Comment 39 WebKit Commit Bot 2017-07-26 13:51:30 PDT
Comment on attachment 316304 [details]
Patch for landing

Clearing flags on attachment: 316304

Committed r219961: <http://trac.webkit.org/changeset/219961>
Comment 40 WebKit Commit Bot 2017-07-26 13:51:32 PDT
All reviewed patches have been landed.  Closing bug.