Closed Bug 1366361 Opened 2 years ago Closed 2 years ago

Make form.action return the actual form submission URL

Categories

(Core :: DOM: Core & HTML, enhancement, P3)

55 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: d, Assigned: jessica)

References

(Depends on 1 open bug)

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(2 files, 3 obsolete files)

Tests at http://w3c-test.org/html/semantics/forms/the-form-element/form-action-reflection.html and http://w3c-test.org/html/semantics/forms/the-form-element/form-action-reflection-with-base-url.html

Spec at https://html.spec.whatwg.org/multipage/forms.html#dom-fs-action

Per the separate tests at http://w3c-test.org/html/semantics/forms/the-form-element/form-action-submission.html and http://w3c-test.org/html/semantics/forms/the-form-element/form-action-submission-with-base-url.html, Firefox uses the spec's algorithm for determining the actual URL to which the form submits. However, it does not return the correct URL for form.action.

No browser seems to follow the spec here, but they violate it in different ways, so this is probably safe to change.
Priority: -- → P3
Assignee: nobody → jjong
The same rule should apply to input.formAction and button.formAction as well:
https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#dom-fs-formaction
Attached patch Part 2: update related tests. (obsolete) — Splinter Review
Attachment #8885619 - Flags: review?(bugs)
Comment on attachment 8885618 [details] [diff] [review]
Part 1: add GetAction and GetFormAction implementation.

>+HTMLFormElement::GetAction(nsString& aValue)
>+{
>+  if (!GetAttr(kNameSpaceID_None, nsGkAtoms::action, aValue) ||
>+      aValue.IsEmpty()) {
>+    nsIDocument *document = OwnerDoc();
Nit, * goes with the type. Same also elsewhere.
Attachment #8885618 - Flags: review?(bugs) → review+
Attachment #8885619 - Flags: review?(bugs) → review+
Domenic, have you tested how action or formaction is reflected in different browsers when pushState has been used?
Flags: needinfo?(d)
Nope. Web platform test contributions would be great in that area.
Flags: needinfo?(d)
Fixed nits: * goes with the type. Carrying r+.
Attachment #8885618 - Attachment is obsolete: true
Attachment #8886070 - Flags: review+
Attaching the right patch. :/
Attachment #8886070 - Attachment is obsolete: true
Attachment #8886071 - Flags: review+
Olli, could you take a look again? I added wpt for pushState cases. Note that wpt for form.action has not been merged into our codebase yet, so I didn't add tests for it.
Attachment #8885619 - Attachment is obsolete: true
Attachment #8886072 - Flags: review?(bugs)
Attachment #8886072 - Attachment description: Part 2: update related tests. → Part 2: update related tests, v1.
Comment on attachment 8886072 [details] [diff] [review]
Part 2: update related tests, v1.

Is there wpt for .action and pushState?
Attachment #8886072 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #10)
> Comment on attachment 8886072 [details] [diff] [review]
> Part 2: update related tests, v1.
> 
> Is there wpt for .action and pushState?

wpt for .action with/without @action (the ones in comment 0) hasn't been merged into our codebase yet, so I didn't add them.
Needs rebasing.
Keywords: checkin-needed
Pushed by jjong@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c1a86e1296b
Part 1: .action/formAction should return the document's URL if @action/formaction is missing or empty. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f7663ac2068
Part 2: Update reflectURL() and web platform tests. r=smaug
https://hg.mozilla.org/mozilla-central/rev/1c1a86e1296b
https://hg.mozilla.org/mozilla-central/rev/8f7663ac2068
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Depends on: 1412340
It's a bit late but posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2017/html-form-s-action-property-now-returns-document-url-if-action-attribute-is-empty-or-missing/

Given that only 1 real issue has been reported so far, I think we can simply move it to Tech Evangelism rather than restoring the previous behaviour.
You need to log in before you can comment on or make changes to this bug.