From 3a0e8fefde8eedc1246ddd5058d14e7a680a9874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=BClker?= Date: Thu, 14 Aug 2025 15:27:39 +0200 Subject: [PATCH] script: Always throw when trying to `setProperty` on a readonly style `CSSStyleDeclaration` (#38677) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, [`SetProperty`](https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-setproperty) implemented step 3 before calling into the inner `set_property` method, which implements step 1. Therefore if step 3 returned then step 1 never runs, and can't throw an exception. Testing: A new web platform test starts to pass --------- Signed-off-by: Simon Wülker --- components/script/dom/cssstyledeclaration.rs | 69 +++++++++++++++---- .../computed-style-set-property.html.ini | 4 -- 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index 2cb2f979088..d06f4ed342c 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -305,6 +305,7 @@ impl CSSStyleDeclaration { DOMString::from(string) } + /// fn set_property( &self, id: PropertyId, @@ -312,24 +313,57 @@ impl CSSStyleDeclaration { priority: DOMString, can_gc: CanGc, ) -> ErrorResult { - // Step 1 + self.set_property_inner( + PotentiallyParsedPropertyId::Parsed(id), + value, + priority, + can_gc, + ) + } + + /// + /// + /// This function receives a `PotentiallyParsedPropertyId` instead of a `DOMString` in case + /// the caller already has a parsed property ID. + fn set_property_inner( + &self, + id: PotentiallyParsedPropertyId, + value: DOMString, + priority: DOMString, + can_gc: CanGc, + ) -> ErrorResult { + // Step 1. If the readonly flag is set, then throw a NoModificationAllowedError exception. if self.readonly { return Err(Error::NoModificationAllowed); } - if !id.enabled_for_all_content() { - return Ok(()); - } + let id = match id { + PotentiallyParsedPropertyId::Parsed(id) => { + if !id.enabled_for_all_content() { + return Ok(()); + } + + id + }, + PotentiallyParsedPropertyId::NotParsed(unparsed) => { + match PropertyId::parse_enabled_for_all_content(&unparsed) { + Ok(id) => id, + Err(..) => return Ok(()), + } + }, + }; self.owner.mutate_associated_block( |pdb, changed| { + // Step 3. If value is the empty string, invoke removeProperty() + // with property as argument and return. if value.is_empty() { - // Step 3 *changed = remove_property(pdb, &id); return Ok(()); } - // Step 4 + // Step 4. If priority is not the empty string and is not an ASCII case-insensitive + // match for the string "important", then return. let importance = match &*priority { "" => Importance::Normal, p if p.eq_ignore_ascii_case("important") => Importance::Important, @@ -410,6 +444,11 @@ pub(crate) static ENABLED_LONGHAND_PROPERTIES: LazyLock> = LazyL enabled_longhands }); +enum PotentiallyParsedPropertyId { + Parsed(PropertyId), + NotParsed(DOMString), +} + impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-length fn Length(&self) -> u32 { @@ -460,7 +499,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { }) } - // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-setproperty + /// fn SetProperty( &self, property: DOMString, @@ -468,12 +507,12 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { priority: DOMString, can_gc: CanGc, ) -> ErrorResult { - // Step 3 - let id = match PropertyId::parse_enabled_for_all_content(&property) { - Ok(id) => id, - Err(..) => return Ok(()), - }; - self.set_property(id, value, priority, can_gc) + self.set_property_inner( + PotentiallyParsedPropertyId::NotParsed(property), + value, + priority, + can_gc, + ) } // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-removeproperty @@ -501,12 +540,12 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { Ok(DOMString::from(string)) } - // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat + /// fn CssFloat(&self) -> DOMString { self.get_property_value(PropertyId::NonCustom(LonghandId::Float.into())) } - // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat + /// fn SetCssFloat(&self, value: DOMString, can_gc: CanGc) -> ErrorResult { self.set_property( PropertyId::NonCustom(LonghandId::Float.into()), diff --git a/tests/wpt/meta/css/cssom/computed-style-set-property.html.ini b/tests/wpt/meta/css/cssom/computed-style-set-property.html.ini index dedc1756b02..012f9e2179c 100644 --- a/tests/wpt/meta/css/cssom/computed-style-set-property.html.ini +++ b/tests/wpt/meta/css/cssom/computed-style-set-property.html.ini @@ -1,10 +1,6 @@ [computed-style-set-property.html] - [Exception thrown when trying to change a computed style alias via setProperty] - expected: FAIL - [Exception thrown when trying to change a computed style alias via property] expected: FAIL [Computed style parent (should be null)] expected: FAIL -