Fix a dialog focus test that expects a wrong element to be focused

[sefeng211]

Per spec, when there are no descendant elements that match
the requirement, the dialog element itself should be focused.

Please feel free to correct me if I am wrong here.

[domenic]

This doesn't seem right. https://html.spec.whatwg.org/#focusing-steps step 1 will bail out. You can't focus the outer-dialog since it's not focusable.

[domenic]

Hmm, in fact by my reading, outerButton should remain the focusable area. Which would mean Chrome doesn't match the current spec. /cc @mfreed7

[josepharhar]

Why isn't outer-dialog focusable? And how does getting the focusable area for the outer-dialog return outer-button? It looks to me like it would return null, right? Is that why it is currently set to document.body...?

[domenic]

Outer dialog isn't focusable since it has does not have tabindex set, so it's similar to a div. Thus indeed getting the focusable area returns null, which causes the focusing steps to return early, without changing the focus. Thus focus stays on the button. (I think this is right!)

[sefeng211]

Got it! The inner dialog can be focused because tabIndex=0, makes sense!
Thanks!

[josepharhar]

It looks like https://html.spec.whatwg.org/#focusing-steps is not used for focusing when dialog.showModal() is called, this is used instead: https://html.spec.whatwg.org/multipage/interactive-elements.html#dialog-focusing-steps

[domenic]

Step 3 of the latter calls the former.

[josepharhar]

Ah I didn't actually read through it, my bad. I noticed that the function which implements the focusing steps was never called in chrome in this case, so I skipped over it.

It looks like #dialog-focusing-steps in chrome has an additional step which was added in 2013 to set the focus to body appears to have been omitted from the spec.

Perhaps we should add this step to the spec? Here is the reasoning in the commit message which added it:

When a modal is opened, then

1. The first autofocusable element in the dialog gets focus, or else
2. The first focusable element in the dialog gets focus, or else
3. The dialog itself gets focus, if it's focusable, or else
4. Focus is cleared (effectively, document.body gets focus)

I chose to do (4) since the the previous implementation
relied on CheckFocusedElementTask to clear focus, which
isn't great because "dialog.showModal(); document.activeElement"
was a race condition. One problem with (4) is it will
remove focus from a non-inert ancestor, but the spec also recently
changed to make ancestors inert so that's the direction to take.
We can't make that change immediately though since:
a) it would make document.body inert, which violates
assumptions of document.body getting default focus when
nothing else can have focus, and
b) it would prune the modal out of the accessibility
tree

[sefeng211]

What @josepharhar said makes sense to me. I think we should specify the behaviour when the dialog itself is not focusable.

The reasoning in the commit message is also valid, not sure how should we address that.

I wonder, would it make sense to make the dialog element to be focusable by default?

[domenic]

Oh, I think I found the missing spec step here. https://html.spec.whatwg.org/multipage/interaction.html#focus-fixup-rule applies: because outerButton becomes inert, it stops being focusable, so the "focus fixup rule" makes the Document's viewport the new focused area of the document. This translates to document.activeElement being the body.

So I think Chromium's behavior, and the tests in question, match the spec. It's just unclear because of the action at a distance.

I will send a HTML pull request that makes this clearer by adding some notes and examples near the dialog focusing steps.

[josepharhar]

Thanks @domenic!

[sefeng211]

Thanks!

[sefeng211]

This also explains why this test fails https://github.com/web-platform-tests/wpt/blob/master/html/semantics/interactive-elements/the-dialog-element/dialog-showModal.html#L117 on Chrome, I'll update the test.