Skip to content
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

Autofocus element in a <dialog> tries to get focus twice #4788

Closed
tkent-google opened this issue Jul 19, 2019 · 7 comments · Fixed by #4918
Closed

Autofocus element in a <dialog> tries to get focus twice #4788

tkent-google opened this issue Jul 19, 2019 · 7 comments · Fixed by #4918
Labels
topic: focus

Comments

@tkent-google
Copy link
Contributor

The following code shows FOCUS log twice.

<!DOCTYPE html>
<pre></pre>
<dialog>
  <input autofocus onfocus="log()">
  <a href="http://www.google.com/">Help</a>
</dialog>

<script>
function log() {
  document.querySelector('pre').textContent += 'FOCUS\n';
}

document.querySelector('dialog').show();
document.activeElement.blur();
</script>
  1. The HTML parser inserts the input to the document tree, and it posts the autofocus task.
  2. dialog.show() sets focus on the input synchronously.
  3. When the task is executed, the input is focused again.
@rahulpurohit29
Copy link
Contributor

Should removing autofocus solve the problem??

@myakura
Copy link

myakura commented Jul 19, 2019

Should removing autofocus solve the problem??

yes. but 1) asking every author who's coded such to fix will not scale and 2) triggering the focus event yet the surrounding dialog hasn't been shown does seem a spec bug. I guess that's why tkent filed the issue.

@CYBAI
Copy link
Contributor

CYBAI commented Jul 19, 2019

Is this somehow same as #4356 ? 🤔

@domenic
Copy link
Member

domenic commented Jul 19, 2019

This seems unrelated to #4356, which involves neither dialog nor autofocus.

Some options, all based on #4763's processing model (where this will still be a problem, via a slightly different set of steps):

  • Leave as-is. This is an emergent combination of behaviors, and in practice may not be too bad of a problem.

  • Have the parser ignore autofocus in dialog when constructing the autofocus candidates list. This seems pretty reasonable, but I guess there might be slightly-more-complicated cases similar to the OP that this fails to solve?

  • Set the document's autofocus processed flag whenever anything gets focused. (Probably this means, whenever the focusing steps get run for anything.)

  • Remove autofocus behavior inside dialogs, as part of a general project to simplify/maybe eventually remove dialog, because dialog is currently a single-vendor feature. This would make the most sense if a non-Blink implementer supported dialogs-without-autofocus more than they supported dialogs-with-autofocus.

I don't have a strong opinion on any particular solution. I'll CC @rniwa since he has been helpful on focus thoughts from WebKit's perspective in the past.

@tkent-google
Copy link
Contributor Author

I don't think web developers have problems due to this behavior. This issue does't have high priority.

The second bullet in @domenic's comment sounds a simple solution. We can just add a step like "If there is a dialog which is an ancestor of the element, then return" to the autofocus steps.

@domenic
Copy link
Member

domenic commented Aug 1, 2019

If we change this we should also try to resolve #2393 while we're in the area. (Although it's unclear to me from reading that thread if there's any actual changes desired; more that we need to give the general dialog + autofocus space some thinking.)

@domenic
Copy link
Member

domenic commented Sep 18, 2019

@tkent-google would you be up for trying to resolve this and #2393 now that #4763 is merged?

tkent-google added a commit to tkent-google/html that referenced this issue Sep 19, 2019
@tkent-google
dialog, autofocus: Do not enqueue elements with autofocus attribute i…
9e6b1b6 
…nside a dialog element

This fixes whatwg#4788
tkent-google added a commit to web-platform-tests/wpt that referenced this issue Sep 19, 2019
@tkent-google
hmtl: Add a test for an autofocus issue with <dialog>
80c89ba 
whatwg/html#4788
This was referenced Sep 19, 2019
Merged
Merged
domenic pushed a commit that referenced this issue Nov 7, 2019
@tkent-google @domenic
Prevent double-focusing with autofocus="" inside <dialog>
b48bb22 
When the dialog focusing steps are called, we prevent further
autofocusing for the document. This fixes #4788.

Tests: web-platform-tests/wpt#19151
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic: focus
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

dialog, autofocus: Do not enqueue elements with autofocus attribute i… tkent-google/html
5 participants
@myakura @domenic @CYBAI @tkent-google @rahulpurohit29