New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Html option element.form#13111 #13126
Conversation
Heads up! This PR modifies the following files:
|
This does not seem to implement the logic described:
This seems to grab the parent, test of that is an opt group, and if so, return its form. Otherwise it returns the parent. Nowhere is select checked for or form invoked on these parents. Also, it seems like we should be returning None if the parent isn't opt group or a select. |
@metajack it checks if the parent is HTMLOptGroupElement, if it is - parent is set to grandparent, else to parent. Then it checks if the node, pointed by parent is HTMLSelectElement and returns its form. The None is forwarded through all the chain in all other cases. |
Ok. I have read the code a little more carefully this time, and it does appear to do the right thing. The extra checks I was looking for are just implicit in the and_then calls. |
@bors-servo r+ |
📌 Commit e4006fa has been approved by |
Html option element.form#13111 <!-- Please describe your changes on the following line: --> Add HTMLOptionElement form attribute support --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #13111 (github issue number if applicable). <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13126) <!-- Reviewable:end -->
💔 Test failed - linux-rel |
@metajack updated passing tests expectations. |
@bors-servo r=metajack |
📌 Commit 3b3b871 has been approved by |
Html option element.form#13111 <!-- Please describe your changes on the following line: --> Add HTMLOptionElement form attribute support --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #13111 (github issue number if applicable). <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13126) <!-- Reviewable:end -->
☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev |
Add HTMLOptionElement form attribute support
./mach build -d
does not report any errors./mach test-tidy
does not report any errorsThis change is