1272011 - [a11y] Entering the breadcrumbs widget with the keyboard should focus the container only and aria-activedescendant should be set on the current active breadcrumb

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P2)

Description

[Patrick Brosset <:pbro>]

We're fixing/implementing keyboard navigation in the toolbox, and decided that keyboard navigation in the breadcrumbs should be handled as follows:

- when entering the widget (click on a breadcrumb, or tabbing through elements), the container should gain focus only, so that the next (sidebar) or previous (markup-view) elements are one TAB away
- once focused, navigation in the breadcrumbs is handled like in a toolbar, simple left/right arrow-key to navigate to each crumb linearly (depends on bug 1264907 for this)
- the widget should set the aria-activedescendant attribute on the right button

Comment 1

[Patrick Brosset <:pbro>]

Inspector bug triage (filter on CLIMBING SHOES).
:yzen, assigning P2 for now, please feel free to change this if you think it should more or less important.

Comment 2

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: Bug 1272011 - improving keyboard accessibility for the inspector breadcrumbs.

Review commit: https://reviewboard.mozilla.org/r/69094/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69094/

Comment 3

[Yura Zenevich [:yzen] (please use "needinfo")]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=31589a1f3948

Comment 4

[Brian Grinstead [:bgrins]]

https://reviewboard.mozilla.org/r/69094/#review66486

::: devtools/client/inspector/breadcrumbs.js:505
(Diff revision 1)
> -      event.preventDefault();
> +    event.preventDefault();
> -      control.focus();
> +
> +    this.outer.setAttribute("aria-activedescendant",
> +      this.nodeHierarchy[this.currentIndex].button.id);
> +
> +    if (event.target !== this.outer) {

How does this.outer get focused in the first place?  If event.target is this.outer then this event has been stopped / prevented and this condition isn't true.  Should `this.outer.focus()` run unconditionally?  It seems to work as-is, I just don't know why.

Comment 5

[Yura Zenevich [:yzen] (please use "needinfo")]

https://reviewboard.mozilla.org/r/69094/#review66494

::: devtools/client/inspector/breadcrumbs.js:505
(Diff revision 1)
> -      event.preventDefault();
> +    event.preventDefault();
> -      control.focus();
> +
> +    this.outer.setAttribute("aria-activedescendant",
> +      this.nodeHierarchy[this.currentIndex].button.id);
> +
> +    if (event.target !== this.outer) {

It has a tabindex="0" (see changes in inspector.xul). So if navigation is done via keyboard and we are tabbing from the markup view to breadcrumbs, then the #inspector-breadcrumbs will be focused. The only time that the target is not an outer is when we click on a particular breadcrump with mouse.

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Yura Zenevich [:yzen] from comment #5)
> https://reviewboard.mozilla.org/r/69094/#review66494
> 
> ::: devtools/client/inspector/breadcrumbs.js:505
> (Diff revision 1)
> > -      event.preventDefault();
> > +    event.preventDefault();
> > -      control.focus();
> > +
> > +    this.outer.setAttribute("aria-activedescendant",
> > +      this.nodeHierarchy[this.currentIndex].button.id);
> > +
> > +    if (event.target !== this.outer) {
> 
> It has a tabindex="0" (see changes in inspector.xul). So if navigation is
> done via keyboard and we are tabbing from the markup view to breadcrumbs,
> then the #inspector-breadcrumbs will be focused.

Right, but the first thing the focus handler does is `event.stopPropagation(); event.preventDefault();`, so I was thinking it would be prevented by JS.

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

I agree, I think I looked into it but was not sure how it's not prevented in capture. Ill take a look again..

Comment 8

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8777534 [details]
Bug 1272011 - improving keyboard accessibility for the inspector breadcrumbs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69094/diff/1-2/

Comment 9

[Yura Zenevich [:yzen] (please use "needinfo")]

it looks like this never got r+/- so I can't use review board to land the patch.

Comment 10

[Brian Grinstead [:bgrins]]

Comment on attachment 8777534 [details]
Bug 1272011 - improving keyboard accessibility for the inspector breadcrumbs.

https://reviewboard.mozilla.org/r/69094/#review66842

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa4472559aa9
improving keyboard accessibility for the inspector breadcrumbs. r=bgrins

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/aa4472559aa9

Comment 13

[Sylvestre Ledru [:Sylvestre]]

I guess we can let this ride the train