Bug 607145 - form.action, button.formAction and input.formAction should be reflected as URL. r=bz a=blocking-b7

content/html/content/src/nsGenericHTMLElement.h

@@ -1186,6 +1186,27 @@ NS_NewHTML##_elementName##Element(already_AddRefed<nsINodeInfo> aNodeInfo,   \
     return SetAttrHelper(nsGkAtoms::_atom, aValue);                        \
   }
 
+/**
+ * A macro to implement getter and setter for action and form action content
+ * attributes. It's very similar to NS_IMPL_URI_ATTR expected that if the
+ * content attribute is the empty string, the empty string is returned.
+ */
+#define NS_IMPL_ACTION_ATTR(_class, _method, _atom)                 \
+  NS_IMETHODIMP                                                     \
+  _class::Get##_method(nsAString& aValue)                           \
+  {                                                                 \
+    GetAttr(kNameSpaceID_None, nsGkAtoms::_atom, aValue);           \
+    if (aValue.IsEmpty()) {                                         \
+      return NS_OK;                                                 \
+    }                                                               \
+    return GetURIAttr(nsGkAtoms::_atom, nsnull, aValue);            \
+  }                                                                 \
+  NS_IMETHODIMP                                                     \
+  _class::Set##_method(const nsAString& aValue)                     \
+  {                                                                 \
+    return SetAttrHelper(nsGkAtoms::_atom, aValue);                 \
+  }
+
 /**
  * A macro to implement the getter and setter for a given content
  * property that needs to set a non-negative integer. The method

content/html/content/src/nsHTMLButtonElement.cpp

@@ -218,7 +218,7 @@ nsHTMLButtonElement::GetForm(nsIDOMHTMLFormElement** aForm)
 NS_IMPL_STRING_ATTR(nsHTMLButtonElement, AccessKey, accesskey)
 NS_IMPL_BOOL_ATTR(nsHTMLButtonElement, Autofocus, autofocus)
 NS_IMPL_BOOL_ATTR(nsHTMLButtonElement, Disabled, disabled)
-NS_IMPL_STRING_ATTR(nsHTMLButtonElement, FormAction, formaction)
+NS_IMPL_ACTION_ATTR(nsHTMLButtonElement, FormAction, formaction)
 NS_IMPL_ENUM_ATTR_DEFAULT_VALUE(nsHTMLButtonElement, FormEnctype, formenctype,
                                 kFormDefaultEnctype->tag)
 NS_IMPL_ENUM_ATTR_DEFAULT_VALUE(nsHTMLButtonElement, FormMethod, formmethod,

content/html/content/src/nsHTMLFormElement.cpp

@@ -83,6 +83,8 @@
 #include "nsIConstraintValidation.h"
 #include "nsIEventStateManager.h"
 
+#include "nsIDOMHTMLButtonElement.h"
+
 static const int NS_FORM_CONTROL_LIST_HASHTABLE_SIZE = 16;
 
 static const PRUint8 NS_FORM_AUTOCOMPLETE_ON  = 1;
@@ -375,7 +377,7 @@ nsHTMLFormElement::SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
 }
 
 NS_IMPL_STRING_ATTR(nsHTMLFormElement, AcceptCharset, acceptcharset)
-NS_IMPL_STRING_ATTR(nsHTMLFormElement, Action, action)
+NS_IMPL_ACTION_ATTR(nsHTMLFormElement, Action, action)
 NS_IMPL_ENUM_ATTR_DEFAULT_VALUE(nsHTMLFormElement, Autocomplete, autocomplete,
                                 kFormDefaultAutocomplete->tag)
 NS_IMPL_ENUM_ATTR_DEFAULT_VALUE(nsHTMLFormElement, Enctype, enctype,
@@ -386,17 +388,6 @@ NS_IMPL_BOOL_ATTR(nsHTMLFormElement, NoValidate, novalidate)
 NS_IMPL_STRING_ATTR(nsHTMLFormElement, Name, name)
 NS_IMPL_STRING_ATTR(nsHTMLFormElement, Target, target)
 
-NS_IMETHODIMP
-nsHTMLFormElement::GetMozActionUri(nsAString& aValue)
-{
-  GetAttr(kNameSpaceID_None, nsGkAtoms::action, aValue);
-  if (aValue.IsEmpty()) {
-    // Avoid resolving action="" to the base uri, bug 297761.
-    return NS_OK;
-  }
-  return GetURIAttr(nsGkAtoms::action, nsnull, aValue);
-}
-
 NS_IMETHODIMP
 nsHTMLFormElement::Submit()
 {
@@ -1431,17 +1422,29 @@ nsHTMLFormElement::GetActionURL(nsIURI** aActionURL,
   // from the form element should be used.
   //
   nsAutoString action;
-  nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(aOriginatingElement);
-  if (formControl && formControl->IsSubmitControl() &&
-      aOriginatingElement->GetAttr(kNameSpaceID_None, nsGkAtoms::formaction,
-                                   action)) {
-    // Avoid resolving action="" to the base uri, bug 297761.
-    if (!action.IsEmpty()) {
-      static_cast<nsGenericHTMLElement*>(aOriginatingElement)->
-        GetURIAttr(nsGkAtoms::formaction, nsnull, action);
+
+  if (aOriginatingElement &&
+      aOriginatingElement->HasAttr(kNameSpaceID_None, nsGkAtoms::formaction)) {
+#ifdef DEBUG
+    nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(aOriginatingElement);
+    NS_ASSERTION(formControl && formControl->IsSubmitControl(),
+                 "The originating element must be a submit form control!");
+#endif // DEBUG
+
+    nsCOMPtr<nsIDOMHTMLInputElement> inputElement = do_QueryInterface(aOriginatingElement);
+    if (inputElement) {
+      inputElement->GetFormAction(action);
+    } else {
+      nsCOMPtr<nsIDOMHTMLButtonElement> buttonElement = do_QueryInterface(aOriginatingElement);
+      if (buttonElement) {
+        buttonElement->GetFormAction(action);
+      } else {
+        NS_ERROR("Originating element must be an input or button element!");
+        return NS_ERROR_UNEXPECTED;
+      }
     }
   } else {
-    GetMozActionUri(action);
+    GetAction(action);
   }
 
   //

content/html/content/src/nsHTMLInputElement.cpp

@@ -962,7 +962,7 @@ NS_IMPL_ENUM_ATTR_DEFAULT_VALUE(nsHTMLInputElement, Autocomplete, autocomplete,
 NS_IMPL_BOOL_ATTR(nsHTMLInputElement, Autofocus, autofocus)
 //NS_IMPL_BOOL_ATTR(nsHTMLInputElement, Checked, checked)
 NS_IMPL_BOOL_ATTR(nsHTMLInputElement, Disabled, disabled)
-NS_IMPL_STRING_ATTR(nsHTMLInputElement, FormAction, formaction)
+NS_IMPL_ACTION_ATTR(nsHTMLInputElement, FormAction, formaction)
 NS_IMPL_ENUM_ATTR_DEFAULT_VALUE(nsHTMLInputElement, FormEnctype, formenctype,
                                 kFormDefaultEnctype->tag)
 NS_IMPL_ENUM_ATTR_DEFAULT_VALUE(nsHTMLInputElement, FormMethod, formmethod,

content/html/content/test/Makefile.in

@@ -238,6 +238,7 @@ _TEST_FILES = \
 		test_bug606817.html \
 		test_bug297761.html \
 		file_bug297761.html \
+		test_bug607145.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)

content/html/content/test/test_bug392567.html

@@ -42,17 +42,17 @@ function runTests()
 
   // List of tests to run, each test consists of form action URL and expected result URL
   var tests = [
-    [jarUrl, jarUrl + "?$PARAMS", null],
-    [jarUrl + "?jarTest1=jarTest2", jarUrl + "?$PARAMS", null],
-    [jarUrl + "?jarTest3=jarTest4#jarTest5", jarUrl + "?$PARAMS#jarTest5", null],
-    ["data:text/html,<html></html>", "data:text/html,<html></html>?$PARAMS", null],
-    ["data:text/html,<html>How%20about%20this?</html>", "data:text/html,<html>How%20about%20this?$PARAMS", null],
-    [httpUrl, httpUrl + "?$PARAMS", null],
-    [httpUrl + "?httpTest1=httpTest2", httpUrl + "?$PARAMS", null ],
-    [httpUrl + "?httpTest3=httpTest4#httpTest5", httpUrl + "?$PARAMS#httpTest5", null],
-    ["", documentURL + "?$PARAMS", null],
-    [" ", documentURL + "?$PARAMS", document.location],
-    ["../", previousDir + "?$PARAMS", previousDir],
+    [jarUrl, jarUrl + "?$PARAMS"],
+    [jarUrl + "?jarTest1=jarTest2", jarUrl + "?$PARAMS"],
+    [jarUrl + "?jarTest3=jarTest4#jarTest5", jarUrl + "?$PARAMS#jarTest5"],
+    ["data:text/html,<html></html>", "data:text/html,<html></html>?$PARAMS"],
+    ["data:text/html,<html>How%20about%20this?</html>", "data:text/html,<html>How%20about%20this?$PARAMS"],
+    [httpUrl, httpUrl + "?$PARAMS"],
+    [httpUrl + "?httpTest1=httpTest2", httpUrl + "?$PARAMS"],
+    [httpUrl + "?httpTest3=httpTest4#httpTest5", httpUrl + "?$PARAMS#httpTest5"],
+    ["", documentURL + "?$PARAMS"],
+    [" ", documentURL + "?$PARAMS"],
+    ["../", previousDir + "?$PARAMS"],
   ];
 
   var currentTest = -1;
@@ -67,10 +67,6 @@ function runTests()
     }
 
     form.setAttribute("action", tests[currentTest][0]);
-    is(form.action, tests[currentTest][0],
-       "action IDL attribute should reflect the action content attribute");
-    is(form.mozActionUri, tests[currentTest][2] ? tests[currentTest][2] : tests[currentTest][0],
-       "mozActionUri IDL attribute should resolve the action URI");
     form.key.value = "value" + currentTest;
     form.submit();
   }

content/html/content/test/test_bug566160.html

@@ -83,41 +83,8 @@ var gTestResults = {
 
 var gPendingLoad = 0; // Has to be set after depending on the frames number.
 
-function checkFormActionAttribute(aElement)
-{
-  ok("formAction" in aElement, "formAction IDL attribute should be available in "
-     + aElement);
-
-  is(aElement.formAction, "", "formAction IDL attribute should be " +
-     "undefined by default");
-  is(aElement.getAttribute('formaction'), null, "formaction content attribute " +
-     "should be the empty string by default");
-
-  aElement.formAction = "foo";
-  is(aElement.getAttribute('formaction'), "foo", "formaction content attribute " +
-     "should be 'foo'.");
-  is(aElement.formAction, "foo", "formAction IDL attribute should reflect " +
-     "the content attribute");
-
-  aElement.setAttribute('formaction', 'bar');
-  is(aElement.getAttribute('formaction'), "bar", "formaction content attribute " +
-     "should be 'foo'.");
-  is(aElement.formAction, "bar", "formAction IDL attribute should reflect " +
-     "the content attribute");
-
-  aElement.removeAttribute('formaction');
-  is(aElement.formAction, "", "formAction IDL attribute should be " +
-     "undefined by default");
-  is(aElement.getAttribute('formaction'), null, "formaction content attribute " +
-     "should be the empty string by default");
-}
-
 function runTests()
 {
-  // First of all, let's check if .formAction and @formaction work correctly.
-  checkFormActionAttribute(document.createElement('input'));
-  checkFormActionAttribute(document.createElement('button'));
-
   // We add a load event for the frames which will be called when the forms
   // will be submitted.
   var frames = [ document.getElementById('frame1'),

content/html/content/test/test_bug607145.html

@@ -0,0 +1,78 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=607145
+-->
+<head>
+  <title>Test for Bug 607145</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=607145">Mozilla Bug 607145</a>
+<p id="display"></p>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 607145 **/
+
+/**
+ * This is not really reflecting an URL as the HTML5 specs want to.
+ * It's how .action is reflected in Gecko (might change later).
+ */
+function reflectURL(aElement, aAttr)
+{
+  var idl = aAttr;
+  var attr = aAttr.toLowerCase();
+  var elmtName = aElement.tagName.toLowerCase();
+
+  ok(idl in aElement, idl + " should be available in " + elmtName);
+
+  // Default values.
+  is(aElement[idl], "", "." + idl + " default value should be the empty string");
+  is(aElement.getAttribute(attr), null,
+     "@" + attr + " default value should be null");
+
+  var previousDir = location.href.replace(/test\/[^\/]*$/, "");
+  var dir = location.href.replace(/test_bug607145.html[^\/]*$/, "");
+  var doc = location.href.replace(/\.html.*/, ".html")
+  var values = [
+    /* value to set, resolved value */
+    [ "foo.html", dir + "foo.html" ],
+    [ "data:text/html,<html></html>", "data:text/html,<html></html>" ],
+    [ "http://example.org/", "http://example.org/" ],
+    [ "//example.org/", "http://example.org/" ],
+    [ "?foo=bar", doc + "?foo=bar" ],
+    [ "#foo", location.href + "#foo" ],
+    [ "", "" ], // TODO: doesn't follow the specs, should be location.href.
+    [ " ", location.href ],
+    [ "../", previousDir ],
+    [ "...", dir + "..." ],
+    // invalid URL
+    [ "http://a b/", "http://a b/" ], // TODO: doesn't follow the specs, should be "".
+  ];
+
+  for each (var value in values) {
+    aElement[idl] = value[0];
+    is(aElement[idl], value[1], "." + idl + " value should be " + value[1]);
+    is(aElement.getAttribute(attr), value[0],
+       "@" + attr + " value should be " + value[0]);
+  }
+
+  for each (var value in values) {
+    aElement.setAttribute(attr, value[0]);
+    is(aElement[idl], value[1], "." + idl + " value should be " + value[1]);
+    is(aElement.getAttribute(attr), value[0],
+       "@" + attr + " value should be " + value[0]);
+  }
+}
+
+reflectURL(document.createElement("form"), "action");
+reflectURL(document.createElement("input"), "formAction");
+reflectURL(document.createElement("button"), "formAction");
+
+</script>
+</pre>
+</body>
+</html>

