servo: Merge #13358 - Form action url 11219 (from ducks:form-action-url-11219); r=mbrubeck
authorJake Goldsborough <rjgoldsborough@gmail.com>
Tue, 27 Sep 2016 15:28:09 -0500
changeset 339762 bd478fc9bb0ee7573e39cd9a48fe7d512981dbf1
parent 339761 28013ff2fdd1e9008581469ee779e3006e0e320f
child 339763 96d5cceb016c415bf1685b066e29b6fa2c795527
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmbrubeck
servo: Merge #13358 - Form action url 11219 (from ducks:form-action-url-11219); r=mbrubeck First pass at fixing #10580. I've added a new macro that returns a DomString with either the attr val or the doc url. I then made the form element use that macro on the action attribute. I also added a test that contains an iframe with a form and base url that submits to a page in a resources directory. I made all these changes based on https://github.com/servo/servo/pull/11219#issuecomment-223318881. The only thing I'm confused on is how to change step 8. It looks to just be getting the action so I'm wondering if I need to change either step 9 or 10 instead? --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #10580 (github issue number if applicable). - [X] There are tests for these changes OR using that macro with the form action, making the form submit process use base url, adding tests. Source-Repo: https://github.com/servo/servo Source-Revision: 040075ad085f7e74f207241fedba144d710f42b7
servo/components/script/dom/htmlformelement.rs
servo/components/script/dom/macros.rs
--- a/servo/components/script/dom/htmlformelement.rs
+++ b/servo/components/script/dom/htmlformelement.rs
@@ -93,17 +93,17 @@ impl HTMLFormElement {
 impl HTMLFormElementMethods for HTMLFormElement {
     // https://html.spec.whatwg.org/multipage/#dom-form-acceptcharset
     make_getter!(AcceptCharset, "accept-charset");
 
     // https://html.spec.whatwg.org/multipage/#dom-form-acceptcharset
     make_setter!(SetAcceptCharset, "accept-charset");
 
     // https://html.spec.whatwg.org/multipage/#dom-fs-action
-    make_url_or_base_getter!(Action, "action");
+    make_string_or_document_url_getter!(Action, "action");
 
     // https://html.spec.whatwg.org/multipage/#dom-fs-action
     make_setter!(SetAction, "action");
 
     // https://html.spec.whatwg.org/multipage/#dom-form-autocomplete
     make_enumerated_getter!(Autocomplete, "autocomplete", "on", ("off"));
 
     // https://html.spec.whatwg.org/multipage/#dom-form-autocomplete
@@ -290,17 +290,17 @@ impl HTMLFormElement {
         // Step 7
         result
     }
 
     /// [Form submission](https://html.spec.whatwg.org/multipage/#concept-form-submit)
     pub fn submit(&self, submit_method_flag: SubmittedFrom, submitter: FormSubmitter) {
         // Step 1
         let doc = document_from_node(self);
-        let base = doc.url();
+        let base = doc.base_url();
         // TODO: Handle browsing contexts (Step 2, 3)
         // Step 4
         if submit_method_flag == SubmittedFrom::NotFromForm &&
            !submitter.no_validate(self)
         {
             if self.interactive_validation().is_err() {
                 // TODO: Implement event handlers on all form control elements
                 self.upcast::<EventTarget>().fire_simple_event("invalid");
--- a/servo/components/script/dom/macros.rs
+++ b/servo/components/script/dom/macros.rs
@@ -119,16 +119,36 @@ macro_rules! make_url_or_base_getter(
             } else {
                 url
             }
         }
     );
 );
 
 #[macro_export]
+macro_rules! make_string_or_document_url_getter(
+    ( $attr:ident, $htmlname:tt ) => (
+        fn $attr(&self) -> DOMString {
+            use dom::bindings::inheritance::Castable;
+            use dom::element::Element;
+            use dom::node::document_from_node;
+            let element = self.upcast::<Element>();
+            let val = element.get_string_attribute(&atom!($htmlname));
+
+            if val.is_empty() {
+                let doc = document_from_node(self);
+                DOMString::from(doc.url().clone().into_string())
+            } else {
+                val
+            }
+        }
+    );
+);
+
+#[macro_export]
 macro_rules! make_enumerated_getter(
     ( $attr:ident, $htmlname:tt, $default:expr, $(($choices: pat))|+) => (
         fn $attr(&self) -> DOMString {
             use dom::bindings::inheritance::Castable;
             use dom::element::Element;
             use std::ascii::AsciiExt;
             let element = self.upcast::<Element>();
             let mut val = element.get_string_attribute(&atom!($htmlname));