Bug 162091 - Text nodes assigned to a linked slot are not clickable
Summary: Text nodes assigned to a linked slot are not clickable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified OS X 10.11
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks: 148695
  Show dependency treegraph
 
Reported: 2016-09-16 14:27 PDT by Jan Miksovsky
Modified: 2016-09-29 12:52 PDT (History)
10 users (show)

See Also:


Attachments
Fixes the bug (7.00 KB, patch)
2016-09-28 17:48 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Fixes the bug (7.00 KB, patch)
2016-09-28 18:52 PDT, Ryosuke Niwa
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Miksovsky 2016-09-16 14:27:25 PDT
See http://jsbin.com/qinaxo/edit?html,output

This shows a link in a Shadow DOM subtree. Half the content is directly inside the anchor element, the other half of the content is assigned via a `<slot>` element.

Expect: The entire link should be clickable.
Actual: The content assigned to the `<slot>` element is not clickable.
Comment 1 Radar WebKit Bug Importer 2016-09-20 02:12:36 PDT
<rdar://problem/28383300>
Comment 2 Ryosuke Niwa 2016-09-28 17:48:15 PDT
Created attachment 290153 [details]
Fixes the bug
Comment 3 Ryosuke Niwa 2016-09-28 18:52:32 PDT
Created attachment 290159 [details]
Fixes the bug
Comment 4 Antti Koivisto 2016-09-29 02:06:34 PDT
Comment on attachment 290159 [details]
Fixes the bug

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

> Source/WebCore/dom/Node.cpp:1845
> -    for (Node* node = this; node; node = node->parentOrShadowHostNode()) {
> +    for (Node* node = this; node; node = node->parentInComposedTree()) {

It would be more stylish to use ComposedTreeAncestorIterator. The current composedTreeAncestors() starts from the parent though so dealing with the OrSelf part here would require refactoring a bit (or adding new composedTreeLineage() helper that starts from the current node).

I think we should eventually get rid of all these random helpers. On the other hand just switching parentOrShadowHostNode->parentInComposedTree is a good way to make progress fast.
Comment 5 Ryosuke Niwa 2016-09-29 12:42:11 PDT
Comment on attachment 290159 [details]
Fixes the bug

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

>> Source/WebCore/dom/Node.cpp:1845
>> +    for (Node* node = this; node; node = node->parentInComposedTree()) {
> 
> It would be more stylish to use ComposedTreeAncestorIterator. The current composedTreeAncestors() starts from the parent though so dealing with the OrSelf part here would require refactoring a bit (or adding new composedTreeLineage() helper that starts from the current node).
> 
> I think we should eventually get rid of all these random helpers. On the other hand just switching parentOrShadowHostNode->parentInComposedTree is a good way to make progress fast.

I'd keep parentInComposedTree for now, and we can do this in a separate patch if we'd so wished.
Comment 6 Ryosuke Niwa 2016-09-29 12:52:18 PDT
Committed r206605: <http://trac.webkit.org/changeset/206605>