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.

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

Probably-wrong focus requirement #3675

Closed
domenic opened this issue May 8, 2018 · 5 comments · Fixed by #3798
Closed

Probably-wrong focus requirement #3675

domenic opened this issue May 8, 2018 · 5 comments · Fixed by #3798
Assignees

Comments

@domenic
Copy link
Member

domenic commented May 8, 2018

Discovered this while reviewing #3647. /cc @TakayoshiKochi

When a focusable area is added to an empty Document, it must be designated the focused area of the document.

I can't find where this might be applied. It's kind of ambiguous what "empty Document" means; an active Document that is not inert always has the viewport as a focusable area, so maybe it's not "empty"? It can't mean "has no child nodes", since there will almost always be a html element at least...

https://jsbin.com/loxilon/edit?html,console,output shows no real change here.

Probably we should just remove this? Unless someone can figure out what it was trying to do?

@domenic
Copy link
Member Author

domenic commented May 8, 2018

We should try harder to figure out what "empty" means I guess, since it shows up in several other parts of the focus spec. Maybe we could get some info on when activeElement ever returns null in implementations?

@domenic
Copy link
Member Author

domenic commented May 9, 2018

Both Gecko and Blink implement activeElement by basically asking for the focused element of the document, not the focused area.

I think basically there are three states for the focusable area of a document, per the post-#3647 spec:

  • null: the document is inert, inactive, or has no child nodes at all
  • the document's viewport
  • A specific element or focusable area within an element

So here "empty" would mean "has no child nodes at all".

This is hard to test because activeElement does not give direct access to the focusable area of the document. I wish I knew more ways to test the "focusable area of the document". Maybe it is untestable and we'd be better off with a "focused element" concept that maps to what implementations do.

I think I can work on a PR to clean this and related areas up after #3647 lands...

@annevk
Copy link
Member

annevk commented May 9, 2018

This ends up logging <iframe onload=this.contentDocument.onfocus=w></iframe> when I tab to it in Firefox, so presumably that's a way to test things?

@TakayoshiKochi
Copy link
Member

The original spec was talking about "empty control group", such as a <dialog> without any focusable element in it. But as the result that we removed <dialog>-specific behavior that the original spec intended, dumb replacement of "control group" to "document" made such conditions.
I'll take closer look to find out inconsistency.

@annevk I don't understand your comment. Could you elaborate?

@annevk
Copy link
Member

annevk commented May 11, 2018

@TakayoshiKochi if you paste that into https://software.hixie.ch/utilities/js/live-dom-viewer/ and tab around you'll see you can trigger the listener. That means that at least with manual tests the scenarios @domenic mentions can be tested to some extent.

domenic pushed a commit that referenced this issue May 14, 2018
The control group concept existed to give special focus behavior to
<dialog> elements, treating them on par with Documents in terms of
allowing focus to shift inside of them. However, this was never
implemented in user agents.

This change removes the control group concept, instead using Document
directly. This allows some simplifications, e.g. because the first
focusable area of a non-empty Document is always the Document's
viewport.

The change uncovered some areas of the spec that were potentially wrong
or unclear when considering focus around documents. We will follow up on
that in #3675. But for now we keep
the same algorithms, just without <dialog>s.

Fixes #2171.
@TakayoshiKochi TakayoshiKochi self-assigned this May 21, 2018
domenic pushed a commit that referenced this issue Jul 11, 2018
The PR #3647 removed "control group" concept, and most of them
were converted to Document. The "control group" could be empty
when e.g. no focusable element in a dialog element, but for a
document without any focusable element, its viewport is still
focusable.

Removes sentences talking about such a non-existent Document
condition.

Fixes #3675.
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
The control group concept existed to give special focus behavior to
<dialog> elements, treating them on par with Documents in terms of
allowing focus to shift inside of them. However, this was never
implemented in user agents.

This change removes the control group concept, instead using Document
directly. This allows some simplifications, e.g. because the first
focusable area of a non-empty Document is always the Document's
viewport.

The change uncovered some areas of the spec that were potentially wrong
or unclear when considering focus around documents. We will follow up on
that in whatwg#3675. But for now we keep
the same algorithms, just without <dialog>s.

Fixes whatwg#2171.
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
The PR whatwg#3647 removed "control group" concept, and most of them
were converted to Document. The "control group" could be empty
when e.g. no focusable element in a dialog element, but for a
document without any focusable element, its viewport is still
focusable.

Removes sentences talking about such a non-existent Document
condition.

Fixes whatwg#3675.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants