Domenic Denicola Reporter
	Description • 7 years ago

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.

	Jessica Jong [:jessica] (inactive) Assignee
	Comment 1 • 7 years ago

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

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 4 • 7 years ago

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.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 5 • 7 years ago

Domenic, have you tested how action or formaction is reflected in different browsers when pushState has been used?

	Domenic Denicola Reporter
	Comment 6 • 7 years ago

Nope. Web platform test contributions would be great in that area.

	Jessica Jong [:jessica] (inactive) Assignee
	Comment 7 • 7 years ago

Fixed nits: * goes with the type. Carrying r+.

	Jessica Jong [:jessica] (inactive) Assignee
	Comment 8 • 7 years ago

Attaching the right patch. :/

	Jessica Jong [:jessica] (inactive) Assignee
	Comment 9 • 7 years ago

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.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 10 • 7 years ago

Comment on attachment 8886072 [details] [diff] [review]
Part 2: update related tests, v1.

Is there wpt for .action and pushState?

	Jessica Jong [:jessica] (inactive) Assignee
	Comment 11 • 7 years ago

(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.

	Jessica Jong [:jessica] (inactive) Assignee
	Comment 12 • 7 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=387678da52c472b0bd55df3e8c21bc53d7d66707&group_state=expanded

	Ryan VanderMeulen [:RyanVM]
	Comment 13 • 7 years ago

Needs rebasing.

	Pulsebot
	Comment 14 • 7 years ago

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

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 15 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/1c1a86e1296b
https://hg.mozilla.org/mozilla-central/rev/8f7663ac2068

	Chris Mills [:cmills]
	Comment 16 • 7 years ago

I've added notes to the relevant MDN pages to detail this change:

https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement#Browser_compatibility
https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement#Browser_compatibility
https://developer.mozilla.org/en-US/docs/Web/API/HTMLButtonElement#Browser_compatibility
https://developer.mozilla.org/en-US/Firefox/Releases/56#DOM

Let me know if that is OK. Thanks!

	Kohei Yoshino [:kohei]
	Comment 17 • 7 years ago

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.