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

Move the focus back to the previous focused element for dialog.close() #5678

Closed
sefeng211 opened this issue Jun 25, 2020 · 37 comments
Closed
Labels
a11y-tracker Group bringing to attention of a11y, or tracked by the a11y Group but not needing response. accessibility Affects accessibility addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: dialog The <dialog> element topic: focus

Comments

@sefeng211
Copy link
Contributor

From an accessibility pointer of view, dialog.close() should move the focus back to the previously focused element to provide
good accessibility.

@domenic
Copy link
Member

domenic commented Jun 25, 2020

What is "the previously focused element"?

@sefeng211
Copy link
Contributor Author

I mean the element that was focused before the dialog showed up.

@domenic
Copy link
Member

domenic commented Jun 25, 2020

What if that element has been made un-focusable (removed, made inert, etc.)? What about if it has moved to another location or reused for a different purpose entirely (e.g. via a virtual DOM framework)?

In general, this would be the first time that the specification focuses an element without explicit web developer request or user interaction, so it raises all sorts of unprecedented questions.

@annevk annevk added a11y-tracker Group bringing to attention of a11y, or tracked by the a11y Group but not needing response. accessibility Affects accessibility topic: dialog The <dialog> element topic: focus labels Jun 25, 2020
@annevk
Copy link
Member

annevk commented Jun 25, 2020

Using https://software.hixie.ch/utilities/js/live-dom-viewer/ I played with variants on <input oninput="alert(1); document.body.appendChild(this); w(document.activeElement);"> to figure out what might be reasonable solutions to those problems. It seems that in general browsers fallback to the body element (the document in the model, iirc).

