Bug 149441 - Implement ::slotted pseudo element
Summary: Implement ::slotted pseudo element
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 148695
  Show dependency treegraph
 
Reported: 2015-09-21 18:17 PDT by Ryosuke Niwa
Modified: 2016-02-26 06:57 PST (History)
5 users (show)

See Also:


Attachments
wip (13.99 KB, patch)
2016-02-25 07:50 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (20.72 KB, patch)
2016-02-25 13:00 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (20.68 KB, patch)
2016-02-25 13:21 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (20.76 KB, patch)
2016-02-25 13:24 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (25.40 KB, patch)
2016-02-26 00:09 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (28.15 KB, patch)
2016-02-26 02:57 PST, Antti Koivisto
koivisto: review+
Details | Formatted Diff | Diff
patch (26.95 KB, patch)
2016-02-26 03:11 PST, Antti Koivisto
kling: 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 2015-09-21 18:17:11 PDT
We should probide a mechanism to style nodes assigned to a slot inside a shadow tree.

For example, if we're creating a list view, we may want to add a line between items by having rules like
"::slotted > * { border-bottom: solid 1px #ccc; }"
Comment 1 Ryosuke Niwa 2015-09-21 18:17:33 PDT
<rdar://problem/22731987>
Comment 2 Antti Koivisto 2016-02-25 07:50:37 PST
Created attachment 272203 [details]
wip
Comment 3 Antti Koivisto 2016-02-25 13:00:51 PST
Created attachment 272226 [details]
patch
Comment 4 WebKit Commit Bot 2016-02-25 13:03:32 PST
Attachment 272226 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:9:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Antti Koivisto 2016-02-25 13:21:58 PST
Created attachment 272230 [details]
patch
Comment 6 Antti Koivisto 2016-02-25 13:24:49 PST
Created attachment 272231 [details]
patch
Comment 7 Antti Koivisto 2016-02-26 00:09:30 PST
Created attachment 272310 [details]
patch
Comment 8 Ryosuke Niwa 2016-02-26 00:36:31 PST
Comment on attachment 272310 [details]
patch

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

The test change looks correct.  Someone who's more familiar with selector code should review it.

> Source/WebCore/css/ElementRuleCollector.cpp:256
> +    auto* hostShadowRoot = m_element.parentNode()->shadowRoot();
> +    ASSERT(hostShadowRoot);
> +    auto* slot = hostShadowRoot->findAssignedSlot(m_element);
> +    if (!slot)
> +        return;

I think this isn't quite right for the distributed node case.
e.g. when an element X is assigned to a slot A, and the slot A is assigned to another slot B,
then the rule in B's shadow DOM that says ::slotted(X) is supposed to match X.

What's not clear from the Github issue is
whether we also need to match the rule in A's shadow DOM or not.

> Source/WebCore/css/ElementRuleCollector.h:104
> +    bool m_isMatchindSlottedPseudoElement { false };

Typo: Matchind -> Matching.

> LayoutTests/fast/shadow-dom/slotted-pseudo-element-css-text.html:65
> +  for (var i = 0; i < expectedCSSTexts.length; ++i)
> +    assert_equals(cssRules.item(i).cssText, expectedCSSTexts[i]);
> +  assert_equals(cssRules.length, expectedCSSTexts.length);

Can we call test for each selector so that we can see result per selector?
Comment 9 Antti Koivisto 2016-02-26 01:52:38 PST
> I think this isn't quite right for the distributed node case.
> e.g. when an element X is assigned to a slot A, and the slot A is assigned
> to another slot B,
> then the rule in B's shadow DOM that says ::slotted(X) is supposed to match
> X.

Not sure what you mean here. You can't assign a slot to a slot since (functional) slots are only allowed in shadow trees. Nested hosts should work fine (and are tested by 'nested-host' part of the test).

Can you sketch a test case of what you mean?
Comment 10 Antti Koivisto 2016-02-26 02:57:44 PST
Created attachment 272318 [details]
patch
Comment 11 Antti Koivisto 2016-02-26 03:11:34 PST
Created attachment 272319 [details]
patch
Comment 12 Andreas Kling 2016-02-26 06:29:06 PST
Comment on attachment 272319 [details]
patch

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

r=me

> Source/WebCore/css/ElementRuleCollector.cpp:303
> +    RuleSet::RuleDataVector ruleDataVector;
> +    for (auto& matchedRule : m_matchedRules)
> +        ruleDataVector.append(*matchedRule.ruleData);
> +    return ruleDataVector;

Could use some reserveInitialCapacity+uncheckedAppend here.
Comment 13 Antti Koivisto 2016-02-26 06:57:46 PST
https://trac.webkit.org/r197165