Closed Bug 1476854 Opened 2 years ago Closed Last year

[a11y] Expose whether a tab is (multi)selected for accessibility

Categories

(Firefox :: Tabbed Browser, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: Jamie, Assigned: jaws)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file)

When you select multiple tabs, the fact that a tab (other than the current one) is selected is only presented visually. It is not exposed semantically for accessibility.

This can be fixed by setting the aria-selected="true" attribute on selected tabs.
Also, aria-multiselectable="true" should probably be set on the tabs element (#tabbrowser-tabs) to indicate that multiple selection is supported.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Priority: P3 → P1
Thanks for working on this. The patch looks good. I started testing it, only to discover that aria-selected/aria-multiselectable don't get applied to XUL tab/tabs by the a11y code. I'm working on a patch to fix that. The front-end patch doesn't have the expected effect without the a11y fix.
Depends on: 1480058
Jamie, can you confirm that this patch works now?
Flags: needinfo?(jteh)
Comment on attachment 8996445 [details]
Bug 1476854 - Expose whether tabs are multiselected for accessibility.

https://reviewboard.mozilla.org/r/260532/#review269044

Thanks for this. Sorry about the delay; I wanted to get the required a11y core fixes into central before this got landed so we didn't have potentially dead code floating about. The patch looks good to me, and I tested this against latest central and all works as expected. Note that it needs some rebase intervention, but it's pretty trivial.
Attachment #8996445 - Flags: review?(jteh) → review+
Flags: needinfo?(jteh)
Comment on attachment 8996445 [details]
Bug 1476854 - Expose whether tabs are multiselected for accessibility.

https://reviewboard.mozilla.org/r/260532/#review269068

::: browser/base/content/tabbrowser.xml:182
(Diff revision 1)
>          </setter>
>        </property>
>  
> +      <property name="_multiselectEnabled">
> +        <setter>
> +          // Can't use toggleAttribute since the value is needed.

It took me some time to understand this sentence.

Why is aria-multiselectable different from standard HTML attributes?
(In reply to Dão Gottwald [::dao] from comment #5)
> Comment on attachment 8996445 [details]
> Bug 1476854 - Expose whether tabs are multiselected for accessibility.
> 
> https://reviewboard.mozilla.org/r/260532/#review269068
> 
> ::: browser/base/content/tabbrowser.xml:182
> (Diff revision 1)
> >          </setter>
> >        </property>
> >  
> > +      <property name="_multiselectEnabled">
> > +        <setter>
> > +          // Can't use toggleAttribute since the value is needed.
> 
> It took me some time to understand this sentence.
> 
> Why is aria-multiselectable different from standard HTML attributes?

http://www.maxability.co.in/2015/06/aria-multiselectable-property/ and https://www.w3.org/TR/wai-aria-1.1/#aria-multiselectable

I'll redirect to Jamie.
Flags: needinfo?(jteh)
(In reply to Dão Gottwald [::dao] from comment #5)
> It took me some time to understand this sentence.

It could perhaps be clarified by noting that ARIA requires literal "true"/"false" strings.

> Why is aria-multiselectable different from standard HTML attributes?

The use of "true"/"false" strings for boolean ARIA attributes is specified in the ARIA spec:
https://www.w3.org/WAI/PF/aria-1.1/appendices#typemapping
I'm not sure of the exact reasons, but I can take an educated guess:
1. ARIA isn't actually part of HTML. It works within HTML, but it's not part of HTML itself.
2. I believe ARIA was designed so it could theoretically be used in markup languages other than HTML.
At the end of the day, we need to do it this way because the spec says so. :)
Flags: needinfo?(jteh)
Comment on attachment 8996445 [details]
Bug 1476854 - Expose whether tabs are multiselected for accessibility.

https://reviewboard.mozilla.org/r/260532/#review269216

::: browser/base/content/tabbrowser.xml:187
(Diff revision 1)
> +          // Can't use toggleAttribute since the value is needed.
> +          if (val) {
> +            this.setAttribute("aria-multiselectable", "true");
> +          } else {
> +            this.removeAttribute("aria-multiselectable");
> +          }

Instead of the two cases, how about:

this.setAttribute("aria-multiselectable", !!val);

::: browser/base/content/tabbrowser.xml:191
(Diff revision 1)
> +            this.removeAttribute("aria-multiselectable");
> +          }
> +          return val;
> +        </setter>
> +        <getter>
> +          return this.hasAttribute("aria-multiselectable");

So this should really return this.getAttribute("aria-multiselectable") == "true";

::: browser/base/content/test/tabs/browser_multiselect_tabs_using_selectedTabs.js:15
(Diff revision 1)
>        [PREF_MULTISELECT_TABS, true]
>      ]
>    });
>  
>    function testSelectedTabs(tabs) {
> +    ok(gBrowser.tabContainer.hasAttribute("aria-multiselectable"),

should check the actual value

::: browser/base/content/test/tabs/browser_multiselect_tabs_using_selectedTabs.js:25
(Diff revision 1)
>      if (tabs.length == 1) {
>        ok(!selectedTab.multiselected, "Selected tab shouldn't be multi-selected because we are not in multi-select context yet");
>        ok(!_multiSelectedTabsSet.has(selectedTab), "Selected tab shouldn't be in _multiSelectedTabsSet");
>        is(selectedTabs.length, 1, "selectedTabs should contain a single tab");
>        is(selectedTabs[0], selectedTab, "selectedTabs should contain the selected tab");
> +      ok(!selectedTab.hasAttribute("aria-selected"),

here too, I guess?
Attachment #8996445 - Flags: review?(dao+bmo)
Comment on attachment 8996445 [details]
Bug 1476854 - Expose whether tabs are multiselected for accessibility.

https://reviewboard.mozilla.org/r/260532/#review269412

::: browser/base/content/tabbrowser.xml:182
(Diff revision 3)
>          </setter>
>        </property>
>  
> +      <property name="_multiselectEnabled">
> +        <setter>
> +          // Unlike HTML, the value of the attribute is used for ARIA.

This comment is possibly even more confusing, because some HTML attribute values are actually used for accessibility.

How about:

Unlike boolean HTML attributes, the value of boolean ARIA attributes actually matters.
Attachment #8996445 - Flags: review?(dao+bmo) → review+
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c22115a387fe
Expose whether tabs are multiselected for accessibility. r=dao,Jamie
https://hg.mozilla.org/mozilla-central/rev/c22115a387fe
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.