Implement the Form Owner concept

[mukilan]

Implement the core logic for form owners (#3553).

NOTE: servo/html5ever#137 must land to be able to build this PR.
This PR only implements the logic necessary to associate a form control with its owner and reset them as required. It does not implement the other related interfaces such as HTMLFormControlCollections (form.elements) and the named getter for HTMLFormElement.

[hoppipolla-critic-bot]

Critic review: https://critic.hoppipolla.co.uk/r/5539

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

[bors-servo]

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

[bors-servo]

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

[mukilan]

Review status: 0 of 34 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

^{components/script/dom/htmlformelement.rs, line 595 [r3] (raw file):}
I'm certain that this step is unnecessary. But I am not sure why it is in the spec.

---

Comments from the review on Reviewable.io

[mukilan]

Review status: 0 of 34 files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

^{components/script/dom/htmlformelement.rs, line 595 [r3] (raw file):}
Oops, I accidentally clicked 'Resolve'.

---

Comments from the review on Reviewable.io

[metajack]

@Ms2ger ping

[jdm]

FTR: I am about 50% reviewing the first commit here.

[jdm]

Thank you so much for doing this work! The tests in particular are great; my only concern there is whether there are any tests for fragment parsing that already exist or if we should write some too.

-S-awaiting-review +S-needs-code-changes

---

Reviewed 25 of 25 files at r1, 5 of 5 files at r2, 6 of 6 files at r3.
Review status: all files reviewed at latest revision, 26 unresolved discussions, some commit checks failed.

---

^{components/script/dom/document.rs, line 472 [r1] (raw file):}
if let instead.

---

^{components/script/dom/htmlbuttonelement.rs, line 26 [r1] (raw file):}
nit: util is after string_cache alphabetically.

---

^{components/script/dom/htmlbuttonelement.rs, line 227 [r1] (raw file):}
I think this can use get_rooted instead (same for all other implementations).

---

^{components/script/dom/htmlbuttonelement.rs, line 234 [r1] (raw file):}
Why this change?

---

^{components/script/dom/htmlformelement.rs, line 613 [r1] (raw file):}
Prefer if let over map with side effects.

---

^{components/script/dom/htmlformelement.rs, line 618 [r1] (raw file):}
https://html.spec.whatwg.org/multipage/#association-of-controls-and-forms:category-form-attr-3

---

^{components/script/dom/htmlformelement.rs, line 632 [r1] (raw file):}
https://html.spec.whatwg.org/multipage/#association-of-controls-and-forms:category-form-attr-3

---

^{components/script/dom/htmlformelement.rs, line 636 [r1] (raw file):}
We don't care about being in the document here?

---

^{components/script/dom/htmlformelement.rs, line 642 [r1] (raw file):}
https://html.spec.whatwg.org/multipage/#association-of-controls-and-forms:category-form-attr-3

---

^{components/script/dom/htmlformelement.rs, line 643 [r1] (raw file):}
Do we not care about being in the document?

---

^{components/script/dom/htmlformelement.rs, line 646 [r1] (raw file):}
https://html.spec.whatwg.org/multipage/#association-of-controls-and-forms:form-associated-element-8

---

^{components/script/dom/htmlformelement.rs, line 662 [r1] (raw file):}
https://html.spec.whatwg.org/multipage/#association-of-controls-and-forms:form-associated-element-10

---

^{components/script/dom/htmlformelement.rs, line 722 [r1] (raw file):}
nit: for control in &to_reset {

---

^{components/script/dom/htmlformelement.rs, line 726 [r1] (raw file):}
I suppose this is necessary for the weird cases where elements can have a form owner that is not actually a parent?

---

^{components/script/dom/htmlobjectelement.rs, line 30 [r1] (raw file):}
nit: default comes before sync alphabetically.

---

^{components/script/dom/htmlselectelement.rs, line 27 [r1] (raw file):}
nit: default before borrow

---

^{components/script/dom/node.rs, line 168 [r1] (raw file):}
nit: add a \ here

---

^{components/script/dom/node.rs, line 316 [r3] (raw file):}
A comment explaining why it's important to perform these operations separately would help. I had to think about it for a minute.

---

^{components/script/dom/node.rs, line 2691 [r1] (raw file):}
nit: self.is_empty()

---

^{components/script/dom/node.rs, line 2703 [r1] (raw file):}
Why would head == self.len() at any point before the traverse_preorder iterator is exhausted?

---

^{components/script/dom/node.rs, line 2707 [r1] (raw file):}
Could you walk me through how this algorithm works? I'm having trouble understanding how it maintains tree order sorting. Maybe we should also assert that the element being inserted shares the same home subtree?

---

^{tests/wpt/mozilla/tests/mozilla/form_attribute.html, line 104 [r3] (raw file):}
nit: unindent

---

^{tests/wpt/mozilla/tests/mozilla/form_attribute.html, line 149 [r3] (raw file):}
nit: extra space after =

---

^{tests/wpt/mozilla/tests/mozilla/form_attribute.html, line 175 [r3] (raw file):}
s/the //?

---

^{tests/wpt/mozilla/tests/mozilla/form_attribute.html, line 229 [r3] (raw file):}
Doesn't this contradict "When an element changes its parent node resulting in a form-associated element and its form owner (if any) no longer being in the same home subtree, then the user agent must reset the form owner of that form-associated element.", which then would reach step 5 in https://html.spec.whatwg.org/multipage/forms.html#reset-the-form-owner ?

---

Comments from the review on Reviewable.io

[frewsxcv]

Any updates on this?

[mukilan]

Sorry for the delay. I've been meaning to address the review comments and complete this PR, but I couldn't find the time. I'll definitely look into it this weekend.

[jdm]

Thanks @mukilan!

[KiChjang]

@mukilan do you need any help?

[jdm]

Closing due to lack of updates.