commit cb8e73febc93037de0395400c98226aa2245e85e
parent 3ca670655367342124f1f1e9134cbd57205cd155
Author: David Shin <dshin@mozilla.com>
Date: Wed, 29 Oct 2025 17:33:56 +0000
Bug 1995774: Consider parent rules on nested declaration mutation. r=firefox-style-system-reviewers,jwatt,emilio
Differential Revision: https://phabricator.services.mozilla.com/D269646
Diffstat:
11 files changed, 202 insertions(+), 48 deletions(-)
diff --git a/layout/style/ServoBindings.h b/layout/style/ServoBindings.h
@@ -52,7 +52,8 @@ extern "C" {
uint32_t* column); \
void Servo_StyleSet_##type_##RuleChanged( \
const StylePerDocumentStyleData*, const Style##prefix_##type_##Rule*, \
- const StyleDomStyleSheet*, StyleRuleChangeKind); \
+ const StyleDomStyleSheet*, StyleRuleChangeKind, \
+ const nsTArray<StyleCssRuleRef>*); \
BASIC_RULE_FUNCS_WITHOUT_GETTER_WITH_PREFIX(type_##Rule, prefix_)
#define BASIC_RULE_FUNCS_LOCKED(type_) \
diff --git a/layout/style/ServoStyleSet.cpp b/layout/style/ServoStyleSet.cpp
@@ -992,16 +992,61 @@ void ServoStyleSet::RuleRemoved(StyleSheet& aSheet, css::Rule& aRule) {
RuleChangedInternal(aSheet, aRule, StyleRuleChangeKind::Removal);
}
+static Maybe<StyleCssRuleRef> ToRuleRef(css::Rule& aRule) {
+ switch (aRule.Type()) {
+#define CASE_FOR(constant_, type_) \
+ case StyleCssRuleType::constant_: \
+ return Some(StyleCssRuleRef::constant_( \
+ static_cast<dom::CSS##type_##Rule&>(aRule).Raw())); \
+ break;
+ CASE_FOR(CounterStyle, CounterStyle)
+ CASE_FOR(Style, Style)
+ CASE_FOR(Import, Import)
+ CASE_FOR(Media, Media)
+ CASE_FOR(Keyframes, Keyframes)
+ CASE_FOR(Margin, Margin)
+ CASE_FOR(FontFeatureValues, FontFeatureValues)
+ CASE_FOR(FontPaletteValues, FontPaletteValues)
+ CASE_FOR(FontFace, FontFace)
+ CASE_FOR(Page, Page)
+ CASE_FOR(Property, Property)
+ CASE_FOR(Document, MozDocument)
+ CASE_FOR(Supports, Supports)
+ CASE_FOR(LayerBlock, LayerBlock)
+ CASE_FOR(LayerStatement, LayerStatement)
+ CASE_FOR(Container, Container)
+ CASE_FOR(Scope, Scope)
+ CASE_FOR(StartingStyle, StartingStyle)
+ CASE_FOR(PositionTry, PositionTry)
+ CASE_FOR(NestedDeclarations, NestedDeclarations)
+ CASE_FOR(Namespace, Namespace)
+#undef CASE_FOR
+ case StyleCssRuleType::Keyframe:
+ // No equivalent.
+ break;
+ }
+ return Nothing{};
+}
+
void ServoStyleSet::RuleChangedInternal(StyleSheet& aSheet, css::Rule& aRule,
const StyleRuleChange& aChange) {
MOZ_ASSERT(aSheet.IsApplicable());
SetStylistStyleSheetsDirty();
+ nsTArray<StyleCssRuleRef> ancestors;
+
+ auto* parent = aRule.GetParentRule();
+ while (parent) {
+ if (const auto ref = ToRuleRef(*parent)) {
+ ancestors.AppendElement(*ref);
+ }
+ parent = parent->GetParentRule();
+ }
#define CASE_FOR(constant_, type_) \
case StyleCssRuleType::constant_: \
return Servo_StyleSet_##constant_##RuleChanged( \
mRawData.get(), static_cast<dom::CSS##type_##Rule&>(aRule).Raw(), \
- &aSheet, aChange.mKind);
+ &aSheet, aChange.mKind, &ancestors);
switch (aRule.Type()) {
CASE_FOR(CounterStyle, CounterStyle)
CASE_FOR(Style, Style)
diff --git a/servo/components/style/gecko/arc_types.rs b/servo/components/style/gecko/arc_types.rs
@@ -14,10 +14,13 @@ use crate::properties::{ComputedValues, PropertyDeclarationBlock};
use crate::shared_lock::Locked;
use crate::stylesheets::keyframes_rule::Keyframe;
use crate::stylesheets::{
- ContainerRule, CounterStyleRule, CssRules, DocumentRule, FontFaceRule, FontFeatureValuesRule,
- FontPaletteValuesRule, ImportRule, KeyframesRule, LayerBlockRule, LayerStatementRule,
- MarginRule, MediaRule, NamespaceRule, NestedDeclarationsRule, PageRule, PositionTryRule,
- PropertyRule, ScopeRule, StartingStyleRule, StyleRule, StylesheetContents, SupportsRule,
+ ContainerRule, CssRules, DocumentRule, FontFeatureValuesRule, FontPaletteValuesRule,
+ LayerBlockRule, LayerStatementRule, MarginRule, MediaRule, NamespaceRule, PropertyRule,
+ ScopeRule, StartingStyleRule, StylesheetContents, SupportsRule,
+};
+pub use crate::stylesheets::{
+ LockedCounterStyleRule, LockedFontFaceRule, LockedImportRule, LockedKeyframesRule,
+ LockedNestedDeclarationsRule, LockedPageRule, LockedPositionTryRule, LockedStyleRule,
};
use servo_arc::Arc;
@@ -55,14 +58,12 @@ impl_locked_arc_ffi!(
Servo_DeclarationBlock_AddRef,
Servo_DeclarationBlock_Release
);
-impl_locked_arc_ffi!(
- StyleRule,
+impl_simple_arc_ffi!(
LockedStyleRule,
Servo_StyleRule_AddRef,
Servo_StyleRule_Release
);
-impl_locked_arc_ffi!(
- ImportRule,
+impl_simple_arc_ffi!(
LockedImportRule,
Servo_ImportRule_AddRef,
Servo_ImportRule_Release
@@ -73,8 +74,7 @@ impl_locked_arc_ffi!(
Servo_Keyframe_AddRef,
Servo_Keyframe_Release
);
-impl_locked_arc_ffi!(
- KeyframesRule,
+impl_simple_arc_ffi!(
LockedKeyframesRule,
Servo_KeyframesRule_AddRef,
Servo_KeyframesRule_Release
@@ -106,8 +106,7 @@ impl_simple_arc_ffi!(
Servo_MarginRule_AddRef,
Servo_MarginRule_Release
);
-impl_locked_arc_ffi!(
- PageRule,
+impl_simple_arc_ffi!(
LockedPageRule,
Servo_PageRule_AddRef,
Servo_PageRule_Release
@@ -142,14 +141,12 @@ impl_simple_arc_ffi!(
Servo_FontPaletteValuesRule_AddRef,
Servo_FontPaletteValuesRule_Release
);
-impl_locked_arc_ffi!(
- FontFaceRule,
+impl_simple_arc_ffi!(
LockedFontFaceRule,
Servo_FontFaceRule_AddRef,
Servo_FontFaceRule_Release
);
-impl_locked_arc_ffi!(
- CounterStyleRule,
+impl_simple_arc_ffi!(
LockedCounterStyleRule,
Servo_CounterStyleRule_AddRef,
Servo_CounterStyleRule_Release
@@ -182,14 +179,12 @@ impl_simple_arc_ffi!(
Servo_StartingStyleRule_Release
);
-impl_locked_arc_ffi!(
- PositionTryRule,
+impl_simple_arc_ffi!(
LockedPositionTryRule,
Servo_PositionTryRule_AddRef,
Servo_PositionTryRule_Release
);
-impl_locked_arc_ffi!(
- NestedDeclarationsRule,
+impl_simple_arc_ffi!(
LockedNestedDeclarationsRule,
Servo_NestedDeclarationsRule_AddRef,
Servo_NestedDeclarationsRule_Release
diff --git a/servo/components/style/invalidation/stylesheets.rs b/servo/components/style/invalidation/stylesheets.rs
@@ -15,7 +15,7 @@ use crate::media_queries::Device;
use crate::selector_parser::{SelectorImpl, Snapshot, SnapshotMap};
use crate::shared_lock::SharedRwLockReadGuard;
use crate::simple_buckets_map::SimpleBucketsMap;
-use crate::stylesheets::{CssRule, StylesheetInDocument};
+use crate::stylesheets::{CssRule, CssRuleRef, StylesheetInDocument};
use crate::stylesheets::{EffectiveRules, EffectiveRulesIterator};
use crate::values::AtomIdent;
use crate::LocalName as SelectorLocalName;
@@ -153,6 +153,8 @@ impl StylesheetInvalidationSet {
device,
quirks_mode,
/* is_generic_change = */ false,
+ // Note(dshin): Technically, the iterator should provide the ancestor chain as it traverses down, but it shouldn't make a difference.
+ &[],
);
if self.fully_invalid {
break;
@@ -525,6 +527,7 @@ impl StylesheetInvalidationSet {
device: &Device,
quirks_mode: QuirksMode,
change_kind: RuleChangeKind,
+ ancestors: &[CssRuleRef],
) where
S: StylesheetInDocument,
{
@@ -538,17 +541,34 @@ impl StylesheetInvalidationSet {
return; // Nothing to do here.
}
+ if ancestors
+ .iter()
+ .any(|r| !EffectiveRules::is_effective(guard, device, quirks_mode, r))
+ {
+ debug!(" > Ancestor rules not effective");
+ return;
+ }
+
// If the change is generic, we don't have the old rule information to know e.g., the old
// media condition, or the old selector text, so we might need to invalidate more
// aggressively. That only applies to the changed rules, for other rules we can just
// collect invalidations as normal.
let is_generic_change = change_kind == RuleChangeKind::Generic;
- self.collect_invalidations_for_rule(rule, guard, device, quirks_mode, is_generic_change);
+ self.collect_invalidations_for_rule(
+ rule,
+ guard,
+ device,
+ quirks_mode,
+ is_generic_change,
+ ancestors,
+ );
if self.fully_invalid {
return;
}
- if !is_generic_change && !EffectiveRules::is_effective(guard, device, quirks_mode, rule) {
+ if !is_generic_change
+ && !EffectiveRules::is_effective(guard, device, quirks_mode, &rule.into())
+ {
return;
}
@@ -560,6 +580,8 @@ impl StylesheetInvalidationSet {
device,
quirks_mode,
/* is_generic_change = */ false,
+ // Note(dshin): Technically, the iterator should provide the ancestor chain as it traverses down, which sould be appended to `ancestors`, but it shouldn't matter.
+ &[],
);
if self.fully_invalid {
break;
@@ -575,6 +597,7 @@ impl StylesheetInvalidationSet {
device: &Device,
quirks_mode: QuirksMode,
is_generic_change: bool,
+ ancestors: &[CssRuleRef],
) {
use crate::stylesheets::CssRule::*;
debug!("StylesheetInvalidationSet::collect_invalidations_for_rule");
@@ -600,7 +623,9 @@ impl StylesheetInvalidationSet {
}
},
NestedDeclarations(..) => {
- // Our containing style rule would handle invalidation for us.
+ if ancestors.iter().any(|r| matches!(r, CssRuleRef::Scope(_))) {
+ self.fully_invalid = true;
+ }
},
Namespace(..) => {
// It's not clear what handling changes for this correctly would
diff --git a/servo/components/style/stylesheet_set.rs b/servo/components/style/stylesheet_set.rs
@@ -9,7 +9,7 @@ use crate::invalidation::stylesheets::{RuleChangeKind, StylesheetInvalidationSet
use crate::media_queries::Device;
use crate::selector_parser::SnapshotMap;
use crate::shared_lock::SharedRwLockReadGuard;
-use crate::stylesheets::{CssRule, Origin, OriginSet, PerOrigin, StylesheetInDocument};
+use crate::stylesheets::{CssRule, CssRuleRef, Origin, OriginSet, PerOrigin, StylesheetInDocument};
use std::mem;
/// Entry for a StylesheetSet.
@@ -403,6 +403,7 @@ macro_rules! sheet_set_methods {
rule: &CssRule,
guard: &SharedRwLockReadGuard,
change_kind: RuleChangeKind,
+ ancestors: &[CssRuleRef],
) {
if let Some(device) = device {
let quirks_mode = device.quirks_mode();
@@ -413,6 +414,7 @@ macro_rules! sheet_set_methods {
device,
quirks_mode,
change_kind,
+ ancestors,
);
}
diff --git a/servo/components/style/stylesheets/mod.rs b/servo/components/style/stylesheets/mod.rs
@@ -419,6 +419,79 @@ impl CssRule {
}
}
+// These aliases are required on Gecko side to avoid generating bindings for `Locked`.
+/// Alias for a locked style rule.
+pub type LockedStyleRule = Locked<StyleRule>;
+/// Alias for a locked import rule.
+pub type LockedImportRule = Locked<ImportRule>;
+/// Alias for a locked font-face rule.
+pub type LockedFontFaceRule = Locked<FontFaceRule>;
+/// Alias for a locked counter-style rule.
+pub type LockedCounterStyleRule = Locked<CounterStyleRule>;
+/// Alias for a locked keyframes rule.
+pub type LockedKeyframesRule = Locked<KeyframesRule>;
+/// Alias for a locked page rule.
+pub type LockedPageRule = Locked<PageRule>;
+/// Alias for a locked position-try rule.
+pub type LockedPositionTryRule = Locked<PositionTryRule>;
+/// Alias for a locked nested declarations rule.
+pub type LockedNestedDeclarationsRule = Locked<NestedDeclarationsRule>;
+
+/// A CSS rule reference. Should mirror `CssRule`.
+#[repr(C)]
+#[allow(missing_docs)]
+pub enum CssRuleRef<'a> {
+ Style(&'a LockedStyleRule),
+ Namespace(&'a NamespaceRule),
+ Import(&'a LockedImportRule),
+ Media(&'a MediaRule),
+ Container(&'a ContainerRule),
+ FontFace(&'a LockedFontFaceRule),
+ FontFeatureValues(&'a FontFeatureValuesRule),
+ FontPaletteValues(&'a FontPaletteValuesRule),
+ CounterStyle(&'a LockedCounterStyleRule),
+ Keyframes(&'a LockedKeyframesRule),
+ Margin(&'a MarginRule),
+ Supports(&'a SupportsRule),
+ Page(&'a LockedPageRule),
+ Property(&'a PropertyRule),
+ Document(&'a DocumentRule),
+ LayerBlock(&'a LayerBlockRule),
+ LayerStatement(&'a LayerStatementRule),
+ Scope(&'a ScopeRule),
+ StartingStyle(&'a StartingStyleRule),
+ PositionTry(&'a LockedPositionTryRule),
+ NestedDeclarations(&'a LockedNestedDeclarationsRule),
+}
+
+impl<'a> From<&'a CssRule> for CssRuleRef<'a> {
+ fn from(value: &'a CssRule) -> Self {
+ match value {
+ CssRule::Style(r) => CssRuleRef::Style(r.as_ref()),
+ CssRule::Namespace(r) => CssRuleRef::Namespace(r.as_ref()),
+ CssRule::Import(r) => CssRuleRef::Import(r.as_ref()),
+ CssRule::Media(r) => CssRuleRef::Media(r.as_ref()),
+ CssRule::Container(r) => CssRuleRef::Container(r.as_ref()),
+ CssRule::FontFace(r) => CssRuleRef::FontFace(r.as_ref()),
+ CssRule::FontFeatureValues(r) => CssRuleRef::FontFeatureValues(r.as_ref()),
+ CssRule::FontPaletteValues(r) => CssRuleRef::FontPaletteValues(r.as_ref()),
+ CssRule::CounterStyle(r) => CssRuleRef::CounterStyle(r.as_ref()),
+ CssRule::Keyframes(r) => CssRuleRef::Keyframes(r.as_ref()),
+ CssRule::Margin(r) => CssRuleRef::Margin(r.as_ref()),
+ CssRule::Supports(r) => CssRuleRef::Supports(r.as_ref()),
+ CssRule::Page(r) => CssRuleRef::Page(r.as_ref()),
+ CssRule::Property(r) => CssRuleRef::Property(r.as_ref()),
+ CssRule::Document(r) => CssRuleRef::Document(r.as_ref()),
+ CssRule::LayerBlock(r) => CssRuleRef::LayerBlock(r.as_ref()),
+ CssRule::LayerStatement(r) => CssRuleRef::LayerStatement(r.as_ref()),
+ CssRule::Scope(r) => CssRuleRef::Scope(r.as_ref()),
+ CssRule::StartingStyle(r) => CssRuleRef::StartingStyle(r.as_ref()),
+ CssRule::PositionTry(r) => CssRuleRef::PositionTry(r.as_ref()),
+ CssRule::NestedDeclarations(r) => CssRuleRef::NestedDeclarations(r.as_ref()),
+ }
+ }
+}
+
/// https://drafts.csswg.org/cssom-1/#dom-cssrule-type
#[allow(missing_docs)]
#[derive(Clone, Copy, Debug, Eq, FromPrimitive, PartialEq)]
diff --git a/servo/components/style/stylesheets/rules_iterator.rs b/servo/components/style/stylesheets/rules_iterator.rs
@@ -7,7 +7,7 @@
use crate::context::QuirksMode;
use crate::media_queries::Device;
use crate::shared_lock::SharedRwLockReadGuard;
-use crate::stylesheets::{CssRule, DocumentRule, ImportRule, MediaRule, SupportsRule};
+use crate::stylesheets::{CssRule, CssRuleRef, DocumentRule, ImportRule, MediaRule, SupportsRule};
use smallvec::SmallVec;
use std::slice;
@@ -214,20 +214,20 @@ impl EffectiveRules {
guard: &SharedRwLockReadGuard,
device: &Device,
quirks_mode: QuirksMode,
- rule: &CssRule,
+ rule: &CssRuleRef,
) -> bool {
match *rule {
- CssRule::Import(ref import_rule) => {
+ CssRuleRef::Import(import_rule) => {
let import_rule = import_rule.read_with(guard);
Self::process_import(guard, device, quirks_mode, import_rule)
},
- CssRule::Document(ref doc_rule) => {
+ CssRuleRef::Document(doc_rule) => {
Self::process_document(guard, device, quirks_mode, doc_rule)
},
- CssRule::Media(ref media_rule) => {
+ CssRuleRef::Media(media_rule) => {
Self::process_media(guard, device, quirks_mode, media_rule)
},
- CssRule::Supports(ref supports_rule) => {
+ CssRuleRef::Supports(supports_rule) => {
Self::process_supports(guard, device, quirks_mode, supports_rule)
},
_ => true,
diff --git a/servo/components/style/stylist.rs b/servo/components/style/stylist.rs
@@ -52,8 +52,7 @@ use crate::stylesheets::{
PagePseudoClassFlags, PositionTryRule,
};
use crate::stylesheets::{
- CssRule, EffectiveRulesIterator, Origin, OriginSet, PageRule, PerOrigin, PerOriginIter,
- StylesheetContents, StylesheetInDocument,
+ CssRule, CssRuleRef, EffectiveRulesIterator, Origin, OriginSet, PageRule, PerOrigin, PerOriginIter, StylesheetContents, StylesheetInDocument
};
use crate::values::computed::DashedIdentAndOrTryTactic;
use crate::values::specified::position::PositionTryFallbacksTryTactic;
@@ -977,9 +976,16 @@ impl Stylist {
rule: &CssRule,
guard: &SharedRwLockReadGuard,
change_kind: RuleChangeKind,
+ ancestors: &[CssRuleRef],
) {
- self.stylesheets
- .rule_changed(Some(&self.device), sheet, rule, guard, change_kind)
+ self.stylesheets.rule_changed(
+ Some(&self.device),
+ sheet,
+ rule,
+ guard,
+ change_kind,
+ ancestors,
+ )
}
/// Appends a new stylesheet to the current set.
diff --git a/servo/ports/geckolib/cbindgen.toml b/servo/ports/geckolib/cbindgen.toml
@@ -84,6 +84,14 @@ exclude = [
"NS_LogDtor",
"SelectorList",
"AuthorStyles",
+ "LockedStyleRule",
+ "LockedImportRule",
+ "LockedFontFaceRule",
+ "LockedCounterStyleRule",
+ "LockedKeyframesRule",
+ "LockedPageRule",
+ "LockedPositionTryRule",
+ "LockedNestedDeclarationsRule",
]
include = [
"AnchorName",
@@ -205,6 +213,7 @@ include = [
"BorderRadius",
"ColorScheme",
"ColumnCount",
+ "CssRuleRef",
"NonNegativeLengthOrNumberRect",
"Perspective",
"ZIndex",
diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs
@@ -135,13 +135,13 @@ use style::stylesheets::keyframes_rule::{Keyframe, KeyframeSelector, KeyframesSt
use style::stylesheets::scope_rule::{ImplicitScopeRoot, ScopeRootCandidate, ScopeSubjectMap};
use style::stylesheets::supports_rule::parse_condition_or_declaration;
use style::stylesheets::{
- AllowImportRules, ContainerRule, CounterStyleRule, CssRule, CssRuleType, CssRuleTypes,
- CssRules, DocumentRule, FontFaceRule, FontFeatureValuesRule, FontPaletteValuesRule, ImportRule,
- KeyframesRule, LayerBlockRule, LayerStatementRule, MarginRule, MediaRule, NamespaceRule,
- NestedDeclarationsRule, Origin, OriginSet, PagePseudoClassFlags, PageRule, PositionTryRule,
- PropertyRule, SanitizationData, SanitizationKind, ScopeRule, StartingStyleRule, StyleRule,
- StylesheetContents, StylesheetInDocument, StylesheetLoader as StyleStylesheetLoader,
- SupportsRule, UrlExtraData,
+ AllowImportRules, ContainerRule, CounterStyleRule, CssRule, CssRuleRef, CssRuleType,
+ CssRuleTypes, CssRules, DocumentRule, FontFaceRule, FontFeatureValuesRule,
+ FontPaletteValuesRule, ImportRule, KeyframesRule, LayerBlockRule, LayerStatementRule,
+ MarginRule, MediaRule, NamespaceRule, NestedDeclarationsRule, Origin, OriginSet,
+ PagePseudoClassFlags, PageRule, PositionTryRule, PropertyRule, SanitizationData,
+ SanitizationKind, ScopeRule, StartingStyleRule, StyleRule, StylesheetContents,
+ StylesheetInDocument, StylesheetLoader as StyleStylesheetLoader, SupportsRule, UrlExtraData,
};
use style::stylist::{
add_size_of_ua_cache, replace_parent_selector_with_implicit_scope, scope_root_candidates,
@@ -2359,6 +2359,7 @@ macro_rules! impl_basic_rule_funcs {
rule: &$maybe_locked_rule_type,
sheet: &DomStyleSheet,
change_kind: RuleChangeKind,
+ ancestors: &nsTArray<CssRuleRef>,
) {
let mut data = styleset.borrow_mut();
let data = &mut *data;
@@ -2368,7 +2369,7 @@ macro_rules! impl_basic_rule_funcs {
// but it's probably not a huge deal.
let rule = unsafe { CssRule::$name(Arc::from_raw_addrefed(rule)) };
let sheet = unsafe { GeckoStyleSheet::new(sheet) };
- data.stylist.rule_changed(&sheet, &rule, &guard, change_kind);
+ data.stylist.rule_changed(&sheet, &rule, &guard, change_kind, ancestors.as_slice());
}
impl_basic_rule_funcs_without_getter! {
diff --git a/testing/web-platform/meta/css/css-cascade/scope-nesting.html.ini b/testing/web-platform/meta/css/css-cascade/scope-nesting.html.ini
@@ -1,3 +0,0 @@
-[scope-nesting.html]
- [Insert a CSSNestedDeclarations rule directly in top-level @scope]
- expected: FAIL
