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

Add notes explaining how <dialog>.showModal() focuses #5534

Merged
merged 1 commit into from
Jun 4, 2020
Merged

Add notes explaining how <dialog>.showModal() focuses #5534

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56626,8 +56626,17 @@ interface <dfn>HTMLDialogElement</dfn> : <span>HTMLElement</span> {

<li><p>Set the <code>dialog</code> to the <span>centered alignment</span> mode.</p></li>

<li><p>Let <var>subject</var>'s <span>node document</span> be <span data-x="blocked by a
modal dialog">blocked by the modal dialog</span> <var>subject</var>.</p></li>
<li>
<p>Let <var>subject</var>'s <span>node document</span> be <span data-x="blocked by a
modal dialog">blocked by the modal dialog</span> <var>subject</var>.</p>

<p class="note" id="note-dialog-plus-focus-fixup">This will cause the <span>focused area of the
document</span> to become <span>inert</span> (unless that currently focused area is a
<span>shadow-including descendant</span> of <var>subject</var>). In such cases, the <span>focus
fixup rule</span> will kick in and reset the <span>focused area of the document</span> to the
<span>viewport</span> for now. In a couple steps we will attempt to find a better candidate to
focus.</p>
</li>

<li><p>If <var>subject</var>'s <span>node document</span>'s <span>top layer</span> does not
already <span data-x="list contains">contain</span> <var>subject</var>, then <span
Expand All @@ -56653,7 +56662,13 @@ interface <dfn>HTMLDialogElement</dfn> : <span>HTMLElement</span> {
<p>If there isn't one of those either, then let <var>control</var> be <var>subject</var>.</p>
</li>

<li><p>Run the <span>focusing steps</span> for <var>control</var>.</p></li>
<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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It resets focus to the body/viewport instead of doing nothing, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that step does nothing. The step on line 56630 is the one that resets to the body/viewport.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@domenic domenic May 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Thanks!

Copy link
Contributor

@sefeng211 sefeng211 May 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

modal dialogs, this means that any <a href="#note-dialog-plus-focus-fixup">earlier
modifications</a> to the <span>focused area of the document</span> will apply.</p>
</li>

<li><p>Let <var>topDocument</var> be the <span>active document</span> of <var>control</var>'s
<span>node document</span>'s <span data-x="concept-document-bc">browsing context</span>'s
Expand Down