Move storage of bgcolor attribute on <body>.

servo · Nov 9, 2015 · 0901e5b · 0901e5b
1 parent 3780fb7
commit 0901e5b
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 19 deletions.
diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
@@ -968,7 +968,9 @@ impl Document {
 
     pub fn set_body_attribute(&self, local_name: &Atom, value: DOMString) {
         if let Some(ref body) = self.GetBody().and_then(Root::downcast::<HTMLBodyElement>) {
-            body.upcast::<Element>().set_string_attribute(local_name, value);
+            let body = body.upcast::<Element>();
+            let value = body.parse_attribute(&ns!(""), &local_name, value);
+            body.set_attribute(local_name, value);
         }
     }
 

diff --git a/components/script/dom/htmlbodyelement.rs b/components/script/dom/htmlbodyelement.rs
@@ -20,12 +20,11 @@ use dom::virtualmethods::VirtualMethods;
 use msg::constellation_msg::ConstellationChan;
 use msg::constellation_msg::Msg as ConstellationMsg;
 use std::borrow::ToOwned;
-use std::cell::Cell;
 use std::rc::Rc;
 use string_cache::Atom;
 use time;
 use url::{Url, UrlParser};
-use util::str::{self, DOMString};
+use util::str::DOMString;
 
 /// How long we should wait before performing the initial reflow after `<body>` is parsed, in
 /// nanoseconds.
@@ -34,7 +33,6 @@ const INITIAL_REFLOW_DELAY: u64 = 200_000_000;
 #[dom_struct]
 pub struct HTMLBodyElement {
     htmlelement: HTMLElement,
-    background_color: Cell<Option<RGBA>>,
     background: DOMRefCell<Option<Url>>
 }
 
@@ -43,7 +41,6 @@ impl HTMLBodyElement {
                      -> HTMLBodyElement {
         HTMLBodyElement {
             htmlelement: HTMLElement::new_inherited(localName, prefix, document),
-            background_color: Cell::new(None),
             background: DOMRefCell::new(None)
         }
     }
@@ -61,17 +58,13 @@ impl HTMLBodyElementMethods for HTMLBodyElement {
     make_getter!(BgColor, "bgcolor");
 
     // https://html.spec.whatwg.org/multipage/#dom-body-bgcolor
-    make_setter!(SetBgColor, "bgcolor");
+    make_legacy_color_setter!(SetBgColor, "bgcolor");
 
     // https://html.spec.whatwg.org/multipage/#dom-body-text
     make_getter!(Text);
 
     // https://html.spec.whatwg.org/multipage/#dom-body-text
-    fn SetText(&self, value: DOMString) {
-        let element = self.upcast::<Element>();
-        let color = str::parse_legacy_color(&value).ok();
-        element.set_attribute(&atom!("text"), AttrValue::Color(value, color));
-    }
+    make_legacy_color_setter!(SetText, "text");
 
     // https://html.spec.whatwg.org/multipage/#the-body-element
     fn GetOnunload(&self) -> Option<Rc<EventHandlerNonNull>> {
@@ -96,8 +89,14 @@ impl HTMLBodyElementMethods for HTMLBodyElement {
 
 
 impl HTMLBodyElement {
+    #[allow(unsafe_code)]
     pub fn get_background_color(&self) -> Option<RGBA> {
-        self.background_color.get()
+        unsafe {
+            self.upcast::<Element>()
+                .get_attr_for_layout(&ns!(""), &atom!("bgcolor"))
+                .and_then(AttrValue::as_color)
+                .cloned()
+        }
     }
 
     #[allow(unsafe_code)]
@@ -141,20 +140,16 @@ impl VirtualMethods for HTMLBodyElement {
     }
 
     fn parse_plain_attribute(&self, name: &Atom, value: DOMString) -> AttrValue {
-        match name {
-            &atom!("text") => AttrValue::from_legacy_color(value),
+        match *name {
+            atom!("bgcolor") |
+            atom!("text") => AttrValue::from_legacy_color(value),
             _ => self.super_type().unwrap().parse_plain_attribute(name, value),
         }
     }
 
     fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
         self.super_type().unwrap().attribute_mutated(attr, mutation);
         match (attr.local_name(), mutation) {
-            (&atom!(bgcolor), _) => {
-                self.background_color.set(mutation.new_value(attr).and_then(|value| {
-                    str::parse_legacy_color(&value).ok()
-                }));
-            },
             (&atom!(background), _) => {
                 *self.background.borrow_mut() = mutation.new_value(attr).and_then(|value| {
                     let document = document_from_node(self);

diff --git a/components/script/dom/macros.rs b/components/script/dom/macros.rs
@@ -215,6 +215,21 @@ macro_rules! make_atomic_setter(
     );
 );
 
+#[macro_export]
+macro_rules! make_legacy_color_setter(
+    ( $attr:ident, $htmlname:expr ) => (
+        fn $attr(&self, value: DOMString) {
+            use dom::bindings::inheritance::Castable;
+            use dom::element::Element;
+            use string_cache::Atom;
+            let element = self.upcast::<Element>();
+            let value = AttrValue::from_legacy_color(value);
+            // FIXME(pcwalton): Do this at compile time, not at runtime.
+            element.set_attribute(&Atom::from_slice($htmlname), value)
+        }
+    );
+);
+
 /// For use on non-jsmanaged types
 /// Use #[derive(JSTraceable)] on JS managed types
 macro_rules! no_jsmanaged_fields(