Bug 162882 - ShadowRoot interface should have elementFromPoint
Summary: ShadowRoot interface should have elementFromPoint
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks: 148695
  Show dependency treegraph
 
Reported: 2016-10-03 16:34 PDT by Ryosuke Niwa
Modified: 2016-10-05 16:29 PDT (History)
5 users (show)

See Also:


Attachments
Adds the support (33.30 KB, patch)
2016-10-03 17:16 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-yosemite (893.99 KB, application/zip)
2016-10-03 18:21 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (930.11 KB, application/zip)
2016-10-03 18:24 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-yosemite (1.94 MB, application/zip)
2016-10-03 18:33 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews122 for ios-simulator-elcapitan-wk2 (6.73 MB, application/zip)
2016-10-03 18:44 PDT, Build Bot
no flags Details
Reverted the unintended type changes in IDL (35.44 KB, patch)
2016-10-03 23:55 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-yosemite (879.05 KB, application/zip)
2016-10-04 01:00 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (1003.92 KB, application/zip)
2016-10-04 01:03 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-elcapitan-wk2 (13.41 MB, application/zip)
2016-10-04 01:08 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews112 for mac-yosemite (1.68 MB, application/zip)
2016-10-04 01:09 PDT, Build Bot
no flags Details
Fixed testharness.js path (35.38 KB, patch)
2016-10-04 14:41 PDT, Ryosuke Niwa
cdumez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2016-10-03 16:34:33 PDT
Add elementFromPoint on ShadowRoot. Chrome already supports it.
Comment 1 Ryosuke Niwa 2016-10-03 17:16:04 PDT
Created attachment 290539 [details]
Adds the support
Comment 2 Build Bot 2016-10-03 18:21:32 PDT
Comment on attachment 290539 [details]
Adds the support

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

New failing tests:
fast/dom/non-numeric-values-numeric-parameters.html
fast/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint.html
Comment 3 Build Bot 2016-10-03 18:21:35 PDT
Created attachment 290543 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 4 Build Bot 2016-10-03 18:24:41 PDT
Comment on attachment 290539 [details]
Adds the support

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

New failing tests:
fast/dom/non-numeric-values-numeric-parameters.html
fast/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint.html
Comment 5 Build Bot 2016-10-03 18:24:45 PDT
Created attachment 290544 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 6 Build Bot 2016-10-03 18:33:17 PDT
Comment on attachment 290539 [details]
Adds the support

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

New failing tests:
media/video-click-dblckick-standalone.html
editing/selection/selection-in-iframe-removed-crash.html
fast/dom/non-numeric-values-numeric-parameters.html
fast/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint.html
fast/dom/adopt-node-crash-2.html
Comment 7 Build Bot 2016-10-03 18:33:20 PDT
Created attachment 290546 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 8 Build Bot 2016-10-03 18:44:24 PDT
Comment on attachment 290539 [details]
Adds the support

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

New failing tests:
fast/dom/non-numeric-values-numeric-parameters.html
fast/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint.html
Comment 9 Build Bot 2016-10-03 18:44:28 PDT
Created attachment 290548 [details]
Archive of layout-test-results from ews122 for ios-simulator-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 10 Ryosuke Niwa 2016-10-03 23:55:54 PDT
Created attachment 290572 [details]
Reverted the unintended type changes in IDL
Comment 11 Build Bot 2016-10-04 01:00:42 PDT
Comment on attachment 290572 [details]
Reverted the unintended type changes in IDL

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

New failing tests:
fast/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint.html
Comment 12 Build Bot 2016-10-04 01:00:46 PDT
Created attachment 290574 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 13 Build Bot 2016-10-04 01:03:53 PDT
Comment on attachment 290572 [details]
Reverted the unintended type changes in IDL

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

