Auto merge of #8434 - frewsxcv:dimension-attrvalue, r=eefriedman

Add Dimension member to AttrValue

Fixes #8417

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

components/script/dom/attr.rs

@@ -21,7 +21,7 @@ use std::mem;
 use std::ops::Deref;
 use string_cache::{Atom, Namespace};
 use style::values::specified::Length;
-use util::str::{DOMString, parse_unsigned_integer, parse_legacy_color};
+use util::str::{DOMString, LengthOrPercentageOrAuto, parse_unsigned_integer, parse_legacy_color, parse_length};
 use util::str::{split_html_space_chars, str_join};
 
 #[derive(JSTraceable, PartialEq, Clone, HeapSizeOf)]
@@ -32,6 +32,7 @@ pub enum AttrValue {
     Atom(Atom),
     Length(DOMString, Option<Length>),
     Color(DOMString, Option<RGBA>),
+    Dimension(DOMString, LengthOrPercentageOrAuto),
 }
 
 impl AttrValue {
@@ -83,6 +84,11 @@ impl AttrValue {
         AttrValue::Color(string, parsed)
     }
 
+    pub fn from_dimension(string: DOMString) -> AttrValue {
+        let parsed = parse_length(&string);
+        AttrValue::Dimension(string, parsed)
+    }
+
     /// Assumes the `AttrValue` is a `TokenList` and returns its tokens
     ///
     /// ## Panics
@@ -131,6 +137,18 @@ impl AttrValue {
         }
     }
 
+    /// Assumes the `AttrValue` is a `Dimension` and returns its value
+    ///
+    /// ## Panics
+    ///
+    /// Panics if the `AttrValue` is not a `Dimension`
+    pub fn as_dimension(&self) -> &LengthOrPercentageOrAuto {
+        match *self {
+            AttrValue::Dimension(_, ref l) => l,
+            _ => panic!("Dimension 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()`.
@@ -156,7 +174,8 @@ impl Deref for AttrValue {
                 AttrValue::TokenList(ref value, _) |
                 AttrValue::UInt(ref value, _) |
                 AttrValue::Length(ref value, _) |
-                AttrValue::Color(ref value, _) => &value,
+                AttrValue::Color(ref value, _) |
+                AttrValue::Dimension(ref value, _) => &value,
             AttrValue::Atom(ref value) => &value,
         }
     }

components/script/dom/htmltableelement.rs

@@ -10,7 +10,7 @@ use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
 use dom::bindings::inheritance::Castable;
 use dom::bindings::js::{LayoutJS, Root, RootedReference};
 use dom::document::Document;
-use dom::element::{AttributeMutation, Element};
+use dom::element::{AttributeMutation, Element, RawLayoutElementHelpers};
 use dom::htmlelement::HTMLElement;
 use dom::htmltablecaptionelement::HTMLTableCaptionElement;
 use dom::htmltablesectionelement::HTMLTableSectionElement;
@@ -26,7 +26,6 @@ pub struct HTMLTableElement {
     background_color: Cell<Option<RGBA>>,
     border: Cell<Option<u32>>,
     cellspacing: Cell<Option<u32>>,
-    width: Cell<LengthOrPercentageOrAuto>,
 }
 
 impl HTMLTableElement {
@@ -37,7 +36,6 @@ impl HTMLTableElement {
             background_color: Cell::new(None),
             border: Cell::new(None),
             cellspacing: Cell::new(None),
-            width: Cell::new(LengthOrPercentageOrAuto::Auto),
         }
     }
 
@@ -151,7 +149,11 @@ impl HTMLTableElementLayoutHelpers for LayoutJS<HTMLTableElement> {
     #[allow(unsafe_code)]
     fn get_width(&self) -> LengthOrPercentageOrAuto {
         unsafe {
-            (*self.unsafe_get()).width.get()
+            (*self.upcast::<Element>().unsafe_get())
+                .get_attr_for_layout(&ns!(""), &atom!("width"))
+                .map(AttrValue::as_dimension)
+                .cloned()
+                .unwrap_or(LengthOrPercentageOrAuto::Auto)
         }
     }
 }
@@ -180,12 +182,6 @@ impl VirtualMethods for HTMLTableElement {
                     str::parse_unsigned_integer(value.chars())
                 }));
             },
-            atom!(width) => {
-                let width = mutation.new_value(attr).map(|value| {
-                    str::parse_length(&value)
-                });
-                self.width.set(width.unwrap_or(LengthOrPercentageOrAuto::Auto));
-            },
             _ => {},
         }
     }
@@ -193,6 +189,7 @@ impl VirtualMethods for HTMLTableElement {
     fn parse_plain_attribute(&self, local_name: &Atom, value: DOMString) -> AttrValue {
         match *local_name {
             atom!("border") => AttrValue::from_u32(value, 1),
+            atom!("width") => AttrValue::from_dimension(value),
             _ => self.super_type().unwrap().parse_plain_attribute(local_name, value),
         }
     }

components/script/dom/macros.rs

@@ -231,6 +231,21 @@ macro_rules! make_legacy_color_setter(
     );
 );
 
+#[macro_export]
+macro_rules! make_dimension_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_dimension(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(

components/util/str.rs

@@ -173,7 +173,7 @@ pub fn parse_unsigned_integer<T: Iterator<Item=char>>(input: T) -> Option<u32> {
     })
 }
 
-#[derive(Copy, Clone, Debug)]
+#[derive(Copy, Clone, Debug, PartialEq)]
 pub enum LengthOrPercentageOrAuto {
     Auto,
     Percentage(f32),