-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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: Add a test for 'autofocus' as a global attribute #18558
Conversation
html/semantics/forms/autofocus/ should be moved to html/interaction/focus/autofocus/. I'd like to do it in a separated PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nit.
// doesn't work well. | ||
async function waitUntilStableAutofocusState(test) { | ||
// TODO: Update this for https://github.com/web-platform-tests/wpt/pull/17929 | ||
await timeOut(test, 100); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'm missing something, but won't this just cause the test to always time out before the rest of the test gets run?
https://github.com/web-platform-tests/wpt/pull/17929/files#diff-c05e2b49bb15cca07f18649490b4e74eR25 has a different implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I misremembered what step_timeout
does. Never mind.
Still, which of this PR and #17929 has the implementation you want?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still, which of this PR and #17929 has the implementation you want?
One in #17929 is the final implementation.
However, it's not correct until the specification change for #17929 is merged.
Added a test for SVG elements. |
Reassigning to @AmeliaBR to review the SVG test |
The HTML side of this has been merged into the spec. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a little more work to adapt the script to SVG elements, because those elements are only rendered within an SVG layout context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SVG tests look good to me now.
For whatwg/html#4830 and w3c/svgwg#728