Move CSSStyleDeclaration.RemoveProperty logic to style

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<DOMString> {
+    fn RemoveProperty(&self, property: DOMString) -> Fallible<DOMString> {
         // 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::<Element>();
-
-        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::<Node>();
+        let node = self.owner.upcast::<Node>();
         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::<Node>());
-        let element = self.owner.upcast::<Element>();
 
         // 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::<Node>();
+        let node = self.owner.upcast::<Node>();
         node.dirty(NodeDamage::NodeStyleDamaged);
         Ok(())
     }

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<PropertyDeclaration>,
                                importance: Importance) {

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<W>(&self, property_name: &str, dest: &mut W) -> fmt::Result
     where W: fmt::Write {