[HTML5][Patch] Make isindex magic in submission

RESOLVED FIXED

Status

()

defect
P1
normal
RESOLVED FIXED
10 years ago
4 months ago

People

(Reporter: hsivonen, Assigned: hsivonen)

Tracking

Other Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

To make sense for the DOM the HTML5 parser builds, the form submission code needs to do the IE-like magic when a text input is named isindex:
http://www.whatwg.org/specs/web-apps/current-work/multipage/forms.html#form-submission

'If the entry's name is "isindex", its type is "text", and this is the first entry in the form data set, then append the value to result and skip the rest of the substeps for this entry, moving on to the next entry, if any, or the next step in the overall algorithm otherwise.'
sicking: this bug may be relevant to your form submission cleanup.
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Attachment #429694 - Flags: review?(jonas)
Summary: [HTML5] Make isindex magic in submission → [HTML5][Patch] Make isindex magic in submission
Comment on attachment 429694 [details] [diff] [review]
Implement HTML5 exception for isindex

> nsresult
> nsFormData::AddNameValuePair(const nsAString& aName,
>-                             const nsAString& aValue)
>+                             const nsAString& aValue,
>+                             PRBool aInputTypeText)

I don't like fixing this by adding another argument to AddNameValuePair and thus moving some of the type logic into the submission, the code got quite nicer when I removed similar arguments in the recent cleanup IMHO.

An alternative approach would be to add something like the following functions to nsFormSubmission:


  virtual PRBool SupportsIsIndexSubmission()
  {
    return PR_FALSE;
  }

  virtual nsresult AddIsIndex(const nsAString& aValue)
  {
    NS_NOTREACHED("AddIsIndex called when not supported");
    return NS_ERROR_UNEXPECTED;
  }

And then override those in nsFSURLEncoded, and use them in nsHTMLInputElement::SubmitNamesValues.
Attachment #429694 - Flags: review?(jonas) → review-
I removed the last NS_FORM_INPUT_IMAGE check while I was at it, because it appears to be dead code considering the earlier NS_FORM_INPUT_IMAGE check.
Attachment #429694 - Attachment is obsolete: true
Attachment #431621 - Flags: review?(jonas)
Comment on attachment 431621 [details] [diff] [review]
Try again addressing review comments

>diff --git a/content/html/content/public/nsIFormSubmission.h b/content/html/content/public/nsIFormSubmission.h

>+  virtual PRBool SupportsIsIndexSubmission()
...
>+  virtual nsresult AddIsIndex(const nsAString& aValue)

Nit: Looking at it, I think casing it as "Isindex" rather than "IsIndex" is better. Treating it as one word makes it clearer that this is for <isindex> elements.

>diff --git a/content/html/content/src/nsHTMLInputElement.cpp b/content/html/content/src/nsHTMLInputElement.cpp
>+  else if (mType == NS_FORM_INPUT_TEXT &&
>+           aFormSubmission->SupportsIsIndexSubmission() &&
>+           name.EqualsLiteral("isindex")) {
>+    rv = aFormSubmission->AddIsIndex(value);

Nit: It's probably slightly faster to reverse the last two tests. Mostly because of the virtual call overhead, but also because the SupportsIsIndexSubmission test likely will test true much more often.

r=me with those nits fixed. Thanks for the patch!
Attachment #431621 - Flags: review?(jonas) → review+
Thanks. Pushed with the nits fixed.

http://hg.mozilla.org/mozilla-central/rev/a0061217d338
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Component: HTML: Form Submission → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.