dom/interfaces/html/nsIDOMHTMLFormElement.idl

@@ -47,7 +47,7 @@
  * http://www.w3.org/TR/DOM-Level-2-HTML/
  */
 
-[scriptable, uuid(653dc482-d6db-4e85-bdd9-151fd110e7b1)]
+[scriptable, uuid(0884ce23-e069-499e-a13c-a91c8ae0fc98)]
 interface nsIDOMHTMLFormElement : nsIDOMHTMLElement
 {
            attribute DOMString            name;
@@ -65,7 +65,4 @@ interface nsIDOMHTMLFormElement : nsIDOMHTMLElement
 
   void                      submit();
   void                      reset();
-
-  // This property returns the resolved action URI.
-  readonly attribute DOMString            mozActionUri;
 };

dom/tests/mochitest/dom-level2-html/test_button03.html

@@ -99,7 +99,7 @@ testNode = nodeList.item(0);
 
       // its not clear that this test is valid
       // mozilla returns a fully expanded uri
-      is(vfaction, "...", "formLink");
+      is(vfaction, formNode.ownerDocument.location.href.replace(/button\.html/, "..."), "formLink");
        
 }
 

toolkit/components/passwordmgr/src/nsLoginManager.js

@@ -978,9 +978,9 @@ LoginManager.prototype = {
     },
 
     _getActionOrigin : function (form) {
-        var uriString = form.mozActionUri;
+        var uriString = form.action;
 
-        // A blank or mission action submits to where it came from.
+        // A blank or missing action submits to where it came from.
         if (uriString == "")
             uriString = form.baseURI; // ala bug 297761