8000 [@property] Rules in shadow trees should be ignored. · WebKit/WebKit@997d987 · GitHub
[go: up one dir, main page]

Skip to content

Commit 997d987

Browse files
committed
[@Property] Rules in shadow trees should be ignored.
https://bugs.webkit.org/show_bug.cgi?id=250567 rdar://104221943 Reviewed by Simon Fraser. "A @Property is invalid if it occurs in a stylesheet inside of a shadow tree, and must be ignored." https://drafts.css-houdini.org/css-properties-values-api/#at-property-rule * LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-shadow-expected.txt: * Source/WebCore/css/CSSVariableReferenceValue.cpp: * Source/WebCore/css/DOMCSSRegisterCustomProperty.cpp: (WebCore::DOMCSSRegisterCustomProperty::registerProperty): No need to create a style resolver. * Source/WebCore/style/RuleSetBuilder.cpp: (WebCore::Style::RuleSetBuilder::addChildRule): Fix indentation. (WebCore::Style::RuleSetBuilder::addMutatingRulesToResolver): The actual fix. Check if we are in a shadow scope and bail. * Source/WebCore/style/StyleResolver.cpp: (WebCore::Style::Resolver::create): Pass the scope as enum value (document or shadow). Due to sharing between identically styled shadow trees we don't provide the actual Style::Scope. (WebCore::Style::Resolver::Resolver): Use WeakPtr for the document. (WebCore::Style::Resolver::initialize): Factor into a function. (WebCore::Style::Resolver::document): (WebCore::Style::Resolver::document const): (WebCore::Style::Resolver::scope): Deleted. (WebCore::Style::Resolver::scope const): Deleted. (WebCore::Style::Resolver::~Resolver): Deleted. * Source/WebCore/style/StyleResolver.h: (WebCore::Style::Resolver::scopeType const): * Source/WebCore/style/StyleScope.cpp: (WebCore::Style::Scope::createDocumentResolver): (WebCore::Style::Scope::createOrFindSharedShadowTreeResolver): Canonical link: https://commits.webkit.org/258880@main
1 parent de0e298 commit 997d987

File tree

7 files changed

+135
-110
lines changed

7 files changed

+135
-110
lines changed
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11

2-
< 8000 span class="x x-first x-last">FAIL @property rules in shadow trees should have no effect assert_equals: expected "calc(1px + 1px)" but got "2px"
2+
PASS @property rules in shadow trees should have no effect
33

Source/WebCore/css/CSSVariableReferenceValue.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "CSSVariableData.h"
3838
#include "ConstantPropertyMap.h"
3939
#include "CustomPropertyRegistry.h"
40+
#include "Document.h"
4041
#include "RenderStyle.h"
4142
#include "StyleBuilder.h"
4243
#include "StyleResolver.h"

