Issue #8113: Support file, about and data schemes as form action.

[simartin]

Fix #8113 by supporting those schemes as form action and unit test the "about:blank" case

[jdm]

Great work, especially on figuring out a way to test it :) I've got some suggestions to improve that below.
-S-awaiting-review +S-needs-code-changes

---

Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

^{components/script/dom/htmlformelement.rs, line 210 [r1] (raw file):}
Let's do:

// https://html.spec.whatwg.org/multipage/#submit-get-action
("file", _) | ("about", _) | ("data", FormMethod::FormGet) |
("ftp", _) | ("javascript", _) => (),

---

^{tests/ref/basic.list, line 23 [r1] (raw file):}
I don't think this test needs to be a reftest. Further suggestions below!

---

^{tests/ref/form_submit_about.html, line 8 [r1] (raw file):}
I propose a WPT test (./mach create-wpt tests/wpt/mozilla/tests/mozilla/form_submit_about.html) which contains an <iframe>. The page in the iframe should contain a form that submits itself, targeted at about:blank. The test page can watch the iframe's onload and pass if there are two load events. Does that make sense?

---

^{tests/ref/form_submit_about_ref.html, line 1 [r1] (raw file):}
We can remove this file.

---

Comments from the review on Reviewable.io

[bors-servo]

☔ The latest upstream changes (presumably #8354) made this pull request unmergeable. Please resolve the merge conflicts.

[simartin]

^{tests/ref/form_submit_about.html, line 8 [r1] (raw file):}
It makes perfect sense however I've failed so far unfortunately, and it looks to me like there might be another issue in Servo... Creating a test as you suggested, the iframe's onload is called only once, while it's called twice (as it should be) with Firefox or Chrome...

---

Comments from the review on Reviewable.io

[simartin]

Will resolve once the discussion with jdm on testing is complete

---

Comments from the review on Reviewable.io

[jdm]

@simartin What is the target of the form that gets submitted? What if you make a blank.html in the same directory and use that as the target? Does the missing onload event occur?

[simartin]

In that case the onload event occurs... I guess there's something special with about:blank; I'll try with about:failure, that I understand will load failure.html

[eefriedman]

The thing that's special about about:blank is that it's cross-origin... getting the load event to fire correctly depends on getting cross-origin wrappers working.

[jdm]

Actually cross-origin iframe load events are part of #6677.

[simartin]

@jdm Thanks for the reference. So either we go with the initial way of testing that I proposed and close this ticket (and we move to wpt-testing when #6677 is fixed), or we defer closure until #6677 is closed. What would you suggest as next step?

[jdm]

#8558 is ready for review, so I'd like to merge that and then get the test using about:blank working.

[simartin]

Awesome! I'll keep an eye on #8558 and resume writing a wpt along with this patch.

[simartin]

The fix for #8558 has been merged, hence resuming work on this issue

[jdm]

💃

[simartin]

wpt test added as suggested

---

Comments from the review on Reviewable.io

[jdm]

@bors-servo: r+
Thanks for finishing this up!

---

Reviewed 7 of 7 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io

[bors-servo]

📌 Commit 2b332a2 has been approved by jdm

[bors-servo]

⌛ Testing commit 2b332a2 with merge 1a7d393...

[bors-servo]

💔 Test failed - linux-rel

[jdm]

@bors-servo: retry

• #8802 and #8769

[bors-servo]

⌛ Testing commit 2b332a2 with merge 18f74a3...

[bors-servo]

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt