Auto merge of #9136 - frewsxcv:htmlbodyelement-background, r=nox

HTMLBodyElement 'background' attribute improvements

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9136)
<!-- Reviewable:end -->

components/script/dom/htmlbodyelement.rs

@@ -4,7 +4,6 @@
 
 use cssparser::RGBA;
 use dom::attr::{Attr, AttrValue};
-use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull;
 use dom::bindings::codegen::Bindings::HTMLBodyElementBinding::{self, HTMLBodyElementMethods};
 use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
@@ -32,7 +31,6 @@ const INITIAL_REFLOW_DELAY: u64 = 200_000_000;
 #[dom_struct]
 pub struct HTMLBodyElement {
     htmlelement: HTMLElement,
-    background: DOMRefCell<Option<Url>>
 }
 
 impl HTMLBodyElement {
@@ -40,7 +38,6 @@ impl HTMLBodyElement {
                      -> HTMLBodyElement {
         HTMLBodyElement {
             htmlelement: HTMLElement::new_inherited(localName, prefix, document),
-            background: DOMRefCell::new(None)
         }
     }
 
@@ -84,6 +81,12 @@ impl HTMLBodyElementMethods for HTMLBodyElement {
     fn SetOnstorage(&self, listener: Option<Rc<EventHandlerNonNull>>) {
         window_from_node(self).SetOnstorage(listener)
     }
+
+    // https://html.spec.whatwg.org/multipage/#dom-body-background
+    make_getter!(Background, "background");
+
+    // https://html.spec.whatwg.org/multipage/#dom-body-background
+    make_url_setter!(SetBackground, "background");
 }
 
 pub trait HTMLBodyElementLayoutHelpers {
@@ -116,7 +119,10 @@ impl HTMLBodyElementLayoutHelpers for LayoutJS<HTMLBodyElement> {
     #[allow(unsafe_code)]
     fn get_background(&self) -> Option<Url> {
         unsafe {
-            (*self.unsafe_get()).background.borrow_for_layout().clone()
+            (*self.upcast::<Element>().unsafe_get())
+                .get_attr_for_layout(&ns!(), &atom!("background"))
+                .and_then(AttrValue::as_url)
+                .cloned()
         }
     }
 }
@@ -147,20 +153,13 @@ impl VirtualMethods for HTMLBodyElement {
         match *name {
             atom!("bgcolor") |
             atom!("text") => AttrValue::from_legacy_color(value),
+            atom!("background") => AttrValue::from_url(document_from_node(self).url(), value),
             _ => self.super_type().unwrap().parse_plain_attribute(name, value),
         }
     }
 
     fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
         let do_super_mutate = match (attr.local_name(), mutation) {
-            (&atom!("background"), _) => {
-                *self.background.borrow_mut() = mutation.new_value(attr).and_then(|value| {
-                    let document = document_from_node(self);
-                    let base = document.url();
-                    base.join(&value).ok()
-                });
-                true
-            },
             (name, AttributeMutation::Set(_)) if name.starts_with("on") => {
                 let window = window_from_node(self);
                 let (cx, url, reflector) = (window.get_cx(),

components/script/dom/macros.rs

@@ -152,6 +152,20 @@ macro_rules! make_bool_setter(
     );
 );
 
+#[macro_export]
+macro_rules! make_url_setter(
+    ( $attr:ident, $htmlname:tt ) => (
+        fn $attr(&self, value: DOMString) {
+            use dom::bindings::inheritance::Castable;
+            use dom::element::Element;
+            use dom::node::document_from_node;
+            let value = AttrValue::from_url(document_from_node(self).url(), value);
+            let element = self.upcast::<Element>();
+            element.set_attribute(&atom!($htmlname), value);
+        }
+    );
+);
+
 #[macro_export]
 macro_rules! make_uint_setter(
     ($attr:ident, $htmlname:tt, $default:expr) => (

components/script/dom/webidls/HTMLBodyElement.webidl

@@ -23,5 +23,5 @@ partial interface HTMLBodyElement {
   //[TreatNullAs=EmptyString] attribute DOMString aLink;
 
     [TreatNullAs=EmptyString] attribute DOMString bgColor;
-  //                          attribute DOMString background;
+  attribute DOMString background;
 };

components/style/attr.rs

@@ -5,6 +5,7 @@
 use cssparser::RGBA;
 use std::ops::Deref;
 use string_cache::{Atom, Namespace};
+use url::Url;
 use util::str::{DOMString, LengthOrPercentageOrAuto, parse_unsigned_integer, parse_legacy_color, parse_length};
 use util::str::{parse_nonzero_length, split_html_space_chars, str_join, parse_integer};
 use values::specified::{Length};
@@ -22,6 +23,7 @@ pub enum AttrValue {
     Length(DOMString, Option<Length>),
     Color(DOMString, Option<RGBA>),
     Dimension(DOMString, LengthOrPercentageOrAuto),
+    Url(DOMString, Option<Url>),
 }
 
 impl AttrValue {
@@ -86,6 +88,11 @@ impl AttrValue {
         AttrValue::Atom(value)
     }
 
+    pub fn from_url(base: &Url, url: DOMString) -> AttrValue {
+        let joined = base.join(&url).ok();
+        AttrValue::Url(url, joined)
+    }
+
     pub fn from_legacy_color(string: DOMString) -> AttrValue {
         let parsed = parse_legacy_color(&string).ok();
         AttrValue::Color(string, parsed)
@@ -161,6 +168,18 @@ impl AttrValue {
         }
     }
 
+    /// Assumes the `AttrValue` is a `Url` and returns its value
+    ///
+    /// ## Panics
+    ///
+    /// Panics if the `AttrValue` is not a `Url`
+    pub fn as_url(&self) -> Option<&Url> {
+        match *self {
+            AttrValue::Url(_, ref url) => url.as_ref(),
+            _ => panic!("Url not found"),
+        }
+    }
+
     /// Return the AttrValue as its integer representation, if any.
     /// This corresponds to attribute values returned as `AttrValue::UInt(_)`
     /// by `VirtualMethods::parse_plain_attribute()`.
@@ -188,6 +207,7 @@ impl Deref for AttrValue {
                 AttrValue::Length(ref value, _) |
                 AttrValue::Color(ref value, _) |
                 AttrValue::Int(ref value, _) |
+                AttrValue::Url(ref value, _) |
                 AttrValue::Dimension(ref value, _) => &value,
             AttrValue::Atom(ref value) => &value,
         }

tests/wpt/metadata/html/dom/interfaces.html.ini

@@ -2169,9 +2169,6 @@
   [HTMLBodyElement interface: attribute aLink]
     expected: FAIL
 
-  [HTMLBodyElement interface: attribute background]
-    expected: FAIL
-
   [HTMLBodyElement interface: attribute onafterprint]
     expected: FAIL
 
@@ -2214,9 +2211,6 @@
   [HTMLBodyElement interface: document.createElement("body") must inherit property "aLink" with the proper type (3)]
     expected: FAIL
 
-  [HTMLBodyElement interface: document.createElement("body") must inherit property "background" with the proper type (5)]
-    expected: FAIL
-
   [HTMLBodyElement interface: document.createElement("body") must inherit property "onafterprint" with the proper type (6)]
     expected: FAIL
 

tests/wpt/metadata/html/dom/reflection-sections.html.ini

@@ -846,135 +846,6 @@
   [body.aLink: IDL set to object "test-valueOf" followed by IDL get]
     expected: FAIL
 
-  [body.background: typeof IDL attribute]
-    expected: FAIL
-
-  [body.background: IDL get with DOM attribute unset]
-    expected: FAIL
-
-  [body.background: setAttribute() to "" followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to " \\0\\x01\\x02\\x03\\x04\\x05\\x06\\x07 \\b\\t\\n\\v\\f\\r\\x0e\\x0f \\x10\\x11\\x12\\x13\\x14\\x15\\x16\\x17 \\x18\\x19\\x1a\\x1b\\x1c\\x1d\\x1e\\x1f  foo " followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to undefined followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to 7 followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to 1.5 followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to true followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to false followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to object "[object Object\]" followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to NaN followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to Infinity followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to -Infinity followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to "\\0" followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to null followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to object "test-toString" followed by IDL get]
-    expected: FAIL
-
-  [body.background: setAttribute() to object "test-valueOf" followed by IDL get]
-    expected: FAIL
-
-  [body.background: IDL set to "" followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to " \\0\\x01\\x02\\x03\\x04\\x05\\x06\\x07 \\b\\t\\n\\v\\f\\r\\x0e\\x0f \\x10\\x11\\x12\\x13\\x14\\x15\\x16\\x17 \\x18\\x19\\x1a\\x1b\\x1c\\x1d\\x1e\\x1f  foo " followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to undefined followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to undefined followed by IDL get]
-    expected: FAIL
-
-  [body.background: IDL set to 7 followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to 7 followed by IDL get]
-    expected: FAIL
-
-  [body.background: IDL set to 1.5 followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to 1.5 followed by IDL get]
-    expected: FAIL
-
-  [body.background: IDL set to true followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to true followed by IDL get]
-    expected: FAIL
-
-  [body.background: IDL set to false followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to false followed by IDL get]
-    expected: FAIL
-
-  [body.background: IDL set to object "[object Object\]" followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to object "[object Object\]" followed by IDL get]
-    expected: FAIL
-
-  [body.background: IDL set to NaN followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to NaN followed by IDL get]
-    expected: FAIL
-
-  [body.background: IDL set to Infinity followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to Infinity followed by IDL get]
-    expected: FAIL
-
-  [body.background: IDL set to -Infinity followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to -Infinity followed by IDL get]
-    expected: FAIL
-
-  [body.background: IDL set to "\\0" followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to null followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to null followed by IDL get]
-    expected: FAIL
-
-  [body.background: IDL set to object "test-toString" followed by getAttribute()]
-    expected: FAIL
-
-  [body.background: IDL set to object "test-toString" followed by IDL get]
-    expected: FAIL
-
-  [body.background: IDL set to object "test-valueOf" followed by IDL get]
-    expected: FAIL
-
   [body.itemScope: typeof IDL attribute]
     expected: FAIL