Move storage of bgcolor for <tr> and <tbody>.

components/script/dom/element.rs

@@ -48,8 +48,8 @@ use dom::htmllegendelement::HTMLLegendElement;
 use dom::htmloptgroupelement::HTMLOptGroupElement;
 use dom::htmltablecellelement::{HTMLTableCellElement, HTMLTableCellElementLayoutHelpers};
 use dom::htmltableelement::{HTMLTableElement, HTMLTableElementLayoutHelpers};
-use dom::htmltablerowelement::HTMLTableRowElement;
+use dom::htmltablerowelement::{HTMLTableRowElement, HTMLTableRowElementLayoutHelpers};
-use dom::htmltablesectionelement::HTMLTableSectionElement;
+use dom::htmltablesectionelement::{HTMLTableSectionElement, HTMLTableSectionElementLayoutHelpers};
 use dom::htmltemplateelement::HTMLTemplateElement;
 use dom::htmltextareaelement::{HTMLTextAreaElement, RawLayoutHTMLTextAreaElementHelpers};
 use dom::namednodemap::NamedNodeMap;
@@ -216,8 +216,6 @@ impl RawLayoutElementHelpers for Element {
 }
 
 pub trait LayoutElementHelpers {
-    #[allow(unsafe_code)]
-    unsafe fn get_attr_atom_for_layout(&self, namespace: &Namespace, name: &Atom) -> Option<Atom>;
     #[allow(unsafe_code)]
     unsafe fn has_class_for_layout(&self, name: &Atom) -> bool;
     #[allow(unsafe_code)]
@@ -231,8 +229,6 @@ pub trait LayoutElementHelpers {
                                                         -> Option<u32>;
     #[allow(unsafe_code)]
     unsafe fn html_element_in_html_document_for_layout(&self) -> bool;
-    #[allow(unsafe_code)]
-    unsafe fn has_attr_for_layout(&self, namespace: &Namespace, name: &Atom) -> bool;
     fn id_attribute(&self) -> *const Option<Atom>;
     fn style_attribute(&self) -> *const Option<PropertyDeclarationBlock>;
     fn local_name(&self) -> &Atom;
@@ -244,15 +240,6 @@ pub trait LayoutElementHelpers {
 }
 
 impl LayoutElementHelpers for LayoutJS<Element> {
-    #[allow(unsafe_code)]
-    #[inline]
-    unsafe fn get_attr_atom_for_layout(&self, namespace: &Namespace, name: &Atom)
-                                      -> Option<Atom> {
-        get_attr_for_layout(&*self.unsafe_get(), namespace, name).and_then(|attr| {
-            attr.value_atom_forever()
-        })
-    }
-
     #[allow(unsafe_code)]
     #[inline]
     unsafe fn has_class_for_layout(&self, name: &Atom) -> bool {
@@ -280,9 +267,9 @@ impl LayoutElementHelpers for LayoutJS<Element> {
         } else if let Some(this) = self.downcast::<HTMLTableCellElement>() {
             this.get_background_color()
         } else if let Some(this) = self.downcast::<HTMLTableRowElement>() {
-            (*this.unsafe_get()).get_background_color()
+            this.get_background_color()
         } else if let Some(this) = self.downcast::<HTMLTableSectionElement>() {
-            (*this.unsafe_get()).get_background_color()
+            this.get_background_color()
         } else {
             None
         };
@@ -538,11 +525,6 @@ impl LayoutElementHelpers for LayoutJS<Element> {
         self.upcast::<Node>().owner_doc_for_layout().is_html_document_for_layout()
     }
 
-    #[allow(unsafe_code)]
-    unsafe fn has_attr_for_layout(&self, namespace: &Namespace, name: &Atom) -> bool {
-        get_attr_for_layout(&*self.unsafe_get(), namespace, name).is_some()
-    }
-
     #[allow(unsafe_code)]
     fn id_attribute(&self) -> *const Option<Atom> {
         unsafe {

components/script/dom/htmltablerowelement.rs

@@ -3,22 +3,22 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use cssparser::RGBA;
-use dom::attr::Attr;
+use dom::attr::AttrValue;
 use dom::bindings::codegen::Bindings::HTMLTableRowElementBinding::{self, HTMLTableRowElementMethods};
 use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
 use dom::bindings::error::{ErrorResult, Fallible};
 use dom::bindings::inheritance::Castable;
-use dom::bindings::js::{JS, MutNullableHeap, Root, RootedReference};
+use dom::bindings::js::{JS, LayoutJS, MutNullableHeap, Root, RootedReference};
 use dom::document::Document;
-use dom::element::{AttributeMutation, Element};
+use dom::element::{Element, RawLayoutElementHelpers};
 use dom::htmlcollection::{CollectionFilter, HTMLCollection};
 use dom::htmlelement::HTMLElement;
 use dom::htmltabledatacellelement::HTMLTableDataCellElement;
 use dom::htmltableheadercellelement::HTMLTableHeaderCellElement;
 use dom::node::{Node, window_from_node};
 use dom::virtualmethods::VirtualMethods;
-use std::cell::Cell;
+use string_cache::Atom;
-use util::str::{self, DOMString};
+use util::str::DOMString;
 
 
 #[derive(JSTraceable)]
@@ -34,7 +34,6 @@ impl CollectionFilter for CellsFilter {
 pub struct HTMLTableRowElement {
     htmlelement: HTMLElement,
     cells: MutNullableHeap<JS<HTMLCollection>>,
-    background_color: Cell<Option<RGBA>>,
 }
 
 impl HTMLTableRowElement {
@@ -43,7 +42,6 @@ impl HTMLTableRowElement {
         HTMLTableRowElement {
             htmlelement: HTMLElement::new_inherited(localName, prefix, document),
             cells: Default::default(),
-            background_color: Cell::new(None),
         }
     }
 
@@ -54,10 +52,6 @@ impl HTMLTableRowElement {
                            document,
                            HTMLTableRowElementBinding::Wrap)
     }
-
-    pub fn get_background_color(&self) -> Option<RGBA> {
-        self.background_color.get()
-    }
 }
 
 impl HTMLTableRowElementMethods for HTMLTableRowElement {
@@ -65,7 +59,7 @@ impl HTMLTableRowElementMethods for HTMLTableRowElement {
     make_getter!(BgColor);
 
     // https://html.spec.whatwg.org/multipage/#dom-tr-bgcolor
-    make_setter!(SetBgColor, "bgcolor");
+    make_legacy_color_setter!(SetBgColor, "bgcolor");
 
     // https://html.spec.whatwg.org/multipage/#dom-tr-cells
     fn Cells(&self) -> Root<HTMLCollection> {
@@ -95,20 +89,31 @@ impl HTMLTableRowElementMethods for HTMLTableRowElement {
     }
 }
 
+pub trait HTMLTableRowElementLayoutHelpers {
+    fn get_background_color(&self) -> Option<RGBA>;
+}
+
+#[allow(unsafe_code)]
+impl HTMLTableRowElementLayoutHelpers for LayoutJS<HTMLTableRowElement> {
+    fn get_background_color(&self) -> Option<RGBA> {
+        unsafe {
+            (&*self.upcast::<Element>().unsafe_get())
+                .get_attr_for_layout(&ns!(""), &atom!("bgcolor"))
+                .and_then(AttrValue::as_color)
+                .cloned()
+        }
+    }
+}
+
 impl VirtualMethods for HTMLTableRowElement {
     fn super_type(&self) -> Option<&VirtualMethods> {
         Some(self.upcast::<HTMLElement>() as &VirtualMethods)
     }
 
-    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
+    fn parse_plain_attribute(&self, local_name: &Atom, value: DOMString) -> AttrValue {
-        self.super_type().unwrap().attribute_mutated(attr, mutation);
+        match *local_name {
-        match *attr.local_name() {
+            atom!("bgcolor") => AttrValue::from_legacy_color(value),
-            atom!(bgcolor) => {
+            _ => self.super_type().unwrap().parse_plain_attribute(local_name, value),
-                self.background_color.set(mutation.new_value(attr).and_then(|value| {
-                    str::parse_legacy_color(&value).ok()
-                }));
-            },
-            _ => {},
         }
     }
 }

components/script/dom/htmltablesectionelement.rs

@@ -3,26 +3,25 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use cssparser::RGBA;
-use dom::attr::Attr;
+use dom::attr::AttrValue;
 use dom::bindings::codegen::Bindings::HTMLTableSectionElementBinding::{self, HTMLTableSectionElementMethods};
 use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
 use dom::bindings::error::{ErrorResult, Fallible};
 use dom::bindings::inheritance::Castable;
-use dom::bindings::js::{Root, RootedReference};
+use dom::bindings::js::{LayoutJS, Root, RootedReference};
 use dom::document::Document;
-use dom::element::{AttributeMutation, Element};
+use dom::element::{Element, RawLayoutElementHelpers};
 use dom::htmlcollection::{CollectionFilter, HTMLCollection};
 use dom::htmlelement::HTMLElement;
 use dom::htmltablerowelement::HTMLTableRowElement;
 use dom::node::{Node, window_from_node};
 use dom::virtualmethods::VirtualMethods;
-use std::cell::Cell;
+use string_cache::Atom;
-use util::str::{self, DOMString};
+use util::str::DOMString;
 
 #[dom_struct]
 pub struct HTMLTableSectionElement {
     htmlelement: HTMLElement,
-    background_color: Cell<Option<RGBA>>,
 }
 
 impl HTMLTableSectionElement {
@@ -30,7 +29,6 @@ impl HTMLTableSectionElement {
                      -> HTMLTableSectionElement {
         HTMLTableSectionElement {
             htmlelement: HTMLElement::new_inherited(localName, prefix, document),
-            background_color: Cell::new(None),
         }
     }
 
@@ -40,10 +38,6 @@ impl HTMLTableSectionElement {
         let element = HTMLTableSectionElement::new_inherited(localName, prefix, document);
         Node::reflect_node(box element, document, HTMLTableSectionElementBinding::Wrap)
     }
-
-    pub fn get_background_color(&self) -> Option<RGBA> {
-        self.background_color.get()
-    }
 }
 
 #[derive(JSTraceable)]
@@ -80,20 +74,31 @@ impl HTMLTableSectionElementMethods for HTMLTableSectionElement {
     }
 }
 
+pub trait HTMLTableSectionElementLayoutHelpers {
+    fn get_background_color(&self) -> Option<RGBA>;
+}
+
+#[allow(unsafe_code)]
+impl HTMLTableSectionElementLayoutHelpers for LayoutJS<HTMLTableSectionElement> {
+    fn get_background_color(&self) -> Option<RGBA> {
+        unsafe {
+            (&*self.upcast::<Element>().unsafe_get())
+                .get_attr_for_layout(&ns!(""), &atom!("bgcolor"))
+                .and_then(AttrValue::as_color)
+                .cloned()
+        }
+    }
+}
+
 impl VirtualMethods for HTMLTableSectionElement {
     fn super_type(&self) -> Option<&VirtualMethods> {
         Some(self.upcast::<HTMLElement>() as &VirtualMethods)
     }
 
-    fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
+    fn parse_plain_attribute(&self, local_name: &Atom, value: DOMString) -> AttrValue {
-        self.super_type().unwrap().attribute_mutated(attr, mutation);
+        match *local_name {
-        match *attr.local_name() {
+            atom!("bgcolor") => AttrValue::from_legacy_color(value),
-            atom!(bgcolor) => {
+            _ => self.super_type().unwrap().parse_plain_attribute(local_name, value),
-                self.background_color.set(mutation.new_value(attr).and_then(|value| {
-                    str::parse_legacy_color(&value).ok()
-                }));
-            },
-            _ => {},
         }
     }
 }

components/script/dom/macros.rs

@@ -219,6 +219,7 @@ macro_rules! make_atomic_setter(
 macro_rules! make_legacy_color_setter(
     ( $attr:ident, $htmlname:expr ) => (
         fn $attr(&self, value: DOMString) {
+            use dom::attr::AttrValue;
             use dom::bindings::inheritance::Castable;
             use dom::element::Element;
             use string_cache::Atom;