From e7e11669a6213bb06eb6d2836b31d3cd5bdc32a5 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Thu, 4 Apr 2024 19:39:28 +0000 Subject: [PATCH] Bug 1889496 - Part 1: Parse @starting-style rule. r=layout-reviewers,firefox-style-system-reviewers,emilio We introduce this rule and parse it in this patch. Also, fix some wpt expectations for ERROR. We will introduce CSSStartingStyleRule in the following patch, and test it there. Differential Revision: https://phabricator.services.mozilla.com/D206428 --- layout/style/ServoCSSRuleList.cpp | 6 ++ layout/style/ServoStyleSet.cpp | 3 + modules/libpref/init/StaticPrefList.yaml | 7 +++ .../style/invalidation/stylesheets.rs | 2 +- servo/components/style/stylesheets/mod.rs | 15 ++++- .../style/stylesheets/rule_parser.rs | 19 ++++++- .../style/stylesheets/rules_iterator.rs | 3 +- .../style/stylesheets/starting_style_rule.rs | 57 +++++++++++++++++++ .../style/stylesheets/stylesheet.rs | 3 +- servo/components/style/stylist.rs | 3 +- .../meta/css/css-transitions/__dir__.ini | 2 +- .../starting-style-cascade.html.ini | 9 ++- ...tarting-style-name-defining-rules.html.ini | 6 -- .../starting-style-rule-none.html.ini | 3 +- 14 files changed, 123 insertions(+), 15 deletions(-) create mode 100644 servo/components/style/stylesheets/starting_style_rule.rs delete mode 100644 testing/web-platform/meta/css/css-transitions/starting-style-name-defining-rules.html.ini diff --git a/layout/style/ServoCSSRuleList.cpp b/layout/style/ServoCSSRuleList.cpp index fc966f78537e..98fe6f94d1dd 100644 --- a/layout/style/ServoCSSRuleList.cpp +++ b/layout/style/ServoCSSRuleList.cpp @@ -106,6 +106,9 @@ css::Rule* ServoCSSRuleList::GetRule(uint32_t aIndex) { case StyleCssRuleType::Keyframe: MOZ_ASSERT_UNREACHABLE("keyframe rule cannot be here"); return nullptr; + case StyleCssRuleType::StartingStyle: + // TODO: Implement this in the following patch. + return nullptr; case StyleCssRuleType::Margin: // Margin rules not implemented yet, see bug 1864737 return nullptr; @@ -288,6 +291,9 @@ void ServoCSSRuleList::SetRawContents(RefPtr aNewRules, RULE_CASE_UNLOCKED(LayerStatement, LayerStatement) RULE_CASE_UNLOCKED(Container, Container) RULE_CASE_UNLOCKED(Scope, Scope) + case StyleCssRuleType::StartingStyle: + // TODO: Implement this in the following patch. + break; case StyleCssRuleType::Keyframe: MOZ_ASSERT_UNREACHABLE("keyframe rule cannot be here"); break; diff --git a/layout/style/ServoStyleSet.cpp b/layout/style/ServoStyleSet.cpp index 7bbfeec78409..3c83a1b8fd05 100644 --- a/layout/style/ServoStyleSet.cpp +++ b/layout/style/ServoStyleSet.cpp @@ -1004,6 +1004,9 @@ void ServoStyleSet::RuleChangedInternal(StyleSheet& aSheet, css::Rule& aRule, CASE_FOR(LayerStatement, LayerStatement) CASE_FOR(Container, Container) CASE_FOR(Scope, Scope) + case StyleCssRuleType::StartingStyle: + // TODO: Implement this in the following patch. + break; // @namespace can only be inserted / removed when there are only other // @namespace and @import rules, and can't be mutated. case StyleCssRuleType::Namespace: diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 5924588279c1..94691df8f2fa 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -8356,6 +8356,13 @@ mirror: always rust: true +# Whether @starting-style is enabled? +- name: layout.css.starting-style-at-rules.enabled + type: RelaxedAtomicBool + value: false + mirror: always + rust: true + # Should we look for counter ancestor scopes first? - name: layout.css.counter-ancestor-scope.enabled type: bool diff --git a/servo/components/style/invalidation/stylesheets.rs b/servo/components/style/invalidation/stylesheets.rs index 82b87dc90438..48ef8c1b5d06 100644 --- a/servo/components/style/invalidation/stylesheets.rs +++ b/servo/components/style/invalidation/stylesheets.rs @@ -616,7 +616,7 @@ impl StylesheetInvalidationSet { return self.invalidate_fully(); }, Document(..) | Import(..) | Media(..) | Supports(..) | Container(..) | - LayerBlock(..) => { + LayerBlock(..) | StartingStyle(..) => { // Do nothing, relevant nested rules are visited as part of rule iteration. }, FontFace(..) => { diff --git a/servo/components/style/stylesheets/mod.rs b/servo/components/style/stylesheets/mod.rs index 222f4a737250..6bb8e19fb49c 100644 --- a/servo/components/style/stylesheets/mod.rs +++ b/servo/components/style/stylesheets/mod.rs @@ -23,6 +23,7 @@ mod property_rule; mod rule_list; mod rule_parser; mod rules_iterator; +mod starting_style_rule; mod style_rule; mod stylesheet; pub mod supports_rule; @@ -70,6 +71,7 @@ pub use self::rules_iterator::{AllRules, EffectiveRules}; pub use self::rules_iterator::{ EffectiveRulesIterator, NestedRuleIterationCondition, RulesIterator, }; +pub use self::starting_style_rule::StartingStyleRule; pub use self::style_rule::StyleRule; pub use self::scope_rule::ScopeRule; pub use self::stylesheet::{AllowImportRules, SanitizationData, SanitizationKind}; @@ -272,6 +274,7 @@ pub enum CssRule { LayerBlock(Arc), LayerStatement(Arc), Scope(Arc), + StartingStyle(Arc), } impl CssRule { @@ -316,6 +319,9 @@ impl CssRule { CssRule::Document(ref arc) => { arc.unconditional_shallow_size_of(ops) + arc.size_of(guard, ops) }, + CssRule::StartingStyle(ref arc) => { + arc.unconditional_shallow_size_of(ops) + arc.size_of(guard, ops) + } // TODO(emilio): Add memory reporting for these rules. CssRule::LayerBlock(_) | CssRule::LayerStatement(_) => 0, CssRule::Scope(ref rule) => { @@ -360,6 +366,8 @@ pub enum CssRuleType { // 20 is an arbitrary number to use for Property. Property = 20, Scope = 21, + // https://drafts.csswg.org/css-transitions-2/#the-cssstartingstylerule-interface + StartingStyle = 22, } impl CssRuleType { @@ -448,6 +456,7 @@ impl CssRule { CssRule::LayerStatement(_) => CssRuleType::LayerStatement, CssRule::Container(_) => CssRuleType::Container, CssRule::Scope(_) => CssRuleType::Scope, + CssRule::StartingStyle(_) => CssRuleType::StartingStyle, } } @@ -585,7 +594,10 @@ impl DeepCloneWithLock for CssRule { }, CssRule::Scope(ref arc) => { CssRule::Scope(Arc::new(arc.deep_clone_with_lock(lock, guard, params))) - } + }, + CssRule::StartingStyle(ref arc) => { + CssRule::StartingStyle(Arc::new(arc.deep_clone_with_lock(lock, guard, params))) + }, } } } @@ -612,6 +624,7 @@ impl ToCssWithGuard for CssRule { CssRule::LayerStatement(ref rule) => rule.to_css(guard, dest), CssRule::Container(ref rule) => rule.to_css(guard, dest), CssRule::Scope(ref rule) => rule.to_css(guard, dest), + CssRule::StartingStyle(ref rule) => rule.to_css(guard, dest), } } } diff --git a/servo/components/style/stylesheets/rule_parser.rs b/servo/components/style/stylesheets/rule_parser.rs index 3252eef7bdc3..634f7c1af3da 100644 --- a/servo/components/style/stylesheets/rule_parser.rs +++ b/servo/components/style/stylesheets/rule_parser.rs @@ -24,6 +24,7 @@ use crate::stylesheets::import_rule::{ImportLayer, ImportRule, ImportSupportsCon use crate::stylesheets::keyframes_rule::parse_keyframe_list; use crate::stylesheets::layer_rule::{LayerBlockRule, LayerName, LayerStatementRule}; use crate::stylesheets::scope_rule::{ScopeBounds, ScopeRule}; +use crate::stylesheets::starting_style_rule::StartingStyleRule; use crate::stylesheets::supports_rule::SupportsCondition; use crate::stylesheets::{ AllowImportRules, CorsMode, CssRule, CssRuleType, CssRuleTypes, CssRules, DocumentRule, @@ -236,6 +237,8 @@ pub enum AtRulePrelude { Layer(Vec), /// A @scope rule prelude. Scope(ScopeBounds), + /// A @starting-style prelude. + StartingStyle, } impl AtRulePrelude { @@ -257,6 +260,7 @@ impl AtRulePrelude { Self::Namespace(..) => "namespace", Self::Layer(..) => "layer", Self::Scope(..) => "scope", + Self::StartingStyle => "starting-style", } } } @@ -525,7 +529,8 @@ impl<'a, 'i> NestedRuleParser<'a, 'i> { AtRulePrelude::Container(..) | AtRulePrelude::Document(..) | AtRulePrelude::Layer(..) | - AtRulePrelude::Scope(..) => true, + AtRulePrelude::Scope(..) | + AtRulePrelude::StartingStyle => true, AtRulePrelude::Namespace(..) | AtRulePrelude::FontFace | @@ -722,6 +727,9 @@ impl<'a, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'i> { let bounds = ScopeBounds::parse(&self.context, input, self.in_style_rule()); AtRulePrelude::Scope(bounds) }, + "starting-style" if static_prefs::pref!("layout.css.starting-style-at-rules.enabled") => { + AtRulePrelude::StartingStyle + }, _ => { if static_prefs::pref!("layout.css.margin-rules.enabled") { if let Some(margin_rule_type) = MarginRuleType::match_name(&name) { @@ -898,6 +906,15 @@ impl<'a, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'i> { source_location, })) }, + AtRulePrelude::StartingStyle => { + let source_location = start.source_location(); + CssRule::StartingStyle(Arc::new(StartingStyleRule { + rules: self + .parse_nested(input, CssRuleType::StartingStyle) + .into_rules(self.shared_lock, source_location), + source_location, + })) + }, }; self.rules.push(rule); Ok(()) diff --git a/servo/components/style/stylesheets/rules_iterator.rs b/servo/components/style/stylesheets/rules_iterator.rs index 1ad95cfdc3f8..60f3df1ea9dc 100644 --- a/servo/components/style/stylesheets/rules_iterator.rs +++ b/servo/components/style/stylesheets/rules_iterator.rs @@ -116,7 +116,8 @@ where Some(supports_rule.rules.read_with(guard).0.iter()) }, CssRule::LayerBlock(ref layer_rule) => Some(layer_rule.rules.read_with(guard).0.iter()), - CssRule::Scope(ref rule) => Some(rule.rules.read_with(guard).0.iter()) + CssRule::Scope(ref rule) => Some(rule.rules.read_with(guard).0.iter()), + CssRule::StartingStyle(ref rule) => Some(rule.rules.read_with(guard).0.iter()), } } } diff --git a/servo/components/style/stylesheets/starting_style_rule.rs b/servo/components/style/stylesheets/starting_style_rule.rs new file mode 100644 index 000000000000..3c2627582b31 --- /dev/null +++ b/servo/components/style/stylesheets/starting_style_rule.rs @@ -0,0 +1,57 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +//! before-change style: the `@starting-style` rules. +//! https://drafts.csswg.org/css-transitions-2/#defining-before-change-style + +use crate::shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked}; +use crate::shared_lock::{SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard}; +use crate::str::CssStringWriter; +use crate::stylesheets::CssRules; +use cssparser::SourceLocation; +use malloc_size_of::{MallocSizeOfOps, MallocUnconditionalShallowSizeOf}; +use servo_arc::Arc; +use std::fmt::{self, Debug, Write}; + +/// A [`@starting-style`][starting-style] rule. +/// +/// [starting-style]: https://drafts.csswg.org/css-transitions-2/#at-ruledef-starting-style +#[derive(Debug, ToShmem)] +pub struct StartingStyleRule { + /// The nested rules to this starting-style rule. + pub rules: Arc>, + /// The source position where this starting-style rule was found. + pub source_location: SourceLocation, +} + +impl StartingStyleRule { + /// Measure heap usage. + #[cfg(feature = "gecko")] + pub fn size_of(&self, guard: &SharedRwLockReadGuard, ops: &mut MallocSizeOfOps) -> usize { + self.rules.unconditional_shallow_size_of(ops) + + self.rules.read_with(guard).size_of(guard, ops) + } +} + +impl ToCssWithGuard for StartingStyleRule { + fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result { + dest.write_str("@starting-style")?; + self.rules.read_with(guard).to_css_block(guard, dest) + } +} + +impl DeepCloneWithLock for StartingStyleRule { + fn deep_clone_with_lock( + &self, + lock: &SharedRwLock, + guard: &SharedRwLockReadGuard, + params: &DeepCloneParams, + ) -> Self { + let rules = self.rules.read_with(guard); + StartingStyleRule { + rules: Arc::new(lock.wrap(rules.deep_clone_with_lock(lock, guard, params))), + source_location: self.source_location.clone(), + } + } +} diff --git a/servo/components/style/stylesheets/stylesheet.rs b/servo/components/style/stylesheets/stylesheet.rs index a65b3f748441..84d84b0d4627 100644 --- a/servo/components/style/stylesheets/stylesheet.rs +++ b/servo/components/style/stylesheets/stylesheet.rs @@ -336,7 +336,8 @@ impl SanitizationKind { CssRule::LayerBlock(..) | // TODO(dshin): Same comment as Layer applies - shouldn't give away // something like display size - erring on the side of "safe" for now. - CssRule::Scope(..) => false, + CssRule::Scope(..) | + CssRule::StartingStyle(..) => false, CssRule::FontFace(..) | CssRule::Namespace(..) | CssRule::Style(..) => true, diff --git a/servo/components/style/stylist.rs b/servo/components/style/stylist.rs index 4afb9414484e..06cbe6276b61 100644 --- a/servo/components/style/stylist.rs +++ b/servo/components/style/stylist.rs @@ -3260,7 +3260,8 @@ impl CascadeData { CssRule::LayerStatement(..) | CssRule::FontPaletteValues(..) | CssRule::FontFeatureValues(..) | - CssRule::Scope(..) => { + CssRule::Scope(..) | + CssRule::StartingStyle(..) => { // Not affected by device changes. continue; }, diff --git a/testing/web-platform/meta/css/css-transitions/__dir__.ini b/testing/web-platform/meta/css/css-transitions/__dir__.ini index d477628216e7..f13aa0c90559 100644 --- a/testing/web-platform/meta/css/css-transitions/__dir__.ini +++ b/testing/web-platform/meta/css/css-transitions/__dir__.ini @@ -1 +1 @@ -prefs: [dom.animations-api.compositing.enabled:true, dom.animations-api.timelines.enabled:true, layout.css.marker.restricted:false] +prefs: [dom.animations-api.compositing.enabled:true, dom.animations-api.timelines.enabled:true, layout.css.marker.restricted:false, layout.css.starting-style-at-rules.enabled:true] diff --git a/testing/web-platform/meta/css/css-transitions/starting-style-cascade.html.ini b/testing/web-platform/meta/css/css-transitions/starting-style-cascade.html.ini index c40d07887298..a494b94b2da9 100644 --- a/testing/web-platform/meta/css/css-transitions/starting-style-cascade.html.ini +++ b/testing/web-platform/meta/css/css-transitions/starting-style-cascade.html.ini @@ -1,2 +1,9 @@ [starting-style-cascade.html] - expected: ERROR + [@starting-style with higher specificity] + expected: FAIL + + [Starting style inheriting from parent's after-change style] + expected: FAIL + + [Starting style inheriting from parent's after-change style while parent transitioning] + expected: FAIL diff --git a/testing/web-platform/meta/css/css-transitions/starting-style-name-defining-rules.html.ini b/testing/web-platform/meta/css/css-transitions/starting-style-name-defining-rules.html.ini deleted file mode 100644 index 92a4cca5c7bc..000000000000 --- a/testing/web-platform/meta/css/css-transitions/starting-style-name-defining-rules.html.ini +++ /dev/null @@ -1,6 +0,0 @@ -[starting-style-name-defining-rules.html] - [@keyframes and @layer in @starting-style apply] - expected: FAIL - - [Load @font-face from @starting-style rule] - expected: FAIL diff --git a/testing/web-platform/meta/css/css-transitions/starting-style-rule-none.html.ini b/testing/web-platform/meta/css/css-transitions/starting-style-rule-none.html.ini index c970ca6b4265..f01ead5128af 100644 --- a/testing/web-platform/meta/css/css-transitions/starting-style-rule-none.html.ini +++ b/testing/web-platform/meta/css/css-transitions/starting-style-rule-none.html.ini @@ -1,2 +1,3 @@ [starting-style-rule-none.html] - expected: ERROR + [@starting-style with display:none] + expected: FAIL