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

RESOLVED FIXED in Firefox 51

Status

DevTools
Inspector
P2
normal
RESOLVED FIXED
2 years ago
a month ago

People

(Reporter: pbro, Assigned: yzen)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 51
Dependency tree / graph

Firefox Tracking Flags

(firefox49 wontfix, firefox50 wontfix, firefox51 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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
Depends on: 1259812
(Reporter)

Comment 1

2 years ago
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.
Priority: -- → P2

Updated

2 years ago
Blocks: 1273125
(Assignee)

Updated

2 years ago
Assignee: nobody → yzenevich
Status: NEW → ASSIGNED
(Assignee)

Comment 2

2 years ago
Created attachment 8777534 [details]
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/
Attachment #8777534 - Flags: review?(bgrinstead)
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.
(Assignee)

Comment 5

2 years ago
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.
(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.
(Assignee)

Comment 7

2 years ago
I agree, I think I looked into it but was not sure how it's not prevented in capture. Ill take a look again..
(Assignee)

Comment 8

2 years ago
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/
(Assignee)

Comment 9

2 years ago
it looks like this never got r+/- so I can't use review board to land the patch.
Flags: needinfo?(bgrinstead)
Comment on attachment 8777534 [details]
Bug 1272011 - improving keyboard accessibility for the inspector breadcrumbs.

https://reviewboard.mozilla.org/r/69094/#review66842
Attachment #8777534 - Flags: review?(bgrinstead) → review+
Flags: needinfo?(bgrinstead)

Comment 11

2 years ago
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa4472559aa9
improving keyboard accessibility for the inspector breadcrumbs. r=bgrins

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/aa4472559aa9
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51

Updated

2 years ago
Depends on: 1295564
I guess we can let this ride the train
status-firefox49: affected → wontfix
status-firefox50: --- → wontfix

Updated

a month ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.