New failing tests:
fast/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint.html
Comment 14 Build Bot 2016-10-04 01:03:57 PDT
Created attachment 290576 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 15 Build Bot 2016-10-04 01:08:31 PDT
Comment on attachment 290572 [details]
Reverted the unintended type changes in IDL

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

New failing tests:
fast/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint.html
Comment 16 Build Bot 2016-10-04 01:08:35 PDT
Created attachment 290579 [details]
Archive of layout-test-results from ews126 for ios-simulator-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 17 Build Bot 2016-10-04 01:09:02 PDT
Comment on attachment 290572 [details]
Reverted the unintended type changes in IDL

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

New failing tests:
fast/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint.html
Comment 18 Build Bot 2016-10-04 01:09:05 PDT
Created attachment 290580 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 19 Chris Dumez 2016-10-04 14:35:08 PDT
Comment on attachment 290572 [details]
Reverted the unintended type changes in IDL

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

> LayoutTests/fast/shadow-dom/DocumentOrShadowRoot-prototype-elementFromPoint.html:9
> +<script src="/Volumes/Data/git-webkit/LayoutTests/resources/testharness.js"></script>

Nice :D
Comment 20 Chris Dumez 2016-10-04 14:38:44 PDT
Comment on attachment 290572 [details]
Reverted the unintended type changes in IDL

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

> Source/WebCore/ChangeLog:12
> +        Added TreeScope::retargetToScope which implements 

implements.. what?
Comment 21 Ryosuke Niwa 2016-10-04 14:41:36 PDT
Created attachment 290647 [details]
Fixed testharness.js path
Comment 22 Chris Dumez 2016-10-04 14:59:37 PDT
Comment on attachment 290647 [details]
Fixed testharness.js path

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

> Source/WebCore/ChangeLog:12
> +        Added TreeScope::retargetToScope which implements 

Which implements.. what?

> Source/WebCore/dom/TreeScope.cpp:174
> +Node& TreeScope::retargetToScope(Node& node) const

The spec says:
"""
To retarget an object A against an object B, repeat these steps until they return an object:

If A’s root is not a shadow root, or A’s root is a shadow-including inclusive ancestor of B, then return A.

Set A to A’s root’s host.
"""

This seems simpler than what you implemented and would not require allocating vectors for the whole ancestry.

Also, I may have misread that doesn't your implementation assume this and node have the same depth?

-> while (i > 0 && j > 0 && nodeTreeScopes[i - 1] == ancestorScopes[j - 1]) {

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3216
> +    RuntimeEnabledFeatures::sharedFeatures().setCustomElementsEnabled(true || store.getBoolValueForKey(WebPreferencesKey::customElementsEnabledKey()));

Probably did not mean to include this?
Comment 23 Chris Dumez 2016-10-04 15:12:30 PDT
Comment on attachment 290647 [details]
Fixed testharness.js path

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

>> Source/WebCore/dom/TreeScope.cpp:174
>> +Node& TreeScope::retargetToScope(Node& node) const
> 
> The spec says:
> """
> To retarget an object A against an object B, repeat these steps until they return an object:
> 
> If A’s root is not a shadow root, or A’s root is a shadow-including inclusive ancestor of B, then return A.
> 
> Set A to A’s root’s host.
> """
> 
> This seems simpler than what you implemented and would not require allocating vectors for the whole ancestry.
> 
> Also, I may have misread that doesn't your implementation assume this and node have the same depth?
> 
> -> while (i > 0 && j > 0 && nodeTreeScopes[i - 1] == ancestorScopes[j - 1]) {

Ryosuke explained offline and I now believe this works. I still think this is a bit obscure and is not guaranteed to be faster in common cases than the trivial implementation.
That said, I don't feel strongly either way.
Comment 24 Ryosuke Niwa 2016-10-04 16:44:49 PDT
Committed r206795: <http://trac.webkit.org/changeset/206795>
Comment 25 Ryosuke Niwa 2016-10-05 16:29:53 PDT
<rdar://problem/28642151>