-
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: remove autofocused element before its task runs #11193
Conversation
See https://bugzilla.mozilla.org/show_bug.cgi?id=1463563 for context. We'll need to update -2 to figure out the correct pass condition. Given Chrome and Safari, I guess we should expect input2 and then we should update the specification to note that if the element is no longer connected you just abort the algorithm, meaning no flags get set.
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.
-2 looks good to me per spec: it will
- queue a task to focus
input
- run the focusing steps which do nothing
- set the "something has been autofocused" flag (implicit in step 8 right now)
- queue a task to focus
input2
- see the "something has been autofocused" flag is set, and do nothing
The first test also looks good, and is an interesting test. I'd suggest commenting the document.body.appendChild(input)
to explain that this removes the element, which causes the focus fixup rule to run removing focus from it.
Thanks for the review. -2 is good per spec, but per the Firefox issue it causes compat issues, so I guess we should change the spec. |
Should this have status:needs-spec-decision and a spec bug filed? (Trying to clear out approved and un-merged PRs!) |
By code inspection, Safari and Blink do very different things here. Safari has a per-element flag, not a per-document flag. Ick. |
I cannot understand Blink's behavior. @tkent-google, can you help? Given const input = document.createElement("input");
input.id = "i1";
input.autofocus = true;
document.body.appendChild(input);
input.remove();
const input2 = document.createElement("input");
input2.id = "i2";
input2.autofocus = true;
document.body.appendChild(input2); My reading of the code at SetAutofocusElement and its call sites is that the second insertion should do nothing. But, somehow, Blink focuses Safari focuses |
Both of WebKit and Blink trigger autofocus processing after style resolution, not on inserting into a document. So, autofocus of I'll file a specification issue. |
Filed a specification issue: whatwg/html#4645 |
Overtaken by whatwg/html#4763 and associated test PR. |
See https://bugzilla.mozilla.org/show_bug.cgi?id=1463563 for context.
We'll need to update -2 to figure out the correct pass condition. Given Chrome and Safari, I guess we should expect input2 and then we should update the specification to note that if the element is no longer connected you just abort the algorithm, meaning no flags get set.