Source/WebCore/css/DOMCSSRegisterCustomProperty.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,22 +54,18 @@ ExceptionOr<void> DOMCSSRegisterCustomProperty::registerProperty(Document& docum
5454
RefPtr<CSSCustomPropertyValue> initialValue;
5555
if (!descriptor.initialValue.isNull()) {
5656
CSSTokenizer tokenizer(descriptor.initialValue);
57-
auto styleResolver = Style::Resolver::create(document);
58-
59-
// We need to initialize this so that we can successfully parse computationally dependent values (like em units).
60-
// We don't actually need the values to be accurate, since they will be rejected later anyway
61-
auto style = styleResolver->defaultStyleForElement(nullptr);
6257

6358
HashSet<CSSPropertyID> dependencies;
6459
CSSPropertyParser::collectParsedCustomPropertyValueDependencies(*syntax, true /* isInitial */, dependencies, tokenizer.tokenRange(), strictCSSParserContext());
6560

6661
if (!dependencies.isEmpty())
6762
return Exception { SyntaxError, "The given initial value must be computationally independent."_s };
6863

69-
auto parentStyle = RenderStyle::clone(*style);
70-
Style::Builder dummyBuilder(*style, { document, parentStyle }, { }, { });
64+
// We don't need to provide a real context style since only computationally independent values are allowed (no 'em' etc).
65+
auto placeholderStyle = RenderStyle::create();
66+
Style::Builder builder { placeholderStyle, { document, RenderStyle::defaultStyle() }, { }, { } };
7167

72-
initialValue = CSSPropertyParser::parseTypedCustomPropertyInitialValue(descriptor.name, *syntax, tokenizer.tokenRange(), dummyBuilder.state(), { document });
68+
initialValue = CSSPropertyParser::parseTypedCustomPropertyInitialValue(descriptor.name, *syntax, tokenizer.tokenRange(), builder.state(), { document });
7369

7470
if (!initialValue)
7571
return Exception { SyntaxError, "The given initial value does not parse for the given syntax."_s };

Source/WebCore/style/RuleSetBuilder.cpp

Lines changed: 70 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -104,81 +104,81 @@ void RuleSetBuilder::addChildRules(const Vector<RefPtr<StyleRuleBase>>& rules)
104104

105105
void RuleSetBuilder::addChildRule(RefPtr<StyleRuleBase> rule)
106106
{
107-
switch (rule->type()) {
108-
case StyleRuleType::Style:
109-
if (m_ruleSet)
110-
addStyleRule(downcast<StyleRule>(*rule));
111-
return;
107+
switch (rule->type()) {
108+
case StyleRuleType::Style:
109+
if (m_ruleSet)
110+
addStyleRule(downcast<StyleRule>(*rule));
111+
return;
112112

113-
case StyleRuleType::Page:
114-
if (m_ruleSet)
115-
m_ruleSet->addPageRule(downcast<StyleRulePage>(*rule));
116-
return;
113+
case StyleRuleType::Page:
114+
if (m_ruleSet)
115+
m_ruleSet->addPageRule(downcast<StyleRulePage>(*rule));
116+
return;
117117

118-
case StyleRuleType::Media: {
119-
auto& mediaRule = downcast<StyleRuleMedia>(*rule);
120-
if (m_mediaQueryCollector.pushAndEvaluate(mediaRule.mediaQueries()))
121-
addChildRules(mediaRule.childRules());
122-
m_mediaQueryCollector.pop(mediaRule.mediaQueries());
123-
return;
124-
}
118+
case StyleRuleType::Media: {
119+
auto& mediaRule = downcast<StyleRuleMedia>(*rule);
120+
if (m_mediaQueryCollector.pushAndEvaluate(mediaRule.mediaQueries()))
121+
addChildRules(mediaRule.childRules());
122+
m_mediaQueryCollector.pop(mediaRule.mediaQueries());
123+
return;
124+
}
125125

126-
case StyleRuleType::Container: {
127-
auto& containerRule = downcast<StyleRuleContainer>(*rule);
128-
auto previousContainerQueryIdentifier = m_currentContainerQueryIdentifier;
129-
if (m_ruleSet) {
130-
m_ruleSet->m_containerQueries.append({ containerRule, previousContainerQueryIdentifier });
131-
m_currentContainerQueryIdentifier = m_ruleSet->m_containerQueries.size();
132-
}
133-
addChildRules(containerRule.childRules());
134-
if (m_ruleSet)
135-
m_currentContainerQueryIdentifier = previousContainerQueryIdentifier;
136-
return;
126+
case StyleRuleType::Container: {
127+
auto& containerRule = downcast<StyleRuleContainer>(*rule);
128+
auto previousContainerQueryIdentifier = m_currentContainerQueryIdentifier;
129+
if (m_ruleSet) {
130+
m_ruleSet->m_containerQueries.append({ containerRule, previousContainerQueryIdentifier });
131+
m_currentContainerQueryIdentifier = m_ruleSet->m_containerQueries.size();
137132
}
133+
addChildRules(containerRule.childRules());
134+
if (m_ruleSet)
135+
m_currentContainerQueryIdentifier = previousContainerQueryIdentifier;
136+
return;
137+
}
138138

139-
case StyleRuleType::LayerBlock:
140-
case StyleRuleType::LayerStatement: {
141-
disallowDynamicMediaQueryEvaluationIfNeeded();
139+
case StyleRuleType::LayerBlock:
140+
case StyleRuleType::LayerStatement: {
141+
disallowDynamicMediaQueryEvaluationIfNeeded();
142142

143-
auto& layerRule = downcast<StyleRuleLayer>(*rule);
144-
if (layerRule.isStatement()) {
145-
// Statement syntax just registers the layers.
146-
registerLayers(layerRule.nameList());
147-
return;
148-
}
149-
// Block syntax.
150-
pushCascadeLayer(layerRule.name());
151-
addChildRules(layerRule.childRules());
152-
popCascadeLayer(layerRule.name());
143+
auto& layerRule = downcast<StyleRuleLayer>(*rule);
144+
if (layerRule.isStatement()) {
145+
// Statement syntax just registers the layers.
146+
registerLayers(layerRule.nameList());
153147
return;
154148
}
155-
case StyleRuleType::CounterStyle:
156 67DE -
case StyleRuleType::FontFace:
157-
case StyleRuleType::FontPaletteValues:
158-
case StyleRuleType::FontFeatureValues:
159-
case StyleRuleType::Keyframes:
160-
case StyleRuleType::Property:
161-
disallowDynamicMediaQueryEvaluationIfNeeded();
162-
if (m_resolver)
163-
m_collectedResolverMutatingRules.append({ *rule, m_currentCascadeLayerIdentifier });
164-
return;
149+
// Block syntax.
150+
pushCascadeLayer(layerRule.name());
151+
addChildRules(layerRule.childRules());
152+
popCascadeLayer(layerRule.name());
153+
return;
154+
}
155+
case StyleRuleType::CounterStyle:
156+
case StyleRuleType::FontFace:
157+
case StyleRuleType::FontPaletteValues:
158+
case StyleRuleType::FontFeatureValues:
159+
case StyleRuleType::Keyframes:
160+
case StyleRuleType::Property:
161+
disallowDynamicMediaQueryEvaluationIfNeeded();
162+
if (m_resolver)
163+
m_collectedResolverMutatingRules.append({ *rule, m_currentCascadeLayerIdentifier });
164+
return;
165165

166-
case StyleRuleType::Supports:
167-
if (downcast<StyleRuleSupports>(*rule).conditionIsSupported())
168-
addChildRules(downcast<StyleRuleSupports>(*rule).childRules());
169-
return;
166+
case StyleRuleType::Supports:
167+
if (downcast<StyleRuleSupports>(*rule).conditionIsSupported())
168+
addChildRules(downcast<StyleRuleSupports>(*rule).childRules());
169+
return;
170170

171-
case StyleRuleType::Import:
172-
case StyleRuleType::Margin:
173-
case StyleRuleType::Namespace:
174-
case StyleRuleType::FontFeatureValuesBlock:
175-
return;
171+
case StyleRuleType::Import:
172+
case StyleRuleType::Margin:
173+
case StyleRuleType::Namespace:
174+
case StyleRuleType::FontFeatureValuesBlock:
175+
return;
176176

177-
case StyleRuleType::Unknown:
178-
case StyleRuleType::Charset:
179-
case StyleRuleType::Keyframe:
180-
ASSERT_NOT_REACHED();
181-
return;
177+
case StyleRuleType::Unknown:
178+
case StyleRuleType::Charset:
179+
case StyleRuleType::Keyframe:
180+
ASSERT_NOT_REACHED();
181+
return;
182182
}
183183
}
184184

@@ -391,15 +391,19 @@ void RuleSetBuilder::addMutatingRulesToResolver()
391391
continue;
392392
}
393393
if (is<StyleRuleCounterStyle>(rule)) {
394+
if (m_resolver->scopeType() == Resolver::ScopeType::ShadowTree)
395+
continue;
394396
auto& registry = m_resolver->document().styleScope().counterStyleRegistry();
395397
registry.addCounterStyle(downcast<StyleRuleCounterStyle>(rule.get()).descriptors());
396398
// FIXME: we probably need a cache solultion like fontSelector (invalidateMatchedDeclarations)
397399
// KeyFrame does it differently, search for keyframesRuleDidChange. (rdar://103018993)
398-
// FIXME: Use the shadow tree scope if applicable (or just skip). (rdar://30318695)
399400
continue;
400401
}
401402
if (is<StyleRuleProperty>(rule)) {
402-
// FIXME: Use the shadow tree scope if applicable (or just skip).
403+
// "A @property is invalid if it occurs in a stylesheet inside of a shadow tree, and must be ignored."
404+
// https://drafts.css-houdini.org/css-properties-values-api/#at-property-rule
405+
if (m_resolver->scopeType() == Resolver::ScopeType::ShadowTree)
406+
continue;
403407
auto& registry = m_resolver->document().styleScope().customPropertyRegistry();
404408
registry.registerFromStylesheet(downcast<StyleRuleProperty>(rule.get()).descriptor());
405409
continue;

Source/WebCore/style/StyleResolver.cpp

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include "CSSStyleSheet.h"
4242
#include "CachedResourceLoader.h"
4343
#include "CompositeOperation.h"
44+
#include "Document.h"
4445
#include "ElementRuleCollector.h"
4546
#include "Frame.h"
4647
#include "FrameSelection.h"
@@ -134,15 +135,21 @@ class Resolver::State {
134135
std::unique_ptr<RenderStyle> m_userAgentAppearanceStyle;
135136
};
136137

137-
Ref<Resolver> Resolver::create(Document& document)
138+
Ref<Resolver> Resolver::create(Document& document, ScopeType scopeType)
138139
{
139-
return adoptRef(*new Resolver(document));
140+
return adoptRef(*new Resolver(document, scopeType));
140141
}
141142

142-
Resolver::Resolver(Document& document)
143-
: m_ruleSets(*this)
144-
, m_document(document)
145-
, m_matchAuthorAndUserStyles(m_document.settings().authorAndUserStylesEnabled())
143+
Resolver::Resolver(Document& document, ScopeType scopeType)
144+
: m_document(document)
145+
, m_scopeType(scopeType)
146+
, m_ruleSets(*this)
147+
, m_matchAuthorAndUserStyles(settings().authorAndUserStylesEnabled())
148+
{
149+
initialize();
150+
}
151+
152+
void Resolver::initialize()
146153
{
147154
UserAgentStyle::initDefaultStyleSheet();
148155

@@ -152,34 +159,51 @@ Resolver::Resolver(Document& document)
152159
// document doesn't have documentElement
153160
// NOTE: this assumes that element that gets passed to styleForElement -call
154161
// is always from the document that owns the style selector
155-
FrameView* view = m_document.view();
162+
FrameView* view = document().view();
156163
if (view)
157164
m_mediaQueryEvaluator = MQ::MediaQueryEvaluator { view->mediaType() };
158165
else
159166
m_mediaQueryEvaluator = MQ::MediaQueryEvaluator { };
160167

161-
if (auto* documentElement = m_document.documentElement()) {
162-
m_rootDefaultStyle = styleForElement(*documentElement, { m_document.initialContainingBlockStyle() }, RuleMatchingBehavior::MatchOnlyUserAgentRules).style;
168+
if (auto* documentElement = document().documentElement()) {
169+
m_rootDefaultStyle = styleForElement(*documentElement, { document().initialContainingBlockStyle() }, RuleMatchingBehavior::MatchOnlyUserAgentRules).style;
163170
// Turn off assertion against font lookups during style resolver initialization. We may need root style font for media queries.
164-
m_document.fontSelector().incrementIsComputingRootStyleFont();
165-
m_rootDefaultStyle->fontCascade().update(&m_document.fontSelector());
171+
document().fontSelector().incrementIsComputingRootStyleFont();
172+
m_rootDefaultStyle->fontCascade().update(&document().fontSelector());
166173
m_rootDefaultStyle->fontCascade().primaryFont();
167-
m_document.fontSelector().decrementIsComputingRootStyleFont();
174+
document().fontSelector().decrementIsComputingRootStyleFont();
168175
}
169176

170177
if (m_rootDefaultStyle && view)
171-
m_mediaQueryEvaluator = MQ::MediaQueryEvaluator { view->mediaType(), m_document, m_rootDefaultStyle.get() };
178+
m_mediaQueryEvaluator = MQ::MediaQueryEvaluator { view->mediaType(), document(), m_rootDefaultStyle.get() };
172179

173180
m_ruleSets.resetAuthorStyle();
174181
m_ruleSets.resetUserAgentMediaQueryStyle();
175182
}
176183

184+
Resolver::~Resolver() = default;
185+
186+
Document& Resolver::document()
187+
{
188+
return *m_document;
189+
}
190+
191+
const Document& Resolver::document() const
192+
{
193+
return *m_document;
194+
}
195+
196+
const Settings& Resolver::settings() const
197+
{
198+
return document().settings();
199+
}
200+
177201
void Resolver::addCurrentSVGFontFaceRules()
178202
{
179-
if (m_document.svgExtensions()) {
180-
auto& svgFontFaceElements = m_document.svgExtensions()->svgFontFaceElements();
203+
if (document().svgExtensions()) {
204+
auto& svgFontFaceElements = document().svgExtensions()->svgFontFaceElements();
181205
for (auto& svgFontFaceElement : svgFontFaceElements)
182-
m_document.fontSelector().addFontFaceRule(svgFontFaceElement.fontFaceRule(), svgFontFaceElement.isInUserAgentShadowTree());
206+
document().fontSelector().addFontFaceRule(svgFontFaceElement.fontFaceRule(), svgFontFaceElement.isInUserAgentShadowTree());
183207
}
184208
}
185209

@@ -196,12 +220,7 @@ void Resolver::addKeyframeStyle(Ref<StyleRuleKeyframes>&& rule)
196220
{
197221
auto& animationName = rule->name();
198222
m_keyframesRuleMap.set(animationName, WTFMove(rule));
199-
m_document.keyframesRuleDidChange(animationName);
200-
}
201-
202-
Resolver::~Resolver()
203-
{
204-
RELEASE_ASSERT(!m_document.isResolvingTreeStyle());
223+
document().keyframesRuleDidChange(animationName);
205224
}
206225

207226
static inline bool isAtShadowBoundary(const Element& element)
@@ -212,7 +231,7 @@ static inline bool isAtShadowBoundary(const Element& element)
212231
BuilderContext Resolver::builderContext(const State& state)
213232
{
214233
return {
215-
m_document,
234+
document(),
216235
*state.parentStyle(),
217236
state.rootElementStyle(),
218237
state.element()
@@ -482,11 +501,11 @@ std::optional<ResolvedStyle> Resolver::styleForPseudoElement(const Element& elem
482501

483502
std::unique_ptr<RenderStyle> Resolver::styleForPage(int pageIndex)
484503
{
485-
auto* documentElement = m_document.documentElement();
504+
auto* documentElement = document().documentElement();
486505
if (!documentElement || !documentElement->renderStyle())
487506
return RenderStyle::createPtr();
488507

489-
auto state = State(*documentElement, m_document.initialContainingBlockStyle());
508+
auto state = State(*documentElement, document().initialContainingBlockStyle());
490509

491510
state.setStyle(RenderStyle::createPtr());
492511
state.style()->inheritFrom(*state.rootElementStyle());

0 commit comments

Comments
 (0)
0