And is it really that different from showing the dialog running focusing steps? (Note that I'm not sure this should be limited to close(). It seems like something that would happen for dismissal in general.)

@domenic domenic added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest labels Jun 25, 2020
@MarcoZehe
Copy link

Yes, it is true that this re-focusing of the element that triggered the dialog is true for any kind of closing the dialog. See also the WAI-ARIA design pattern for role "dialog". It could be argued that dialog is a complex enough compound widget, with modality, anyway, so this is the first of its kind in the HTML specification, and it would also be the first of its kind to request that focus be set back to the triggering element if still possible. The alternative would be the body element, which would require keyboard users to tab a cillion times to get back to the element that triggered the dialog, and completely lose context in the meantime. It would eliminate a very common accessibility mistake if browsers would do this automatically if the triggering element is still focusable.

@annevk
Copy link
Member

annevk commented Jun 26, 2020

As for implementer interest, Mozilla is interested.

@hober @cookiecrook there was some talk about WebKit eventually adopting <dialog> too, would you all like to see this changed too?

@mfreed7 what about Chrome?

@cookiecrook
Copy link

It's always been recommended that authors move focus back to the "trigger" element (the element whose action caused the dialog or menu to be displayed). This is is consistent with platform conventions, too. For example, on popup buttons like <select>, the focus is moved back to the button after a selection has been made (Space/Enter/Click), or when the menu has been dismissed (Escape).

However, perhaps it's too simple to spec the "trigger" as the "previously focused element"? I think most of the time, the previously focused element is the same as the trigger, but there may be some scenarios where it's not. For example, if something is clicked and doesn't received focus from the click, then the "previously focused element" would be whatever was focused before the click. I don't know how common it would be. I'm just trying to consider all scenarios.

@cookiecrook
Copy link

cookiecrook commented Jun 26, 2020

As for WebKit's opinion, there has been support expressed for the inert attribute but <dialog> is still under consideration.

@cookiecrook
Copy link

cookiecrook commented Jun 26, 2020

(Updated: Re-posting this musing as a standalone comment to make it clear it's not an official WebKit position.)

In line with that, perhaps this "focus the previously focused element" idea should not be tied to the <dialog> element at all. It seems like a focus stack could be associated with the inertness primitive. A lot of JavaScript frameworks implement focus container stacks (I've helped with a few) so there is definitely a common use case outside <dialog>.

@annevk
Copy link
Member

annevk commented Jun 29, 2020

#4937 (comment) is what I was referring to above with regards to interest from WebKit.

@cookiecrook
Copy link

IIRC, there were some outstanding issues with dialog positioning yet to be addressed by CSS WG. @smfr may recall if there are any remaining blockers.

@hober
Copy link
Contributor

hober commented Jun 30, 2020

You're probably thinking of w3c/csswg-drafts#4645, @cookiecrook.

@Yay295
Copy link
Contributor

Yay295 commented Jul 13, 2020

For what it's worth, this is how the jQuery UI Dialog works.

Upon closing a dialog, focus is automatically returned to the element that had focus when the dialog was opened.

- https://api.jqueryui.com/dialog/

The jQuery UI Dialog also keeps focus within itself, so you can't tab out of it to something in the background (and if it's a modal dialog you also can't click out of it).

@annevk
Copy link
Member

annevk commented Jul 14, 2020

@mfreed7 @domenic ping. This is a blocking issue for Firefox and it'd be great to make some progress.

@domenic
Copy link
Member

domenic commented Jul 14, 2020

I remain weakly opposed to this for the reasons stated above, but @mfreed7 would be the ultimate implementer. I think it would be ideal if Firefox produced a proposed processing model and tests to answer about how the various edge cases I mentioned are handled.

@alice
Copy link
Contributor

alice commented Jul 15, 2020

It seems like a focus stack could be associated with the inertness primitive. A lot of JavaScript frameworks implement focus container stacks (I've helped with a few) so there is definitely a common use case outside .

I generally agree, but I would tie it to the "modal top layer" idea rather than inertness.

There was some discussion of this in this thread from 2016: #897 (later in that thread)

That also points to a since-deleted "focus fixup rule 3" - does anyone know what happened to that?

@annevk
Copy link
Member

annevk commented Jul 15, 2020

See a4bb346 (and also 90a60b2). A bunch of focus logic around the dialog element was removed in 2018 due to Chrome not implementing it and nobody else voicing a strong opinion.

@mfreed7
Copy link
Contributor

mfreed7 commented Aug 7, 2020

I am supportive of the general fix here - to return the focus to a useful spot, rather than <body>, when the dialog closes. Having said that, I don’t know how to define it, and the questions raised by @domenic need to be answered. Assuming reasonable answers there, I think we would support this.

@annevk
Copy link
Member

annevk commented Aug 17, 2020

@cookiecrook @alice does either of you expect to make progress on a new focus concept around inert/top layer? Otherwise, it seems like a simple and effective way forward here would be to record the previously focused element on the dialog element itself and restore focus there (if still focusable) once the dialog is closed. A broader version of focus scoping can maybe build on top of that in the future?

@cookiecrook
Copy link

cookiecrook commented Aug 17, 2020

@cookiecrook @alice does either of you expect to make progress on a new focus concept around inert/top layer?

I don't have any short terms plans to work on that.

Otherwise, it seems like a simple and effective way forward here would be to record the previously focused element on the dialog element itself and restore focus there (if still focusable) once the dialog is closed. A broader version of focus scoping can maybe build on top of that in the future?

I don't see a problem with this approach being the default behavior for dialog. Seems reasonably forward-compatible to me, even if it's reopened for improvement later via #897 or otherwise.

I'm speculating that there may be a timing issue though… If the "trigger" is inert (or otherwise disabled/inactive) while dialog is displayed, how long should the UA give the author to re-enable it (timeout for attempted re-focus?) or should the autofocus behavior be limited to UAs only?

@melanierichards
Copy link

Assuming reasonable answers there, I think we would support this.

Just wanted to +1 on general implementer interest from another Chromium contributor (MS Edge). Providing focus rewind as a default—with simple author control if a different experience is required—seems likes a win for both authors and users assuming we can work around the identified issues.

@alice
Copy link
Contributor

alice commented Aug 18, 2020

#4633 began a discussion around top layer APIs, I'm not sure whether it was continued elsewhere.

I would love to eventually figure out the interactions between inert, top layer, dialog, blocking elements, and focus fixup/replaying.

In the meantime I agree with @annevk and @cookiecrook that we could specify something sensible for dialog and generalise it as we go.

I like the idea of restoring focus to the previously focused element, although it's worth noting that in browsers which don't always focus elements on click (i.e. browsers not based on chromium), the previously focused element may not be the element that triggered the dialog. Perhaps we should also specify that we should not scroll to the element that gets focused as a result of a dialog closing, to avoid a confusing experience in that case.

I'm not sure I understand the timing issue you refer to though, James. If we're talking about steps for hiding a dialog, won't the UA be doing the focusing? And if we're not talking about that, what are we talking about?

@sefeng211
Copy link
Contributor Author

Gecko's work will be tracked at https://bugzilla.mozilla.org/show_bug.cgi?id=1660271.

We'll start with
By default, dialog.close() returns the focus to the previously focused element if the element is still focusable and within the viewport, otherwise move the focus to <body>;

I think what James' concern was, how do the authors move the focus upon modal dialog closing because the element that they want to move could be in inert state? I think we can solve it by guaranteeing that the document is no longer blocked by modal dialog before the close event is fired?

So the order of steps is Unblock document -> move focus to previously focused element -> dispatch close event. Then using the close event can move the focus to a different element.

If I am missing something, please let me know!

@cookiecrook
Copy link

cookiecrook commented Aug 22, 2020

Alice wrote:

I'm not sure I understand the timing issue you refer to though, James. If we're talking about steps for hiding a dialog, won't the UA be doing the focusing? And if we're not talking about that, what are we talking about?

That's what I was trying to clarify with the comment "or should the autofocus behavior be limited to UAs only?" I don't think there will be an issue if inertness and auto-re-focus behavior is limited to UAs only.

I was thinking of some hypothetical scenario where the author also modified the DOM at the time that the dialog was presented (a DOM change that might break the auto-re-focus behavior). Should the UA provide some non-blocking way to do the focusing after the author has had a chance to do any necessary DOM fix-up at the time of the dialog close?

It's probably a non-issue that can be addressed when/if a more concrete example arises. In the meantime, authors should probably achieve this by performing any necessary DOM fix-up prior to dialog.close()

@cookiecrook
Copy link

@sefeng211

So the order of steps is Unblock document -> move focus to previously focused element -> dispatch close event. Then using the close event can move the focus to a different element.

That order may be enough... but the duplicate focus events in quick succession would potentially be noticed by AT users.

The hypothetical scenario I was thinking of might be more like this:

  1. Unblock document -> [dispatch onbeforeclose event allowing author to fix-up and potentially focus] -> move focus to previously focused element [unless the author moved focus already] -> dispatch close event, or…
  2. Unblock document -> dispatch close event allowing author to fix-up and potentially focus -> move focus to previously focused element [unless the author moved focus already as part of the close event handler]

@sefeng211
Copy link
Contributor Author

@cookiecrook

That order may be enough... but the duplicate focus events in quick succession would potentially be noticed by AT users.

Yeah, fair point, we don't want to move the focus twice.

Unblock document -> [dispatch onbeforeclose event allowing author to fix-up and potentially focus] -> move focus to previously focused element [unless the author moved focus already] -> dispatch close event, or…

It introduces a new event which we hope that we can avoid.

Unblock document -> dispatch close event allowing author to fix-up and potentially focus -> move focus to previously focused element [unless the author moved focus already as part of the close event handler]

The issue is the close event is an async event, it's going to be awkward that we need to wait for an async event before continuing the algorithm.

Can we dispatch the close event as a sync event, then we can rely on the close event to determine whether we need to move the focus to a different element? It's not very clear why the close event is async at the moment. The document is already unblocked and plenty of JS may run before the close event is dispatched. And mousedown/up events could easily get through before close fires. However, making this change requires blink to change what it's already shipped.

@alice @mfreed7 What do you think the above changing for close event?

@cookiecrook
Copy link

cc @smfr

@annevk
Copy link
Member

annevk commented Sep 10, 2020

@alice @mfreed7 @cookiecrook @smfr ping. Would be nice to make progress here!

@rniwa
Copy link

rniwa commented Sep 11, 2020

It's very strange that close event is async. Since we're already mutating the content attribute which would cause mutation events to fire in all major browsers, there doesn't seem to be any security / integrity benefit either.

@sefeng211
Copy link
Contributor Author

@alice @mfreed7 @smfr Do you think changing the close event to a sync event to allow authors to change the focus is a reasonable approach here? I am happy to propose the spec changes.

@josepharhar
Copy link
Contributor

It doesn’t sound like anyone is opposing making the close event sync instead of async, so I suppose I could implement it in chrome along with the other focus change proposed here.

I don’t know why it is async and I don’t know if changing it will break any websites, but I could say the same thing about the proposed focus change and I won’t be sure until I try.

Are there any WPTs for this behavior yet?

@domenic
Copy link
Member

domenic commented Dec 7, 2020

The usual danger with sync events is outlined in https://w3ctag.github.io/design-principles/#guard-against-recursion . I'm not sure whether any of that reasoning applies here; I think it would require code like dialog.onclose = () => { dialog.open(); dialog.close(); } which is probably not something we really need to guard against.

@annevk
Copy link
Member

annevk commented Dec 7, 2020

(There is a small risk with regards to eventually removing support for mutation events (or changing when they fire, at least), but if we manage that we could certainly move this to CEReactions timing then too, I'd think.)

sefeng211 added a commit to sefeng211/html that referenced this issue Mar 25, 2021
Add a new `previously focused` element to dialog as a pointer
to the current focused element when `dialog.showModal()` and
`dialog.show()` are called, such that `dialog.close()` can
use it to restore the focus.

See whatwg#5678 for more details.
sefeng211 added a commit to sefeng211/html that referenced this issue Apr 6, 2021
Add a new `previously focused` element to dialog as a pointer
to the current focused element when `dialog.showModal()` and
`dialog.show()` are called, such that `dialog.close()` can
use it to restore the focus.

See whatwg#5678 for more details.
@annevk annevk closed this as completed in 5d50bfa Apr 27, 2021
@josepharhar
Copy link
Contributor

While implementing this in chrome, it was pointed out that changing focus synchronously, which fires an event, will make us have to write the code somewhat carefully (making sure that changing focus is the very last thing we do).

Was it ever considered to change focus asynchronously, or would that break anything? Am I missing anything on this topic?

@annevk
Copy link
Member

annevk commented May 5, 2021

Is it materially different from calling focus()? Given that screen reader experience is defined by what has focus, keeping the focus on a closed dialog might not yield the best results, I'd think.

@AutoSponge
Copy link

The intent to manage focus of a closing dialog is a good one. However, if
authors have a clear understanding of the process they should have the
ability to change the focus target on dialog.close().

A common example: a dialog created for the "confirm delete" action with
"confirm" and "cancel" buttons. Cancel can target the default element per
this spec (i.e., the "delete" button). Confirm would send focus to
body since the deleted item no longer exists. An author should have the
ability to override this behavior but programmatically setting focus could
introduce a race condition with the default behavior.

Possible solutions:

  • the addition of a dialog property, with an idref value, that will
    override this behavior by targeting the specified element when present
    provided the element exists and is connected to the dom.
  • an options object or flag passed to dialog.close() that overrides the
    default behavior.

@annevk
Copy link
Member

annevk commented May 12, 2021

@AutoSponge thank you! Could you file that as a new issue please? Comments in closed issues tend to get lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y-tracker Group bringing to attention of a11y, or tracked by the a11y Group but not needing response. accessibility Affects accessibility addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: dialog The <dialog> element topic: focus
Development

No branches or pull requests