Skip to content

form-associated custom elements and their ElementInternals #25705

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

form-associated custom elements and their ElementInternals #25705

wants to merge 1 commit into from

Conversation

@pshaughn
Copy link
Contributor

@pshaughn pshaughn commented Feb 7, 2020 •
edited
Loading

Form-associated elements are now working.

Remaining test failures in custom-elements/form-associated/ are:

  • Allow setting default accessibility semantics for custom elements whatwg/html#4658 hasn't made it into the standard yet, and ElementInternals-accessibility is all about that functionality.
  • ElementInternals-setFormValue hits many race conditions with our iframe order of operations. Isolating just the "submit this form with these custom elements, look at the query" part, the payload of each test actually does pass! Running the tests as written, we end up with different tests overwriting each other's elements and getting very confused. One problem is the about:blank double-load-event thing, but fixing that doesn't fix everything.
  • ElementInternals-validation doesn't work because we don't have form validation in general; I've commented the relevant stubs.
  • ElementInternals-form-disabled-callback fails on Mutating the tabindex content attribute isn't changing focusability #25713.

fix #24988 fix #25015 fix #25017

@highfive
Copy link

highfive commented Feb 7, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webidls/ElementInternals.webidl, components/script/dom/mod.rs, components/script/dom/node.rs, components/script/dom/validitystate.rs, components/script/dom/htmlformelement.rs and 2 more
  • @KiChjang: components/script/dom/webidls/ElementInternals.webidl, components/script/dom/mod.rs, components/script/dom/node.rs, components/script/dom/validitystate.rs, components/script/dom/htmlformelement.rs and 2 more

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 7, 2020
@highfive
Copy link

highfive commented Feb 7, 2020

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!

@servo-wpt-sync servo-wpt-sync mentioned this pull request Feb 8, 2020
Closed
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Feb 8, 2020

Opened new PR for upstreamable changes.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21678.

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Feb 9, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21678.

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Feb 9, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21678.

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Feb 13, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21678.

@pshaughn
Copy link
Contributor Author

pshaughn commented Feb 13, 2020

I made an incidental change to a shared test .js file that might have repercussions.
@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Feb 13, 2020

⌛ Trying commit 76315b5 with merge 002fc3b...

bors-servo pushed a commit that referenced this pull request Feb 13, 2020
Auto merge of #25705 - pshaughn:elementinternals, r=<try>
002fc3b 
sketching ElementInternals

Form-associated elements are now working.

Remaining test failures in custom-elements/form-associated/ are:
* whatwg/html#4658 hasn't made it into the standard yet, and ElementInternals-accessibility is all about that functionality.
* ElementInternals-setFormValue hits many race conditions with our iframe order of operations. Isolating just the "submit this form with these custom elements, look at the query" part, the payload of each test actually does pass! Running the tests as written, we end up with different tests overwriting each other's elements and getting very confused. One problem is the about:blank double-load-event thing, but fixing that doesn't fix everything.
* ElementInternals-validation doesn't work because we don't have form validation in general; I've commented the relevant stubs.
* ElementInternals-form-disabled-callback fails on #25713.
@pshaughn pshaughn changed the title sketching ElementInternals form-associated custom elements and their ElementInternals Feb 13, 2020
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Feb 13, 2020

PR title changed; changed existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21678.

@pshaughn pshaughn marked this pull request as ready for review February 13, 2020 01:03
@bors-servo
Copy link
Contributor

bors-servo commented Feb 13, 2020

💔 Test failed - status-taskcluster

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Feb 13, 2020
@pshaughn
Copy link
Contributor Author

pshaughn commented Feb 13, 2020

Just the usual new-exposed-functionality-exists tests I invariably forget to update the metadata for on my first try.

@highfive highfive removed the S-tests-failed The changes caused existing tests to fail. label Feb 13, 2020
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Feb 13, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21678.

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Feb 14, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21678.

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Feb 14, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21678.

@pshaughn
Copy link
Contributor Author

pshaughn commented Feb 14, 2020

web-platform-tests/wpt#21747 just merged; I'll adjust for it in the morning.

@pshaughn
Copy link
Contributor Author

pshaughn commented Feb 14, 2020

I'll also add a _mozilla version of the setFormValue test that works serially and doesn't race iframe loads against promise reactions.

@bors-servo
Copy link
Contributor

bors-servo commented Feb 14, 2020

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

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Feb 14, 2020
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Feb 14, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21678.

@pshaughn pshaughn mentioned this pull request Feb 15, 2020
Closed
@bors-servo
Copy link
Contributor

bors-servo commented Feb 20, 2020

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

@cathiechen cathiechen mentioned this pull request Apr 2, 2024
Merged
4 tasks
@servo-wpt-sync servo-wpt-sync mentioned this pull request Apr 2, 2024
Merged
@mrobinson
Copy link
Member

mrobinson commented Apr 10, 2024

Closing this in favor of #31980.

@mrobinson mrobinson closed this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@jdm jdm

Labels

S-awaiting-review There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors.

Projects

None yet

Milestone

No milestone

6 participants

@pshaughn @highfive @servo-wpt-sync @bors-servo @mrobinson @jdm