From bd4a4c38c86bb7275c24718686ee3a7e65dc1933 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 7 Oct 2016 17:06:56 +0200 Subject: [PATCH] Move CSSStyleDeclaration.RemoveProperty logic to style --- components/script/dom/cssstyledeclaration.rs | 54 ++++++++++--------- components/script/dom/element.rs | 31 ++--------- .../style/properties/declaration_block.rs | 24 +++++++++ 3 files changed, 57 insertions(+), 52 deletions(-) diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs index d347b63b3b00..81ab50903859 100644 --- a/components/script/dom/cssstyledeclaration.rs +++ b/components/script/dom/cssstyledeclaration.rs @@ -125,7 +125,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { } // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority - fn GetPropertyPriority(&self, mut property: DOMString) -> DOMString { + fn GetPropertyPriority(&self, property: DOMString) -> DOMString { let style_attribute = self.owner.style_attribute().borrow(); let style_attribute = if let Some(ref style_attribute) = *style_attribute { style_attribute.read() @@ -237,36 +237,40 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { } // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-removeproperty - fn RemoveProperty(&self, mut property: DOMString) -> Fallible { + fn RemoveProperty(&self, property: DOMString) -> Fallible { // Step 1 if self.readonly { return Err(Error::NoModificationAllowed); } - // Step 2 - property.make_ascii_lowercase(); - - // Step 3 - let value = self.GetPropertyValue(property.clone()); - - let element = self.owner.upcast::(); - - match Shorthand::from_name(&property) { - // Step 4 - Some(shorthand) => { - for longhand in shorthand.longhands() { - element.remove_inline_style_property(longhand) - } - } - // Step 5 - None => element.remove_inline_style_property(&property), + let mut style_attribute = self.owner.style_attribute().borrow_mut(); + let mut string = String::new(); + let empty; + { + let mut style_attribute = if let Some(ref mut style_attribute) = *style_attribute { + style_attribute.write() + } else { + // No style attribute is like an empty style attribute: nothing to remove. + return Ok(DOMString::new()) + }; + + // Step 3 + style_attribute.property_value_to_css(&property, &mut string).unwrap(); + + // Step 4 & 5 + style_attribute.remove_property(&property); + self.owner.set_style_attr(style_attribute.to_css_string()); + empty = style_attribute.declarations.is_empty() + } + if empty { + *style_attribute = None; } - let node = element.upcast::(); + let node = self.owner.upcast::(); node.dirty(NodeDamage::NodeStyleDamaged); // Step 6 - Ok(value) + Ok(DOMString::from(string)) } // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat @@ -311,7 +315,6 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-csstext fn SetCssText(&self, value: DOMString) -> ErrorResult { let window = window_from_node(self.owner.upcast::()); - let element = self.owner.upcast::(); // Step 1 if self.readonly { @@ -321,13 +324,14 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration { // Step 3 let decl_block = parse_style_attribute(&value, &window.get_url(), window.css_error_reporter(), ParserContextExtraData::default()); - *element.style_attribute().borrow_mut() = if decl_block.declarations.is_empty() { + *self.owner.style_attribute().borrow_mut() = if decl_block.declarations.is_empty() { + self.owner.set_style_attr(String::new()); None // Step 2 } else { + self.owner.set_style_attr(decl_block.to_css_string()); Some(Arc::new(RwLock::new(decl_block))) }; - element.sync_property_with_attrs_style(); - let node = element.upcast::(); + let node = self.owner.upcast::(); node.dirty(NodeDamage::NodeStyleDamaged); Ok(()) } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 4c2ac626e7ea..eba2d2cd370d 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -741,8 +741,11 @@ impl Element { } else { String::new() }; + self.set_style_attr(style_str) + } - let mut new_style = AttrValue::String(style_str); + pub fn set_style_attr(&self, new_value: String) { + let mut new_style = AttrValue::String(new_value); if let Some(style_attr) = self.attrs.borrow().iter().find(|a| a.name() == &atom!("style")) { style_attr.swap_value(&mut new_style); @@ -764,32 +767,6 @@ impl Element { self.attrs.borrow_mut().push(JS::from_ref(&attr)); } - pub fn remove_inline_style_property(&self, property: &str) { - fn remove(element: &Element, property: &str) { - let mut inline_declarations = element.style_attribute.borrow_mut(); - if let &mut Some(ref mut declarations) = &mut *inline_declarations { - let mut importance = None; - let index = declarations.read().declarations.iter().position(|&(ref decl, i)| { - let matching = decl.matches(property); - if matching { - importance = Some(i) - } - matching - }); - if let Some(index) = index { - let mut declarations = declarations.write(); - declarations.declarations.remove(index); - if importance.unwrap().important() { - declarations.important_count -= 1; - } - } - } - } - - remove(self, property); - self.sync_property_with_attrs_style(); - } - pub fn update_inline_style(&self, declarations: Vec, importance: Importance) { diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 10f4eb42d848..ec0412b544be 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -130,6 +130,30 @@ impl PropertyDeclarationBlock { } } + /// https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-removeproperty + pub fn remove_property(&mut self, property_name: &str) { + // Step 2 + let property = property_name.to_ascii_lowercase(); + + match Shorthand::from_name(&property) { + // Step 4 + Some(shorthand) => self.remove_longhands(shorthand.longhands()), + // Step 5 + None => self.remove_longhands(&[&*property]), + } + } + + fn remove_longhands(&mut self, names: &[&str]) { + let important_count = &mut self.important_count; + self.declarations.retain(|&(ref declaration, importance)| { + let retain = !names.iter().any(|n| declaration.matches(n)); + if !retain && importance.important() { + *important_count -= 1 + } + retain + }) + } + /// Take a declaration block known to contain a single property and serialize it. pub fn single_value_to_css(&self, property_name: &str, dest: &mut W) -> fmt::Result where W: fmt::Write {