Address review comments

servo · Oct 8, 2014 · 1484acb · 1484acb · bors-servo · Oct 8, 2014
1 parent 3a1f9bd
commit 1484acb
Show file tree

Hide file tree

Showing 9 changed files with 38 additions and 45 deletions.
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
@@ -41,6 +41,7 @@ use std::cell::RefCell;
 use std::default::Default;
 use std::mem;
 use string_cache::{Atom, Namespace};
+use url::UrlParser;
 
 #[jstraceable]
 #[must_root]
@@ -458,8 +459,18 @@ impl<'a> AttributeHandlers for JSRef<'a, Element> {
 
     fn get_url_attribute(self, name: &str) -> DOMString {
         assert!(name == name.to_ascii_lower().as_slice());
-        // XXX Resolve URL.
-        self.get_string_attribute(name)
+        if !self.has_attribute(name) {
+            return "".to_string();
+        }
+        let url = self.get_string_attribute(name);
+        let doc = document_from_node(self).root();
+        let base = doc.url();
+        // https://html.spec.whatwg.org/multipage/infrastructure.html#reflect
+        // XXXManishearth this doesn't handle `javascript:` urls properly
+        match UrlParser::new().base_url(base).parse(url.as_slice()) {
+            Ok(parsed) => parsed.serialize(),
+            Err(_) => "".to_string()
+        }
     }
     fn set_url_attribute(self, name: &str, value: DOMString) {
         self.set_string_attribute(name, value);

diff --git a/components/script/dom/htmlbuttonelement.rs b/components/script/dom/htmlbuttonelement.rs
@@ -4,12 +4,12 @@
 
 use dom::bindings::codegen::Bindings::HTMLButtonElementBinding;
 use dom::bindings::codegen::Bindings::HTMLButtonElementBinding::HTMLButtonElementMethods;
-use dom::bindings::codegen::InheritTypes::{ElementCast, HTMLElementCast, NodeCast};
+use dom::bindings::codegen::InheritTypes::{HTMLElementCast, NodeCast};
 use dom::bindings::codegen::InheritTypes::{HTMLButtonElementDerived, HTMLFieldSetElementDerived};
 use dom::bindings::js::{JSRef, Temporary};
 use dom::bindings::utils::{Reflectable, Reflector};
 use dom::document::Document;
-use dom::element::{AttributeHandlers, Element, HTMLButtonElementTypeId};
+use dom::element::{AttributeHandlers, HTMLButtonElementTypeId};
 use dom::eventtarget::{EventTarget, NodeTargetTypeId};
 use dom::htmlelement::HTMLElement;
 use dom::node::{DisabledStateHelpers, Node, NodeHelpers, ElementNodeTypeId, window_from_node};

diff --git a/components/script/dom/htmlformelement.rs b/components/script/dom/htmlformelement.rs
@@ -11,7 +11,7 @@ use dom::document::Document;
 use dom::element::{Element, AttributeHandlers, HTMLFormElementTypeId};
 use dom::eventtarget::{EventTarget, NodeTargetTypeId};
 use dom::htmlelement::HTMLElement;
-use dom::node::{Node, ElementNodeTypeId};
+use dom::node::{Node, ElementNodeTypeId, window_from_node};
 use servo_util::str::DOMString;
 use std::ascii::OwnedStrAsciiExt;
 
@@ -50,7 +50,17 @@ impl<'a> HTMLFormElementMethods for JSRef<'a, HTMLFormElement> {
     make_setter!(SetAcceptCharset, "accept-charset")
 
     // https://html.spec.whatwg.org/multipage/forms.html#dom-fs-action
-    make_url_getter!(Action)
+    fn Action(self) -> DOMString {
+        let element: JSRef<Element> = ElementCast::from_ref(self);
+        let url = element.get_url_attribute("src");
+        match url.as_slice() {
+            "" => {
+                let window = window_from_node(self).root();
+                window.get_url().serialize()
+            },
+            _ => url
+        }
+    }
 
     // https://html.spec.whatwg.org/multipage/forms.html#dom-fs-action
     make_setter!(SetAction, "action")
@@ -126,6 +136,7 @@ impl<'a> HTMLFormElementMethods for JSRef<'a, HTMLFormElement> {
     // https://html.spec.whatwg.org/multipage/forms.html#dom-fs-target
     make_setter!(SetTarget, "target")
 }
+
 impl Reflectable for HTMLFormElement {
     fn reflector<'a>(&'a self) -> &'a Reflector {
         self.htmlelement.reflector()

diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs
@@ -106,7 +106,7 @@ impl<'a> HTMLImageElementMethods for JSRef<'a, HTMLImageElement> {
 
     make_getter!(UseMap)
 
-    make_bool_setter!(SetUseMap, "usemap")
+    make_setter!(SetUseMap, "usemap")
 
     make_bool_getter!(IsMap)
 
@@ -151,7 +151,7 @@ impl<'a> HTMLImageElementMethods for JSRef<'a, HTMLImageElement> {
 
     make_uint_getter!(Vspace)
 
-    make_uint_setter!(SetVspace, "Vspace")
+    make_uint_setter!(SetVspace, "vspace")
 
     make_getter!(LongDesc)
 

diff --git a/components/script/dom/htmloptgroupelement.rs b/components/script/dom/htmloptgroupelement.rs
@@ -4,12 +4,12 @@
 
 use dom::bindings::codegen::Bindings::HTMLOptGroupElementBinding;
 use dom::bindings::codegen::Bindings::HTMLOptGroupElementBinding::HTMLOptGroupElementMethods;
-use dom::bindings::codegen::InheritTypes::{ElementCast, HTMLElementCast, NodeCast};
+use dom::bindings::codegen::InheritTypes::{HTMLElementCast, NodeCast};
 use dom::bindings::codegen::InheritTypes::{HTMLOptGroupElementDerived, HTMLOptionElementDerived};
 use dom::bindings::js::{JSRef, Temporary};
 use dom::bindings::utils::{Reflectable, Reflector};
 use dom::document::Document;
-use dom::element::{AttributeHandlers, Element, HTMLOptGroupElementTypeId};
+use dom::element::{AttributeHandlers, HTMLOptGroupElementTypeId};
 use dom::eventtarget::{EventTarget, NodeTargetTypeId};
 use dom::htmlelement::HTMLElement;
 use dom::node::{DisabledStateHelpers, Node, NodeHelpers, ElementNodeTypeId};

diff --git a/components/script/dom/htmlselectelement.rs b/components/script/dom/htmlselectelement.rs
@@ -4,14 +4,14 @@
 
 use dom::bindings::codegen::Bindings::HTMLSelectElementBinding;
 use dom::bindings::codegen::Bindings::HTMLSelectElementBinding::HTMLSelectElementMethods;
-use dom::bindings::codegen::InheritTypes::{ElementCast, HTMLElementCast, NodeCast};
+use dom::bindings::codegen::InheritTypes::{HTMLElementCast, NodeCast};
 use dom::bindings::codegen::InheritTypes::{HTMLSelectElementDerived, HTMLFieldSetElementDerived};
 use dom::bindings::codegen::UnionTypes::HTMLElementOrLong::HTMLElementOrLong;
 use dom::bindings::codegen::UnionTypes::HTMLOptionElementOrHTMLOptGroupElement::HTMLOptionElementOrHTMLOptGroupElement;
 use dom::bindings::js::{JSRef, Temporary};
 use dom::bindings::utils::{Reflectable, Reflector};
 use dom::document::Document;
-use dom::element::{AttributeHandlers, Element, HTMLSelectElementTypeId};
+use dom::element::{AttributeHandlers, HTMLSelectElementTypeId};
 use dom::eventtarget::{EventTarget, NodeTargetTypeId};
 use dom::htmlelement::HTMLElement;
 use dom::node::{DisabledStateHelpers, Node, NodeHelpers, ElementNodeTypeId, window_from_node};

diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs
@@ -4,12 +4,12 @@
 
 use dom::bindings::codegen::Bindings::HTMLTextAreaElementBinding;
 use dom::bindings::codegen::Bindings::HTMLTextAreaElementBinding::HTMLTextAreaElementMethods;
-use dom::bindings::codegen::InheritTypes::{ElementCast, HTMLElementCast, NodeCast};
+use dom::bindings::codegen::InheritTypes::{HTMLElementCast, NodeCast};
 use dom::bindings::codegen::InheritTypes::{HTMLTextAreaElementDerived, HTMLFieldSetElementDerived};
 use dom::bindings::js::{JSRef, Temporary};
 use dom::bindings::utils::{Reflectable, Reflector};
 use dom::document::Document;
-use dom::element::{AttributeHandlers, Element, HTMLTextAreaElementTypeId};
+use dom::element::{AttributeHandlers, HTMLTextAreaElementTypeId};
 use dom::eventtarget::{EventTarget, NodeTargetTypeId};
 use dom::htmlelement::HTMLElement;
 use dom::node::{DisabledStateHelpers, Node, NodeHelpers, ElementNodeTypeId};

diff --git a/components/script/dom/macros.rs b/components/script/dom/macros.rs
@@ -59,21 +59,10 @@ macro_rules! make_url_getter(
         fn $attr(self) -> DOMString {
             use dom::element::{Element, AttributeHandlers};
             use dom::bindings::codegen::InheritTypes::ElementCast;
-            use dom::document::DocumentHelpers;
-            use dom::node::document_from_node;
-            use url::UrlParser;
             #[allow(unused_imports)]
             use std::ascii::StrAsciiExt;
-            let elem: JSRef<Element> = ElementCast::from_ref(self);
-            let action = elem.get_string_attribute($htmlname);
-            let doc = document_from_node(self).root();
-            let base = doc.url();
-            // https://html.spec.whatwg.org/multipage/infrastructure.html#reflect
-            // XXXManishearth this doesn't handle `javascript:` urls properly
-            match UrlParser::new().base_url(base).parse(action.as_slice()) {
-                Ok(parsed) => parsed.serialize(),
-                Err(_) => base.serialize()
-            }
+            let element: JSRef<Element> = ElementCast::from_ref(self);
+            element.get_url_attribute($htmlname)
         }
     );
     ($attr:ident) => {

diff --git a/tests/content/test_script_src_attribute.html b/tests/content/test_script_src_attribute.html