566064 - HTMLInputElement and HTMLButtonElement should implement formtarget attribute which override HTMLFormElement target attribute

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Mounir Lamouri (:mounir)]

If a form is submitted via a submit button (input types image and submit and button type submit) and if the element has a formtarget attribute, it should override the form target attribute.

Comment 1

[Mounir Lamouri (:mounir)]

Attachment: Tests v1

Comment 2

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

Comment 3

[Mounir Lamouri (:mounir)]

Attachment: Tests v2

A small improvement in the tests (checking the <base> target selection).

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 445496 [details] [diff] [review]
Patch v1

> nsHTMLFormElement::FlushPendingSubmission()
> {
>   if (mPendingSubmission) {
>     // Transfer owning reference so that the submissioin doesn't get deleted
>     // if we reenter
>     nsAutoPtr<nsFormSubmission> submission = mPendingSubmission;
> 
>-    SubmitSubmission(submission);
>+    SubmitSubmission(submission, nsnull);
>   }
> }

I think nsnull here isn't always the correct value. I.e. the submission could still have come from a <button>, in which case we want to use that buttons formtarget. I think if <button>.click() is called for example.

One way to fix it would be to remember the target in the submission object. Another is to keep a mPendingSubmissionOrigin in parallel with mPendingSubmission. I sort of like the former more.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 445544 [details] [diff] [review]
Tests v2

Resetting request as I assume that you'll need to add more tests for <button>.click().

Comment 6

[Mounir Lamouri (:mounir)]

