Summary: | Implement ::slotted pseudo element | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||||||||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | barraclough, commit-queue, koivisto, rniwa, webkit-bug-importer | ||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
Bug Depends on: | |||||||||||||||||||
Bug Blocks: | 148695 | ||||||||||||||||||
Attachments: |
|
Description
Ryosuke Niwa
2015-09-21 18:17:11 PDT
Created attachment 272203 [details]
wip
Created attachment 272226 [details]
patch
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.
Created attachment 272230 [details]
patch
Created attachment 272231 [details]
patch
Created attachment 272310 [details]
patch
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? > 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?
Created attachment 272318 [details]
patch
Created attachment 272319 [details]
patch
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. |