Fix InsertRule to use the right CssRuleTypes (#32125)

`CSSRule::Type()` returns an u16 for CSSOM. `InsertRule()` was
incorrectly using this to create a `CssRuleTypes`.

Instead of `CssRuleTypes::from_bits(rule_type)`, it should be something
like `CssRuleTypes::from_bits(1 << rule_type)`.

However, that would only work when `Type()` provides an actual value,
which per https://drafts.csswg.org/cssom/#dom-cssrule-type only happens
for old rule types. New rule types just return 0.

Therefore, this patch changes the signature of `SpecificCSSRule::ty()`
to return the actual `CssRuleType`, and then `CSSRule::Type()` can zero
it out when necessary.

The fix is only relevant for CSS Nesting, which is currently disabled on
Servo, so no test is necessary.

<!-- Please describe your changes on the following line: -->


---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by
`[X]` when the step is complete, and replace `___` with appropriate
data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes do not require tests because the fix is only
relevant for CSS Nesting, which is currently disabled

<!-- Also, please make sure that "Allow edits from maintainers" checkbox
is checked, so that we can help you if you get stuck somewhere along the
way.-->

<!-- Pull requests that do not address these steps are welcome, but they
will require additional verification as part of the review process. -->
This commit is contained in:
Oriol Brufau 2024-04-22 09:06:36 +02:00 committed by GitHub
parent 67f239d1ba
commit f9e154af55
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 42 additions and 41 deletions

View file

@ -5,7 +5,7 @@
use dom_struct::dom_struct; use dom_struct::dom_struct;
use servo_arc::Arc; use servo_arc::Arc;
use style::shared_lock::{Locked, ToCssWithGuard}; use style::shared_lock::{Locked, ToCssWithGuard};
use style::stylesheets::FontFaceRule; use style::stylesheets::{CssRuleType, FontFaceRule};
use crate::dom::bindings::reflector::reflect_dom_object; use crate::dom::bindings::reflector::reflect_dom_object;
use crate::dom::bindings::root::DomRoot; use crate::dom::bindings::root::DomRoot;
@ -50,9 +50,8 @@ impl CSSFontFaceRule {
} }
impl SpecificCSSRule for CSSFontFaceRule { impl SpecificCSSRule for CSSFontFaceRule {
fn ty(&self) -> u16 { fn ty(&self) -> CssRuleType {
use crate::dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleConstants; CssRuleType::FontFace
CSSRuleConstants::FONT_FACE_RULE
} }
fn get_css(&self) -> DOMString { fn get_css(&self) -> DOMString {

View file

@ -8,7 +8,6 @@ use style::shared_lock::{Locked, SharedRwLock};
use style::stylesheets::{CssRuleTypes, CssRules as StyleCssRules}; use style::stylesheets::{CssRuleTypes, CssRules as StyleCssRules};
use crate::dom::bindings::codegen::Bindings::CSSGroupingRuleBinding::CSSGroupingRuleMethods; use crate::dom::bindings::codegen::Bindings::CSSGroupingRuleBinding::CSSGroupingRuleMethods;
use crate::dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRule_Binding::CSSRuleMethods;
use crate::dom::bindings::error::{ErrorResult, Fallible}; use crate::dom::bindings::error::{ErrorResult, Fallible};
use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::reflector::DomObject; use crate::dom::bindings::reflector::DomObject;
@ -69,7 +68,8 @@ impl CSSGroupingRuleMethods for CSSGroupingRule {
// https://drafts.csswg.org/cssom/#dom-cssgroupingrule-insertrule // https://drafts.csswg.org/cssom/#dom-cssgroupingrule-insertrule
fn InsertRule(&self, rule: DOMString, index: u32) -> Fallible<u32> { fn InsertRule(&self, rule: DOMString, index: u32) -> Fallible<u32> {
// TODO: this should accumulate the rule types of all ancestors. // TODO: this should accumulate the rule types of all ancestors.
let containing_rule_types = CssRuleTypes::from_bits(self.cssrule.Type().into()); let rule_type = self.cssrule.as_specific().ty();
let containing_rule_types = CssRuleTypes::from(rule_type);
self.rulelist() self.rulelist()
.insert_rule(&rule, index, containing_rule_types) .insert_rule(&rule, index, containing_rule_types)
} }

View file

@ -6,7 +6,7 @@ use dom_struct::dom_struct;
use servo_arc::Arc; use servo_arc::Arc;
use style::shared_lock::{Locked, ToCssWithGuard}; use style::shared_lock::{Locked, ToCssWithGuard};
use style::stylesheets::import_rule::ImportLayer; use style::stylesheets::import_rule::ImportLayer;
use style::stylesheets::ImportRule; use style::stylesheets::{CssRuleType, ImportRule};
use style_traits::ToCss; use style_traits::ToCss;
use crate::dom::bindings::codegen::Bindings::CSSImportRuleBinding::CSSImportRuleMethods; use crate::dom::bindings::codegen::Bindings::CSSImportRuleBinding::CSSImportRuleMethods;
@ -50,9 +50,8 @@ impl CSSImportRule {
} }
impl SpecificCSSRule for CSSImportRule { impl SpecificCSSRule for CSSImportRule {
fn ty(&self) -> u16 { fn ty(&self) -> CssRuleType {
use crate::dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleConstants; CssRuleType::Import
CSSRuleConstants::IMPORT_RULE
} }
fn get_css(&self) -> DOMString { fn get_css(&self) -> DOMString {

View file

@ -6,6 +6,7 @@ use dom_struct::dom_struct;
use servo_arc::Arc; use servo_arc::Arc;
use style::shared_lock::{Locked, ToCssWithGuard}; use style::shared_lock::{Locked, ToCssWithGuard};
use style::stylesheets::keyframes_rule::Keyframe; use style::stylesheets::keyframes_rule::Keyframe;
use style::stylesheets::CssRuleType;
use crate::dom::bindings::codegen::Bindings::CSSKeyframeRuleBinding::CSSKeyframeRuleMethods; use crate::dom::bindings::codegen::Bindings::CSSKeyframeRuleBinding::CSSKeyframeRuleMethods;
use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::inheritance::Castable;
@ -73,9 +74,8 @@ impl CSSKeyframeRuleMethods for CSSKeyframeRule {
} }
impl SpecificCSSRule for CSSKeyframeRule { impl SpecificCSSRule for CSSKeyframeRule {
fn ty(&self) -> u16 { fn ty(&self) -> CssRuleType {
use crate::dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleConstants; CssRuleType::Keyframe
CSSRuleConstants::KEYFRAME_RULE
} }
fn get_css(&self) -> DOMString { fn get_css(&self) -> DOMString {

View file

@ -7,6 +7,7 @@ use dom_struct::dom_struct;
use servo_arc::Arc; use servo_arc::Arc;
use style::shared_lock::{Locked, ToCssWithGuard}; use style::shared_lock::{Locked, ToCssWithGuard};
use style::stylesheets::keyframes_rule::{Keyframe, KeyframeSelector, KeyframesRule}; use style::stylesheets::keyframes_rule::{Keyframe, KeyframeSelector, KeyframesRule};
use style::stylesheets::CssRuleType;
use style::values::KeyframesName; use style::values::KeyframesName;
use crate::dom::bindings::codegen::Bindings::CSSKeyframesRuleBinding::CSSKeyframesRuleMethods; use crate::dom::bindings::codegen::Bindings::CSSKeyframesRuleBinding::CSSKeyframesRuleMethods;
@ -145,9 +146,8 @@ impl CSSKeyframesRuleMethods for CSSKeyframesRule {
} }
impl SpecificCSSRule for CSSKeyframesRule { impl SpecificCSSRule for CSSKeyframesRule {
fn ty(&self) -> u16 { fn ty(&self) -> CssRuleType {
use crate::dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleConstants; CssRuleType::Keyframes
CSSRuleConstants::KEYFRAMES_RULE
} }
fn get_css(&self) -> DOMString { fn get_css(&self) -> DOMString {

View file

@ -5,7 +5,7 @@
use dom_struct::dom_struct; use dom_struct::dom_struct;
use servo_arc::Arc; use servo_arc::Arc;
use style::shared_lock::ToCssWithGuard; use style::shared_lock::ToCssWithGuard;
use style::stylesheets::LayerBlockRule; use style::stylesheets::{CssRuleType, LayerBlockRule};
use style_traits::ToCss; use style_traits::ToCss;
use crate::dom::bindings::codegen::Bindings::CSSLayerBlockRuleBinding::CSSLayerBlockRuleMethods; use crate::dom::bindings::codegen::Bindings::CSSLayerBlockRuleBinding::CSSLayerBlockRuleMethods;
@ -56,8 +56,8 @@ impl CSSLayerBlockRule {
} }
impl SpecificCSSRule for CSSLayerBlockRule { impl SpecificCSSRule for CSSLayerBlockRule {
fn ty(&self) -> u16 { fn ty(&self) -> CssRuleType {
0 CssRuleType::LayerBlock
} }
fn get_css(&self) -> DOMString { fn get_css(&self) -> DOMString {

View file

@ -6,7 +6,7 @@ use dom_struct::dom_struct;
use js::jsval::JSVal; use js::jsval::JSVal;
use servo_arc::Arc; use servo_arc::Arc;
use style::shared_lock::ToCssWithGuard; use style::shared_lock::ToCssWithGuard;
use style::stylesheets::LayerStatementRule; use style::stylesheets::{CssRuleType, LayerStatementRule};
use style_traits::ToCss; use style_traits::ToCss;
use crate::dom::bindings::codegen::Bindings::CSSLayerStatementRuleBinding::CSSLayerStatementRuleMethods; use crate::dom::bindings::codegen::Bindings::CSSLayerStatementRuleBinding::CSSLayerStatementRuleMethods;
@ -55,8 +55,8 @@ impl CSSLayerStatementRule {
} }
impl SpecificCSSRule for CSSLayerStatementRule { impl SpecificCSSRule for CSSLayerStatementRule {
fn ty(&self) -> u16 { fn ty(&self) -> CssRuleType {
0 CssRuleType::LayerStatement
} }
fn get_css(&self) -> DOMString { fn get_css(&self) -> DOMString {

View file

@ -5,7 +5,7 @@
use dom_struct::dom_struct; use dom_struct::dom_struct;
use servo_arc::Arc; use servo_arc::Arc;
use style::shared_lock::ToCssWithGuard; use style::shared_lock::ToCssWithGuard;
use style::stylesheets::MediaRule; use style::stylesheets::{CssRuleType, MediaRule};
use style_traits::ToCss; use style_traits::ToCss;
use crate::dom::bindings::codegen::Bindings::CSSMediaRuleBinding::CSSMediaRuleMethods; use crate::dom::bindings::codegen::Bindings::CSSMediaRuleBinding::CSSMediaRuleMethods;
@ -68,9 +68,8 @@ impl CSSMediaRule {
} }
impl SpecificCSSRule for CSSMediaRule { impl SpecificCSSRule for CSSMediaRule {
fn ty(&self) -> u16 { fn ty(&self) -> CssRuleType {
use crate::dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleConstants; CssRuleType::Media
CSSRuleConstants::MEDIA_RULE
} }
fn get_css(&self) -> DOMString { fn get_css(&self) -> DOMString {

View file

@ -5,7 +5,7 @@
use dom_struct::dom_struct; use dom_struct::dom_struct;
use servo_arc::Arc; use servo_arc::Arc;
use style::shared_lock::ToCssWithGuard; use style::shared_lock::ToCssWithGuard;
use style::stylesheets::NamespaceRule; use style::stylesheets::{CssRuleType, NamespaceRule};
use crate::dom::bindings::codegen::Bindings::CSSNamespaceRuleBinding::CSSNamespaceRuleMethods; use crate::dom::bindings::codegen::Bindings::CSSNamespaceRuleBinding::CSSNamespaceRuleMethods;
use crate::dom::bindings::reflector::reflect_dom_object; use crate::dom::bindings::reflector::reflect_dom_object;
@ -67,9 +67,8 @@ impl CSSNamespaceRuleMethods for CSSNamespaceRule {
} }
impl SpecificCSSRule for CSSNamespaceRule { impl SpecificCSSRule for CSSNamespaceRule {
fn ty(&self) -> u16 { fn ty(&self) -> CssRuleType {
use crate::dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleConstants; CssRuleType::Namespace
CSSRuleConstants::NAMESPACE_RULE
} }
fn get_css(&self) -> DOMString { fn get_css(&self) -> DOMString {

View file

@ -6,7 +6,7 @@ use std::cell::Cell;
use dom_struct::dom_struct; use dom_struct::dom_struct;
use style::shared_lock::SharedRwLock; use style::shared_lock::SharedRwLock;
use style::stylesheets::CssRule as StyleCssRule; use style::stylesheets::{CssRule as StyleCssRule, CssRuleType};
use crate::dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleMethods; use crate::dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleMethods;
use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::inheritance::Castable;
@ -147,7 +147,14 @@ impl CSSRule {
impl CSSRuleMethods for CSSRule { impl CSSRuleMethods for CSSRule {
// https://drafts.csswg.org/cssom/#dom-cssrule-type // https://drafts.csswg.org/cssom/#dom-cssrule-type
fn Type(&self) -> u16 { fn Type(&self) -> u16 {
self.as_specific().ty() let rule_type = self.as_specific().ty() as u16;
// Per https://drafts.csswg.org/cssom/#dom-cssrule-type for constants > 15
// we return 0.
if rule_type > 15 {
0
} else {
rule_type
}
} }
// https://drafts.csswg.org/cssom/#dom-cssrule-parentstylesheet // https://drafts.csswg.org/cssom/#dom-cssrule-parentstylesheet
@ -171,7 +178,7 @@ impl CSSRuleMethods for CSSRule {
} }
pub trait SpecificCSSRule { pub trait SpecificCSSRule {
fn ty(&self) -> u16; fn ty(&self) -> CssRuleType;
fn get_css(&self) -> DOMString; fn get_css(&self) -> DOMString;
/// Remove parentStylesheet from all transitive children /// Remove parentStylesheet from all transitive children
fn deparent_children(&self) { fn deparent_children(&self) {

View file

@ -10,7 +10,7 @@ use selectors::parser::{ParseRelative, SelectorList};
use servo_arc::Arc; use servo_arc::Arc;
use style::selector_parser::SelectorParser; use style::selector_parser::SelectorParser;
use style::shared_lock::{Locked, ToCssWithGuard}; use style::shared_lock::{Locked, ToCssWithGuard};
use style::stylesheets::{Origin, StyleRule}; use style::stylesheets::{CssRuleType, Origin, StyleRule};
use crate::dom::bindings::codegen::Bindings::CSSStyleRuleBinding::CSSStyleRuleMethods; use crate::dom::bindings::codegen::Bindings::CSSStyleRuleBinding::CSSStyleRuleMethods;
use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::inheritance::Castable;
@ -58,9 +58,8 @@ impl CSSStyleRule {
} }
impl SpecificCSSRule for CSSStyleRule { impl SpecificCSSRule for CSSStyleRule {
fn ty(&self) -> u16 { fn ty(&self) -> CssRuleType {
use crate::dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleConstants; CssRuleType::Style
CSSRuleConstants::STYLE_RULE
} }
fn get_css(&self) -> DOMString { fn get_css(&self) -> DOMString {

View file

@ -5,7 +5,7 @@
use dom_struct::dom_struct; use dom_struct::dom_struct;
use servo_arc::Arc; use servo_arc::Arc;
use style::shared_lock::ToCssWithGuard; use style::shared_lock::ToCssWithGuard;
use style::stylesheets::SupportsRule; use style::stylesheets::{CssRuleType, SupportsRule};
use style_traits::ToCss; use style_traits::ToCss;
use crate::dom::bindings::reflector::reflect_dom_object; use crate::dom::bindings::reflector::reflect_dom_object;
@ -58,9 +58,8 @@ impl CSSSupportsRule {
} }
impl SpecificCSSRule for CSSSupportsRule { impl SpecificCSSRule for CSSSupportsRule {
fn ty(&self) -> u16 { fn ty(&self) -> CssRuleType {
use crate::dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleConstants; CssRuleType::Supports
CSSRuleConstants::SUPPORTS_RULE
} }
fn get_css(&self) -> DOMString { fn get_css(&self) -> DOMString {