(In reply to comment #4)
> (From update of attachment 445496 [details] [diff] [review])
> > nsHTMLFormElement::FlushPendingSubmission()
> > {
> >   if (mPendingSubmission) {
> >     // Transfer owning reference so that the submissioin doesn't get deleted
> >     // if we reenter
> >     nsAutoPtr<nsFormSubmission> submission = mPendingSubmission;
> > 
> >-    SubmitSubmission(submission);
> >+    SubmitSubmission(submission, nsnull);
> >   }
> > }
> 
> I think nsnull here isn't always the correct value. I.e. the submission could
> still have come from a <button>, in which case we want to use that buttons
> formtarget. I think if <button>.click() is called for example.
> 
> One way to fix it would be to remember the target in the submission object.
> Another is to keep a mPendingSubmissionOrigin in parallel with
> mPendingSubmission. I sort of like the former more.

I understand the theory but I was not able to test that. Would you have any concrete test ? The only test with pending submission i've been able to write is something like:
button.onclick = function() { form.submit(); return false; }
but the pending submission here do not have any formtarget because we call |submit| directly on the form.

I already wrote the patch but I need to found a test...

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ugh, this is so messy. *If* I understand the code correctly (and I'm not at all sure I do), we set mDeferSubmission while we are firing the onsubmit event. So if another submission occurs inside an event handler then that submission will be deferred.

So maybe something like this:

myFormElement.onsubmit = function() {
  myFormElement.onsubmit = null;
  myButtonElement.submit();
}

where myButtonElement is a <button formtarget=...> inside the <form>. Let me know if that works.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Sorry, that should of course be:

myFormElement.onsubmit = function() {
  myFormElement.onsubmit = null;
  myButtonElement.click();
}

Comment 9

[Mounir Lamouri (:mounir)]

With your code snippet called from a button click, no submit is deferred (checked with a simple printf). I will have another look during the week-end to see if i can found a way to have a submission from an input or a button deferred. Considering the beauty of this code, this is gonna do a real pleasure :)

Comment 10

[Mounir Lamouri (:mounir)]

Attachment: Part 1 - Move originating element to nsFormSubmission

Comment 11

[Mounir Lamouri (:mounir)]

Request for blocker-2.0: we want this HTML5 Feature for ff4.

Comment 12

[Mounir Lamouri (:mounir)]

Attachment: Part 2 - Implement formtarget

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 466893 [details] [diff] [review]
Part 1 - Move originating element to nsFormSubmission

>diff --git a/content/html/content/public/nsFormSubmission.h b/content/html/content/public/nsFormSubmission.h
>+  nsIContent* GetOriginatingElement() const
>+  {
>+    return mOriginatingElement.get();
>+  }

No need for the .get()

> protected:
>   /**
>    * Can only be constructed by subclasses.
>    *
>    * @param aCharset the charset of the form as a string
>+   * @param aOriginatingElement the originating element (can be null)

"originating element" isn't very descriptive. Add something like "i.e. the button that was pushed to submit the form".

>+  // Originating element.
>+  nsCOMPtr<nsIContent> mOriginatingElement;

Same here.

> };
> 
> class nsEncodingFormSubmission : public nsFormSubmission
> {
> public:
>-  nsEncodingFormSubmission(const nsACString& aCharset);
>+  nsEncodingFormSubmission(const nsACString& aCharset,
>+                           nsIContent* aOriginatingElement);
> 
>   virtual ~nsEncodingFormSubmission();
> 
>   /**
>    * Encode a Unicode string to bytes using the encoder (or just copy the input
>    * if there is no encoder).
>    * @param aStr the string to encode
>    * @param aResult the encoded string [OUT]
>@@ -164,17 +175,18 @@ private:
>  * inputs.  This always does POST.
>  */
> class nsFSMultipartFormData : public nsEncodingFormSubmission
> {
> public:
>   /**
>    * @param aCharset the charset of the form as a string
>    */
>-  nsFSMultipartFormData(const nsACString& aCharset);
>+  nsFSMultipartFormData(const nsACString& aCharset,
>+                        nsIContent* aOriginatingElement);
>   ~nsFSMultipartFormData();
>  
>   virtual nsresult AddNameValuePair(const nsAString& aName,
>                                     const nsAString& aValue);
>   virtual nsresult AddNameFilePair(const nsAString& aName,
>                                    nsIFile* aFile);
>   virtual nsresult GetEncodedSubmission(nsIURI* aURI,
>                                         nsIInputStream** aPostDataStream);
>@@ -218,14 +230,16 @@ private:
>    */
>   nsCString mBoundary;
> };
> 
> /**
>  * Get a submission object based on attributes in the form (ENCTYPE and METHOD)
>  *
>  * @param aForm the form to get a submission object based on
>+ * @param aOriginatingElement the originating element (can be null)
>  * @param aFormSubmission the form submission object (out param)
>  */
> nsresult GetSubmissionFromForm(nsGenericHTMLElement* aForm,
>+                               nsIContent* aOriginatingElement,
>                                nsFormSubmission** aFormSubmission);
> 
> #endif /* nsIFormSubmission_h___ */
>diff --git a/content/html/content/public/nsIFormControl.h b/content/html/content/public/nsIFormControl.h
>--- a/content/html/content/public/nsIFormControl.h
>+++ b/content/html/content/public/nsIFormControl.h
>@@ -94,18 +94,18 @@ enum InputElementTypes {
>   eInputElementTypesMax
> };
> 
> PR_STATIC_ASSERT((PRUint32)eFormControlsWithoutSubTypesMax < (PRUint32)NS_FORM_BUTTON_ELEMENT);
> PR_STATIC_ASSERT((PRUint32)eButtonElementTypesMax < (PRUint32)NS_FORM_INPUT_ELEMENT);
> PR_STATIC_ASSERT((PRUint32)eInputElementTypesMax  < 1<<8);
> 
> #define NS_IFORMCONTROL_IID   \
>-{ 0x0dc5083b, 0xb0a8, 0x48c4, \
>- { 0xb2, 0xeb, 0xc2, 0x4f, 0xfb, 0x7e, 0xc2, 0x8e } }
>+{ 0xc2f7723a, 0x106a, 0x47ef, \
>+ { 0xa9, 0xff, 0x4b, 0x4f, 0x73, 0x47, 0xe7, 0xa6 } }
> 
> /**
>  * Interface which all form controls (e.g. buttons, checkboxes, text,
>  * radio buttons, select, etc) implement in addition to their dom specific
>  * interface.
>  */
> class nsIFormControl : public nsISupports
> {
>@@ -151,21 +151,18 @@ public:
>    */
>   NS_IMETHOD Reset() = 0;
> 
>   /**
>    * Tells the form control to submit its names and values to the form
>    * submission object
>    * @param aFormSubmission the form submission to notify of names/values/files
>    *                       to submit
>-   * @param aSubmitElement the element that was pressed to submit (possibly
>-   *                       null)
>    */
>-  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                               nsIContent* aSubmitElement) = 0;
>+  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission) = 0;
> 
>   /**
>    * Save to presentation state.  The form control will determine whether it
>    * has anything to save and if so, create an entry in the layout history for
>    * its pres context.
>    */
>   NS_IMETHOD SaveState() = 0;
> 
>diff --git a/content/html/content/src/nsFormSubmission.cpp b/content/html/content/src/nsFormSubmission.cpp
>--- a/content/html/content/src/nsFormSubmission.cpp
>+++ b/content/html/content/src/nsFormSubmission.cpp
>@@ -93,18 +93,19 @@ class nsFSURLEncoded : public nsEncoding
> public:
>   /**
>    * @param aCharset the charset of the form as a string
>    * @param aMethod the method of the submit (either NS_FORM_METHOD_GET or
>    *        NS_FORM_METHOD_POST).
>    */
>   nsFSURLEncoded(const nsACString& aCharset,
>                  PRInt32 aMethod,
>-                 nsIDocument* aDocument)
>-    : nsEncodingFormSubmission(aCharset),
>+                 nsIDocument* aDocument,
>+                 nsIContent* aOriginatingElement)
>+    : nsEncodingFormSubmission(aCharset, aOriginatingElement),
>       mMethod(aMethod),
>       mDocument(aDocument),
>       mWarnedFileControl(PR_FALSE)
>   {
>   }
> 
>   virtual nsresult AddNameValuePair(const nsAString& aName,
>                                     const nsAString& aValue);
>@@ -398,18 +399,19 @@ nsFSURLEncoded::URLEncode(const nsAStrin
>   NS_ENSURE_TRUE(escapedBuf, NS_ERROR_OUT_OF_MEMORY);
>   aEncoded.Adopt(escapedBuf);
> 
>   return NS_OK;
> }
> 
> // --------------------------------------------------------------------------
> 
>-nsFSMultipartFormData::nsFSMultipartFormData(const nsACString& aCharset)
>-    : nsEncodingFormSubmission(aCharset)
>+nsFSMultipartFormData::nsFSMultipartFormData(const nsACString& aCharset,
>+                                             nsIContent* aOriginatingElement)
>+    : nsEncodingFormSubmission(aCharset, aOriginatingElement)
> {
>   mPostDataStream =
>     do_CreateInstance("@mozilla.org/io/multiplex-input-stream;1");
> 
>   mBoundary.AssignLiteral("---------------------------");
>   mBoundary.AppendInt(rand());
>   mBoundary.AppendInt(rand());
>   mBoundary.AppendInt(rand());
>@@ -590,18 +592,18 @@ nsFSMultipartFormData::AddPostDataStream
>   return rv;
> }
> 
> // --------------------------------------------------------------------------
> 
> class nsFSTextPlain : public nsEncodingFormSubmission
> {
> public:
>-  nsFSTextPlain(const nsACString& aCharset)
>-    : nsEncodingFormSubmission(aCharset)
>+  nsFSTextPlain(const nsACString& aCharset, nsIContent* aOriginatingElement)
>+    : nsEncodingFormSubmission(aCharset, aOriginatingElement)
>   {
>   }
> 
>   virtual nsresult AddNameValuePair(const nsAString& aName,
>                                     const nsAString& aValue);
>   virtual nsresult AddNameFilePair(const nsAString& aName,
>                                    nsIFile* aFile);
>   virtual nsresult GetEncodedSubmission(nsIURI* aURI,
>@@ -686,18 +688,19 @@ nsFSTextPlain::GetEncodedSubmission(nsIU
>     CallQueryInterface(mimeStream, aPostDataStream);
>   }
> 
>   return rv;
> }
> 
> // --------------------------------------------------------------------------
> 
>-nsEncodingFormSubmission::nsEncodingFormSubmission(const nsACString& aCharset)
>-  : nsFormSubmission(aCharset)
>+nsEncodingFormSubmission::nsEncodingFormSubmission(const nsACString& aCharset,
>+                                                   nsIContent* aOriginatingElement)
>+  : nsFormSubmission(aCharset, aOriginatingElement)
> {
>   nsCAutoString charset(aCharset);
>   // canonical name is passed so that we just have to check against
>   // *our* canonical names listed in charsetaliases.properties
>   if (charset.EqualsLiteral("ISO-8859-1")) {
>     charset.AssignLiteral("windows-1252");
>   }
> 
>@@ -795,16 +798,17 @@ GetEnumAttr(nsGenericHTMLElement* aConte
>   const nsAttrValue* value = aContent->GetParsedAttr(atom);
>   if (value && value->Type() == nsAttrValue::eEnum) {
>     *aValue = value->GetEnumValue();
>   }
> }
> 
> nsresult
> GetSubmissionFromForm(nsGenericHTMLElement* aForm,
>+                      nsIContent* aOriginatingElement,
>                       nsFormSubmission** aFormSubmission)
> {
>   // Get all the information necessary to encode the form data
>   NS_ASSERTION(aForm->GetCurrentDoc(),
>                "Should have doc if we're building submission!");
> 
>   // Get encoding type (default: urlencoded)
>   PRInt32 enctype = NS_FORM_ENCTYPE_URLENCODED;
>@@ -816,28 +820,29 @@ GetSubmissionFromForm(nsGenericHTMLEleme
> 
>   // Get charset
>   nsCAutoString charset;
>   GetSubmitCharset(aForm, charset);
> 
>   // Choose encoder
>   if (method == NS_FORM_METHOD_POST &&
>       enctype == NS_FORM_ENCTYPE_MULTIPART) {
>-    *aFormSubmission = new nsFSMultipartFormData(charset);
>+    *aFormSubmission = new nsFSMultipartFormData(charset, aOriginatingElement);
>   } else if (method == NS_FORM_METHOD_POST &&
>              enctype == NS_FORM_ENCTYPE_TEXTPLAIN) {
>-    *aFormSubmission = new nsFSTextPlain(charset);
>+    *aFormSubmission = new nsFSTextPlain(charset, aOriginatingElement);
>   } else {
>     nsIDocument* doc = aForm->GetOwnerDoc();
>     if (enctype == NS_FORM_ENCTYPE_MULTIPART ||
>         enctype == NS_FORM_ENCTYPE_TEXTPLAIN) {
>       nsAutoString enctypeStr;
>       aForm->GetAttr(kNameSpaceID_None, nsGkAtoms::enctype, enctypeStr);
>       const PRUnichar* enctypeStrPtr = enctypeStr.get();
>       SendJSWarning(doc, "ForgotPostWarning",
>                     &enctypeStrPtr, 1);
>     }
>-    *aFormSubmission = new nsFSURLEncoded(charset, method, doc);
>+    *aFormSubmission = new nsFSURLEncoded(charset, method, doc,
>+                                          aOriginatingElement);
>   }
>   NS_ENSURE_TRUE(*aFormSubmission, NS_ERROR_OUT_OF_MEMORY);
> 
>   return NS_OK;
> }
>diff --git a/content/html/content/src/nsHTMLButtonElement.cpp b/content/html/content/src/nsHTMLButtonElement.cpp
>--- a/content/html/content/src/nsHTMLButtonElement.cpp
>+++ b/content/html/content/src/nsHTMLButtonElement.cpp
>@@ -95,18 +95,17 @@ public:
>   NS_FORWARD_NSIDOMHTMLELEMENT(nsGenericHTMLFormElement::)
> 
>   // nsIDOMHTMLButtonElement
>   NS_DECL_NSIDOMHTMLBUTTONELEMENT
> 
>   // overriden nsIFormControl methods
>   NS_IMETHOD_(PRUint32) GetType() const { return mType; }
>   NS_IMETHOD Reset();
>-  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                               nsIContent* aSubmitElement);
>+  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission);
>   NS_IMETHOD SaveState();
>   PRBool RestoreState(nsPresState* aState);
> 
>   /**
>    * Called when an attribute is about to be changed
>    */
>   virtual nsresult BeforeSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
>                                  const nsAString* aValue, PRBool aNotify);
>@@ -510,25 +509,24 @@ nsHTMLButtonElement::SetDefaultValue(con
> 
> NS_IMETHODIMP
> nsHTMLButtonElement::Reset()
> {
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
>-nsHTMLButtonElement::SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                                       nsIContent* aSubmitElement)
>+nsHTMLButtonElement::SubmitNamesValues(nsFormSubmission* aFormSubmission)
> {
>   nsresult rv = NS_OK;
> 
>   //
>   // We only submit if we were the button pressed
>   //
>-  if (aSubmitElement != this) {
>+  if (aFormSubmission->GetOriginatingElement() != this) {
>     return NS_OK;
>   }
> 
>   //
>   // Disabled elements don't submit
>   //
>   PRBool disabled;
>   rv = GetDisabled(&disabled);
>diff --git a/content/html/content/src/nsHTMLFieldSetElement.cpp b/content/html/content/src/nsHTMLFieldSetElement.cpp
>--- a/content/html/content/src/nsHTMLFieldSetElement.cpp
>+++ b/content/html/content/src/nsHTMLFieldSetElement.cpp
>@@ -63,18 +63,17 @@ public:
>   NS_FORWARD_NSIDOMHTMLELEMENT(nsGenericHTMLFormElement::)
> 
>   // nsIDOMHTMLFieldSetElement
>   NS_DECL_NSIDOMHTMLFIELDSETELEMENT
> 
>   // nsIFormControl
>   NS_IMETHOD_(PRUint32) GetType() const { return NS_FORM_FIELDSET; }
>   NS_IMETHOD Reset();
>-  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                               nsIContent* aSubmitElement);
>+  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission);
>   virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const;
>   virtual nsXPCClassInfo* GetClassInfo();
> };
> 
> // construction, destruction
> 
> 
> NS_IMPL_NS_NEW_HTML_ELEMENT(FieldSet)
>@@ -123,13 +122,12 @@ nsHTMLFieldSetElement::GetForm(nsIDOMHTM
> 
> nsresult
> nsHTMLFieldSetElement::Reset()
> {
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
>-nsHTMLFieldSetElement::SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                                         nsIContent* aSubmitElement)
>+nsHTMLFieldSetElement::SubmitNamesValues(nsFormSubmission* aFormSubmission)
> {
>   return NS_OK;
> }
>diff --git a/content/html/content/src/nsHTMLFormElement.cpp b/content/html/content/src/nsHTMLFormElement.cpp
>--- a/content/html/content/src/nsHTMLFormElement.cpp
>+++ b/content/html/content/src/nsHTMLFormElement.cpp
>@@ -746,23 +746,23 @@ nsHTMLFormElement::BuildSubmission(nsFor
>     }
>   }
> 
>   nsresult rv;
> 
>   //
>   // Get the submission object
>   //
>-  rv = GetSubmissionFromForm(this, aFormSubmission);
>+  rv = GetSubmissionFromForm(this, originatingElement, aFormSubmission);
>   NS_ENSURE_SUBMIT_SUCCESS(rv);
> 
>   //
>   // Dump the data into the submission object
>   //
>-  rv = WalkFormElements(*aFormSubmission, originatingElement);
>+  rv = WalkFormElements(*aFormSubmission);
>   NS_ENSURE_SUBMIT_SUCCESS(rv);
> 
>   return NS_OK;
> }
> 
> nsresult
> nsHTMLFormElement::SubmitSubmission(nsFormSubmission* aFormSubmission)
> {
>@@ -937,30 +937,29 @@ nsHTMLFormElement::NotifySubmitObservers
>     }
>   }
> 
>   return rv;
> }
> 
> 
> nsresult
>-nsHTMLFormElement::WalkFormElements(nsFormSubmission* aFormSubmission,
>-                                    nsIContent* aSubmitElement)
>+nsHTMLFormElement::WalkFormElements(nsFormSubmission* aFormSubmission)
> {
>   nsTArray<nsGenericHTMLFormElement*> sortedControls;
>   nsresult rv = mControls->GetSortedControls(sortedControls);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   //
>   // Walk the list of nodes and call SubmitNamesValues() on the controls
>   //
>   PRUint32 len = sortedControls.Length();
>   for (PRUint32 i = 0; i < len; ++i) {
>     // Tell the control to submit its name/value pairs to the submission
>-    sortedControls[i]->SubmitNamesValues(aFormSubmission, aSubmitElement);
>+    sortedControls[i]->SubmitNamesValues(aFormSubmission);
>   }
> 
>   return NS_OK;
> }
> 
> // nsIForm
> 
> NS_IMETHODIMP_(PRUint32)
>@@ -1478,17 +1477,17 @@ nsHTMLFormElement::SetEncoding(const nsA
>   return SetEnctype(aEncoding);
> }
> 
> NS_IMETHODIMP
> nsHTMLFormElement::GetFormData(nsIDOMFormData** aFormData)
> {
>   nsRefPtr<nsFormData> fd = new nsFormData();
> 
>-  nsresult rv = WalkFormElements(fd, nsnull);
>+  nsresult rv = WalkFormElements(fd);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   *aFormData = fd.forget().get();
> 
>   return NS_OK;
> }
>  
> NS_IMETHODIMP    
>diff --git a/content/html/content/src/nsHTMLFormElement.h b/content/html/content/src/nsHTMLFormElement.h
>--- a/content/html/content/src/nsHTMLFormElement.h
>+++ b/content/html/content/src/nsHTMLFormElement.h
>@@ -299,20 +299,18 @@ protected:
>    * @param aFormSubmission the submission object
>    */
>   nsresult SubmitSubmission(nsFormSubmission* aFormSubmission);
>   /**
>    * Walk over the form elements and call SubmitNamesValues() on them to get
>    * their data pumped into the FormSubmitter.
>    *
>    * @param aFormSubmission the form submission object
>-   * @param aSubmitElement the element that was clicked on (nsnull if none)
>    */
>-  nsresult WalkFormElements(nsFormSubmission* aFormSubmission,
>-                            nsIContent* aSubmitElement);
>+  nsresult WalkFormElements(nsFormSubmission* aFormSubmission);
> 
>   /**
>    * Notify any submit observers of the submit.
>    *
>    * @param aActionURL the URL being submitted to
>    * @param aCancelSubmit out param where submit observers can specify that the
>    *        submit should be cancelled.
>    */
>diff --git a/content/html/content/src/nsHTMLInputElement.cpp b/content/html/content/src/nsHTMLInputElement.cpp
>--- a/content/html/content/src/nsHTMLInputElement.cpp
>+++ b/content/html/content/src/nsHTMLInputElement.cpp
>@@ -2427,32 +2427,31 @@ nsHTMLInputElement::Reset()
>     default:
>       break;
>   }
> 
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
>-nsHTMLInputElement::SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                                      nsIContent* aSubmitElement)
>+nsHTMLInputElement::SubmitNamesValues(nsFormSubmission* aFormSubmission)
> {
>   nsresult rv = NS_OK;
> 
>   // Disabled elements don't submit
>   // For type=reset, and type=button, we just never submit, period.
>   // For type=image and type=button, we only submit if we were the button
>   // pressed
>   // For type=radio and type=checkbox, we only submit if checked=true
>   PRBool disabled;
>   rv = GetDisabled(&disabled);
>   if (disabled || mType == NS_FORM_INPUT_RESET ||
>       mType == NS_FORM_INPUT_BUTTON ||
>       ((mType == NS_FORM_INPUT_SUBMIT || mType == NS_FORM_INPUT_IMAGE) &&
>-       aSubmitElement != this) ||
>+       aFormSubmission->GetOriginatingElement() != this) ||
>       ((mType == NS_FORM_INPUT_RADIO || mType == NS_FORM_INPUT_CHECKBOX) &&
>        !GetChecked())) {
>     return NS_OK;
>   }
> 
>   // Get the name
>   nsAutoString name;
>   PRBool nameThere = GetNameIfExists(name);
>diff --git a/content/html/content/src/nsHTMLInputElement.h b/content/html/content/src/nsHTMLInputElement.h
>--- a/content/html/content/src/nsHTMLInputElement.h
>+++ b/content/html/content/src/nsHTMLInputElement.h
>@@ -110,18 +110,17 @@ public:
>   {
>     return nsGenericHTMLElement::GetEditor(aEditor);
>   }
>   NS_IMETHOD SetUserInput(const nsAString& aInput);
> 
>   // Overriden nsIFormControl methods
>   NS_IMETHOD_(PRUint32) GetType() const { return mType; }
>   NS_IMETHOD Reset();
>-  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                               nsIContent* aSubmitElement);
>+  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission);
>   NS_IMETHOD SaveState();
>   virtual PRBool RestoreState(nsPresState* aState);
>   virtual PRBool AllowDrop();
> 
>   // nsIContent
>   virtual PRBool IsHTMLFocusable(PRBool aWithMouse, PRBool *aIsFocusable, PRInt32 *aTabIndex);
> 
>   virtual PRBool ParseAttribute(PRInt32 aNamespaceID,
>diff --git a/content/html/content/src/nsHTMLLabelElement.cpp b/content/html/content/src/nsHTMLLabelElement.cpp
>--- a/content/html/content/src/nsHTMLLabelElement.cpp
>+++ b/content/html/content/src/nsHTMLLabelElement.cpp
>@@ -72,18 +72,17 @@ public:
>   NS_FORWARD_NSIDOMHTMLELEMENT(nsGenericHTMLFormElement::)
> 
>   // nsIDOMHTMLLabelElement
>   NS_DECL_NSIDOMHTMLLABELELEMENT
> 
>   // nsIFormControl
>   NS_IMETHOD_(PRUint32) GetType() const { return NS_FORM_LABEL; }
>   NS_IMETHOD Reset();
>-  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                               nsIContent* aSubmitElement);
>+  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission);
> 
>   NS_IMETHOD Focus();
> 
>   // nsIContent
>   virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
>                               nsIContent* aBindingParent,
>                               PRBool aCompileEventHandlers);
>   virtual void UnbindFromTree(PRBool aDeep = PR_TRUE,
>@@ -341,18 +340,17 @@ nsHTMLLabelElement::PostHandleEvent(nsEv
> 
> nsresult
> nsHTMLLabelElement::Reset()
> {
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
>-nsHTMLLabelElement::SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                                      nsIContent* aSubmitElement)
>+nsHTMLLabelElement::SubmitNamesValues(nsFormSubmission* aFormSubmission)
> {
>   return NS_OK;
> }
> 
> nsresult
> nsHTMLLabelElement::SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName, nsIAtom* aPrefix,
>                             const nsAString& aValue, PRBool aNotify)
> {
>diff --git a/content/html/content/src/nsHTMLObjectElement.cpp b/content/html/content/src/nsHTMLObjectElement.cpp
>--- a/content/html/content/src/nsHTMLObjectElement.cpp
>+++ b/content/html/content/src/nsHTMLObjectElement.cpp
>@@ -99,18 +99,17 @@ public:
> 
>   // Overriden nsIFormControl methods
>   NS_IMETHOD_(PRUint32) GetType() const
>   {
>     return NS_FORM_OBJECT;
>   }
> 
>   NS_IMETHOD Reset();
>-  NS_IMETHOD SubmitNamesValues(nsFormSubmission *aFormSubmission,
>-                               nsIContent *aSubmitElement);
>+  NS_IMETHOD SubmitNamesValues(nsFormSubmission *aFormSubmission);
> 
>   virtual nsresult DoneAddingChildren(PRBool aHaveNotified);
>   virtual PRBool IsDoneAddingChildren();
> 
>   virtual PRBool ParseAttribute(PRInt32 aNamespaceID,
>                                 nsIAtom *aAttribute,
>                                 const nsAString &aValue,
>                                 nsAttrValue &aResult);
>@@ -315,18 +314,17 @@ nsHTMLObjectElement::GetDesiredIMEState(
> 
> NS_IMETHODIMP
> nsHTMLObjectElement::Reset()
> {
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
>-nsHTMLObjectElement::SubmitNamesValues(nsFormSubmission *aFormSubmission,
>-                                       nsIContent *aSubmitElement)
>+nsHTMLObjectElement::SubmitNamesValues(nsFormSubmission *aFormSubmission)
> {
>   nsAutoString name;
>   if (!GetAttr(kNameSpaceID_None, nsGkAtoms::name, name)) {
>     // No name, don't submit.
> 
>     return NS_OK;
>   }
> 
>diff --git a/content/html/content/src/nsHTMLOutputElement.cpp b/content/html/content/src/nsHTMLOutputElement.cpp
>--- a/content/html/content/src/nsHTMLOutputElement.cpp
>+++ b/content/html/content/src/nsHTMLOutputElement.cpp
>@@ -63,18 +63,17 @@ public:
>   NS_FORWARD_NSIDOMHTMLELEMENT(nsGenericHTMLFormElement::)
> 
>   // nsIDOMHTMLOutputElement
>   NS_DECL_NSIDOMHTMLOUTPUTELEMENT
> 
>   // nsIFormControl
>   NS_IMETHOD_(PRUint32) GetType() const { return NS_FORM_OUTPUT; }
>   NS_IMETHOD Reset();
>-  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                               nsIContent* aSubmitElement);
>+  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission);
> 
>   nsresult Clone(nsINodeInfo* aNodeInfo, nsINode** aResult) const;
> 
>   PRBool ParseAttribute(PRInt32 aNamespaceID, nsIAtom* aAttribute,
>                         const nsAString& aValue, nsAttrValue& aResult);
> 
>   // This function is called when a callback function from nsIMutationObserver
>   // has to be used to update the defaultValue attribute.
>@@ -143,18 +142,17 @@ nsHTMLOutputElement::Reset()
> {
>   mValueModeFlag = eModeDefault;
>   nsresult rv = nsContentUtils::SetNodeTextContent(this, mDefaultValue,
>                                                    PR_TRUE);
>   return rv;
> }
> 
> NS_IMETHODIMP
>-nsHTMLOutputElement::SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                                       nsIContent* aSubmitElement)
>+nsHTMLOutputElement::SubmitNamesValues(nsFormSubmission* aFormSubmission)
> {
>   // The output element is not submittable.
>   return NS_OK;
> }
> 
> PRBool
> nsHTMLOutputElement::ParseAttribute(PRInt32 aNamespaceID, nsIAtom* aAttribute,
>                                     const nsAString& aValue, nsAttrValue& aResult)
>diff --git a/content/html/content/src/nsHTMLSelectElement.cpp b/content/html/content/src/nsHTMLSelectElement.cpp
>--- a/content/html/content/src/nsHTMLSelectElement.cpp
>+++ b/content/html/content/src/nsHTMLSelectElement.cpp
>@@ -1591,18 +1591,17 @@ nsHTMLSelectElement::Reset()
>   DispatchContentReset();
> 
>   return NS_OK;
> }
> 
> static NS_DEFINE_CID(kFormProcessorCID, NS_FORMPROCESSOR_CID);
> 
> NS_IMETHODIMP
>-nsHTMLSelectElement::SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                                       nsIContent* aSubmitElement)
>+nsHTMLSelectElement::SubmitNamesValues(nsFormSubmission* aFormSubmission)
> {
>   nsresult rv = NS_OK;
> 
>   //
>   // Disabled elements don't submit
>   //
>   PRBool disabled;
>   rv = GetDisabled(&disabled);
>diff --git a/content/html/content/src/nsHTMLSelectElement.h b/content/html/content/src/nsHTMLSelectElement.h
>--- a/content/html/content/src/nsHTMLSelectElement.h
>+++ b/content/html/content/src/nsHTMLSelectElement.h
>@@ -264,18 +264,17 @@ public:
>   virtual PRBool IsHTMLFocusable(PRBool aWithMouse, PRBool *aIsFocusable, PRInt32 *aTabIndex);
>   virtual nsresult InsertChildAt(nsIContent* aKid, PRUint32 aIndex,
>                                  PRBool aNotify);
>   virtual nsresult RemoveChildAt(PRUint32 aIndex, PRBool aNotify, PRBool aMutationEvent = PR_TRUE);
> 
>   // Overriden nsIFormControl methods
>   NS_IMETHOD_(PRUint32) GetType() const { return NS_FORM_SELECT; }
>   NS_IMETHOD Reset();
>-  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                               nsIContent* aSubmitElement);
>+  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission);
>   NS_IMETHOD SaveState();
>   virtual PRBool RestoreState(nsPresState* aState);
> 
>   // nsISelectElement
>   NS_DECL_NSISELECTELEMENT
> 
>   /**
>    * Called when an attribute is about to be changed
>diff --git a/content/html/content/src/nsHTMLTextAreaElement.cpp b/content/html/content/src/nsHTMLTextAreaElement.cpp
>--- a/content/html/content/src/nsHTMLTextAreaElement.cpp
>+++ b/content/html/content/src/nsHTMLTextAreaElement.cpp
>@@ -118,18 +118,17 @@ public:
>   {
>     return nsGenericHTMLElement::GetEditor(aEditor);
>   }
>   NS_IMETHOD SetUserInput(const nsAString& aInput);
> 
>   // nsIFormControl
>   NS_IMETHOD_(PRUint32) GetType() const { return NS_FORM_TEXTAREA; }
>   NS_IMETHOD Reset();
>-  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                               nsIContent* aSubmitElement);
>+  NS_IMETHOD SubmitNamesValues(nsFormSubmission* aFormSubmission);
>   NS_IMETHOD SaveState();
>   virtual PRBool RestoreState(nsPresState* aState);
> 
>   // nsITextControlElemet
>   NS_IMETHOD SetValueChanged(PRBool aValueChanged);
>   NS_IMETHOD_(PRBool) IsSingleLineTextControl() const;
>   NS_IMETHOD_(PRBool) IsTextArea() const;
>   NS_IMETHOD_(PRBool) IsPlainTextControl() const;
>@@ -832,18 +831,17 @@ nsHTMLTextAreaElement::Reset()
>     rv = SetValue(resetVal);
>     NS_ENSURE_SUCCESS(rv, rv);
>   }
>   SetValueChanged(PR_FALSE);
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
>-nsHTMLTextAreaElement::SubmitNamesValues(nsFormSubmission* aFormSubmission,
>-                                         nsIContent* aSubmitElement)
>+nsHTMLTextAreaElement::SubmitNamesValues(nsFormSubmission* aFormSubmission)
> {
>   nsresult rv = NS_OK;
> 
>   //
>   // Disabled elements don't submit
>   //
>   PRBool disabled;
>   rv = GetDisabled(&disabled);

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Sorry about that :(

For what it's worth, I don't think you need sr on that patch. Though if you prefer definitely go for it.

Comment 15

[Mounir Lamouri (:mounir)]

Comment on attachment 466893 [details] [diff] [review]
Part 1 - Move originating element to nsFormSubmission

So better not waste Olli time :)

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 466937 [details] [diff] [review]
Part 2 - Implement formtarget

>+  nsCOMPtr<nsIFormControl> formControl =
>+    do_QueryInterface(aFormSubmission->GetOriginatingElement());
>+  if (!(formControl && formControl->IsSubmitControl() &&
>+        aFormSubmission->GetOriginatingElement()->GetAttr(kNameSpaceID_None,
>+                                                          nsGkAtoms::formtarget,
>+                                                          target)) &&
>+      !GetAttr(kNameSpaceID_None, nsGkAtoms::target, target)) {

Is there a reason to check IsSubmitControl() here? It must be a submit control otherwise it couldn't be the originating element, right? I think you can remove the call as well as the QI.

r=me with that.

Comment 17

[Mounir Lamouri (:mounir)]

This is now in the tree.
Changesets:
http://hg.mozilla.org/mozilla-central/rev/491b7284f637
http://hg.mozilla.org/mozilla-central/rev/73ad6efd7530