Add notes explaining how <dialog>.showModal() focuses #5534
Conversation
It was unclear how the "focus fixup rule" came into play, since that rule is done using action at a distance. This inserts notes explicitly calling out what happens. See web-platform-tests/wpt#23485 for background.
| <li> | ||
| <p>Run the <span>focusing steps</span> for <var>control</var>.</p> | ||
|
|
||
| <p class="note">If <var>control</var> is not <span>focusable</span>, this will do nothing. For |
There was a problem hiding this comment.
It resets focus to the body/viewport instead of doing nothing, right?
There was a problem hiding this comment.
No, that step does nothing. The step on line 56630 is the one that resets to the body/viewport.
There was a problem hiding this comment.
Ah I see. I expected it to be here because in chrome's implementation it seems like it resets focus to body in the dialog focusing steps, but I trust this is equivalent.
There was a problem hiding this comment.
Yeah. I'm unsure how closely the "focus fixup rule" matches to implementation strategy. I guess it's possible that implementations monitor for all the various situations that make something unfocusable, and then use that to reset focus to the body. But probably more likely implementations have ad-hoc reset-focus-to-body calls on cases like the dialog focus steps, element removal, etc.
There was a problem hiding this comment.
How about show? Does it mean the thing that has been focused previously should remain focused if control is not focusable? I think Chrome's dialog focusing steps applies to both show and showModal.
There was a problem hiding this comment.
Sorry both for the delay in getting back to you on this.
Does it mean the thing that has been focused previously should remain focused if control is not focusable
This is correct, per the spec as-is (including after these informative notes get added).
I think Chrome's dialog focusing steps applies to both show and showModal.
Yep, you're right, ugh. https://boom-bath.glitch.me/dialog-open-focus.html shows this.
However, this seems like pretty buggy behavior in Chrome, because Chrome "clears the focus" without actually focusing the body element. That is, no focus events get fired on it.
As far as I can tell the way to spec what Chrome is doing would be to run the unfocusing steps for the focused area of the document if control is not focusable. This is pretty awkward and unprecedented.
I would rather treat this as a Chrome bug in the case of show(). What do you think, @josepharhar?
There was a problem hiding this comment.
As long as it doesn't break any websites I'm happy to change the behavior in chrome.
Would the difference just be to fire the focus event on <body> when the focus gets set to it?
There was a problem hiding this comment.
Not quite. The idea would be to change step 4 of the algorithm you cited at web-platform-tests/wpt#23485 (comment) to "do nothing" if the caller is show(). (If the caller is showModal(), then it stays as-is.)
This seems unlikely to break anything. My guess is that show() has pretty low usage (compared to showModal()), and it would just amount to a small user experience improvement. The only way it'd cause web-compat problems is if people were expecting document.activeElement to get reset to the body, but it's hard to imagine how code could depend on that.
|
Do nothing instead of “clearing focus,” sounds good! |
|
I think we also want tests for this change? |
|
@sefeng211 yeah, tests would be great! (Although note this isn't a change to the spec, just a clarification. It'll be a change for Chromium though.) Since you're implementing in Firefox, would you be able to write those tests (or modify any existing ones that don't match the spec)? |
|
This may also be an opportunity to revisit the unresolved discussion about focusing the dialog instead of the first focusable element. #4184 for the proposed spec change, which links to the various previous discussions. |
It was unclear how the "focus fixup rule" came into play, since that
rule is done using action at a distance. This inserts notes explicitly
calling out what happens.
See web-platform-tests/wpt#23485 for background.
/cc @sefeng211 and @josepharhar.
/interactive-elements.html ( diff )