596732 - Add WAI-ARIA magic to Tab Candy to make it accessible

Status: RESOLVED WONTFIX

(Firefox Graveyard :: Panorama, defect, P2)

Description

[Marco Zehe (:MarcoZ)]

Tab Candy uses HTML and CSS custom widgets to do its magic. After proper keyboard navigation has been implemented for all features, see bug 587010, WAI-ARIA magic needs to be added so these HTML custom widgets can be made accessible to assistive technologies. WAI-ARIA merely adds semantics, no visible changes will be noticed.

One thing to add are proper WAI-ARIA roles, using the @role attribute to specify what a certain widget represents. Roles that might be appropriate:

tablist: for each individual grouping. aria-label with a proper string, or aria-labelledby with an ID as the value that holds the actual container name mut be used to give each tab list names.
tab: For each tab contained in a container or the main list of tabs from where they are being moved into a container. The name comes from either aria-label, aria-labelledby or the inner HTML text of the individual tab.

aria-activedescendant: Used on the tablist, pointing to the ID of the individual tab that has the keyboard focus or has been selected by the mouse.
aria-selected: a value of true must be specified if the current tab is the selected one. Changes with the selection of the keyboard focus.


For the search of tabs, something similar to what Yahoo! are doing in their search pages should be done: to Indicate via something that contains aria-live="polite" that a search has been successful, and a navigable list of search results that uses list and listitem respectively, and aria-activedescendant and aria-selected as described above.

All of these can be added via JavaScript. Some of them like the roles can be added statically, too. More attributes or rols may be needed as we progress through making the Panorama UI accessible. Without being able to use it, it's hard to say what else might be needed.

Comment 1

[Michael Yoshitaka Erlewine [:mitcho]]

Marco, thanks for your pointers. I'm getting started with adding ARIA roles now.

A couple questions/discussion points for everyone:
1. For groups that have yet to be named, I suppose it's inappropriate/unhelpful to have no aria-label, or a blank aria-label. Does this mean we'll need a localized string for this purpose for something like "untitled group number %d"?
2. In Panorama we have group-level highlighting (a selected group) and tab-level highlighting (a selected tab). Should aria-selected be added to groups (tablists) or to the individual tab, or both?
3. Because Panorama can have orphaned tabs not in any of the groups, we need a @role for the 
4. Dietrich/Dolske (our reviewers): what kind of tests would you expect for this? I plan on writing a mochitest to make sure some of the attributes, like the aria-selected, aria-activedescendant, etc., get updated correctly as focus moves, but otherwise we'll need something like litmus, yes?
5. Is there a particular screen reader or other such software I should be using with Firefox to test out these interactions myself? What accessibility tools are commonly used with Firefox?

Comment 2

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Work in progress 1

Added tablist and tab roles, with appropriate aria-labelledby pointers.
Added aria-owns to create "hierarchical" group-child relationship.
Updates aria-activedescendant and aria-selected on groups, using a new helper method: GroupItem_updateAriaProperties().

Open issues:
- search interaction
- undo close group interaction
- untitled group labels (see comment above)
- adding top-level Items to #content with aria-owns
- no handling of orphan tabs yet

Comment 3

[Aza Raskin [:aza]]

While this is an important feature to have in the long run, the accessibility team said that given that Panorama is written entirely in HTML this is not a must-have for first launch. Not sure how it snuck in as a beta 8 bug, but we should be focusing on higher priority blockers for Firefox 4. If we have time after getting normal functionality and visual polish done, we'll try to sneak this in for 4.

Comment 4

[Michael Yoshitaka Erlewine [:mitcho]]

Thanks for clarifying, Aza.

Comment 5

[Dão Gottwald [:dao]]

(In reply to comment #3)
> While this is an important feature to have in the long run, the accessibility
> team said that given that Panorama is written entirely in HTML this is not a
> must-have for first launch.

While this bug may not be a must-have, I don't understand what the UI being HTML has to do with that. Can someone please elaborate?

Comment 6

[Aza Raskin [:aza]]

> While this bug may not be a must-have, I don't understand what the UI being
> HTML has to do with that. Can someone please elaborate?

Because if it was canvas-based then screen readers would have no introspection ability into Panorama. That it is HTML means that screen readers will be able to enumerate the entities, even if they don't know the semantic roles.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #1)
> Marco, thanks for your pointers. I'm getting started with adding ARIA roles
> now.
> 
> A couple questions/discussion points for everyone:
> 1. For groups that have yet to be named, I suppose it's inappropriate/unhelpful
> to have no aria-label, or a blank aria-label. Does this mean we'll need a
> localized string for this purpose for something like "untitled group number
> %d"?

Right.

> 2. In Panorama we have group-level highlighting (a selected group) and
> tab-level highlighting (a selected tab). Should aria-selected be added to
> groups (tablists) or to the individual tab, or both?

I think you should use aria-selected for both (one is for group, other one is for selected tab in the group). But I wonder if tablist/tabs are suitable for this (I'm not sure if selectable tablist exist in UI and can be handled by AT). Wouldn't it better to use nested lists? That might be similar to HTML select with optgroups.

> 3. Because Panorama can have orphaned tabs not in any of the groups, we need a
> @role for the 

It should be tab I think

> 5. Is there a particular screen reader or other such software I should be using
> with Firefox to test out these interactions myself?

That'll be great, though it might be not easy if you didn't do this before. You always can make try server build and ask Marco to give you his impression. But if you like then you should try NVDA (nvda-project.org). Marco can take care about other screen readers.

> What accessibility tools
> are commonly used with Firefox?

I prefer to use DOMi (though you need to apply bug 601101 to make it working with Firefox 4).

There are number of tools on each platform. Windows: accprobe, Linux: accerciser, OS X: accessibility inspector.

Comment 8

[Ian Gilman [:iangilman]]

Comment on attachment 480508 [details] [diff] [review]
Work in progress 1

Took a quick look at this. I don't know that much about ARIA, but seems like you're taking a sensible enough approach. A couple comments:

+    previousGroupItem.updateAriaProperties();
+    groupItem.updateAriaProperties();

In this routine, both previousGroupItem and groupItem can be null; check for that. 

-  useConsole: true, // as opposed to dump
+  useConsole: false, // as opposed to dump

Switch it back. 

I wonder, maybe we should just set it to use both dump and console so we don't have to keep flipping it back and forth? Would this be frowned upon?

Comment 9

[Dão Gottwald [:dao]]

(In reply to comment #6)
> > While this bug may not be a must-have, I don't understand what the UI being
> > HTML has to do with that. Can someone please elaborate?
> 
> Because if it was canvas-based then screen readers would have no introspection
> ability into Panorama. That it is HTML means that screen readers will be able
> to enumerate the entities, even if they don't know the semantic roles.

Right, it could be worse, but the use of HTML was known when this bug was filed, so that doesn't really change the picture...
The question really is how useful the plain strings are without the roles.

Comment 10

[Aza Raskin [:aza]]

Marco (and oddly many Mozilla peeps) were under the impression that HTML could support the kinds of advanced interactions that Panorama manages. Much of the rush for accessibility was because in a canvas world, it would be entirely opaque to screen readers.

Comment 11

[James Teh [:Jamie]]

(In reply to comment #10)
> Marco (and oddly many Mozilla peeps) were under the impression that HTML could
> support the kinds of advanced interactions that Panorama manages.
Only if the HTML is marked up correctly, including ARIA, etc. Otherwise, it'll be anywhere from practically impossible to somewhat confusing for most screen reader users. They might be able to see text, but that doesn't mean they will be able to understand what it means without semantic info.

> Much of the
> rush for accessibility was because in a canvas world, it would be entirely
> opaque to screen readers.
I think it's probably fairer to say that accessibility would be a far bigger job if this was done in canvas. Doing it in HTML doesn't make accessibility less important; it just means that part of the job has already been done by virtue of the fact that standard HTML is already accessible.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

So it sounds to me like this bug is in fact blocking making TabCandy accessible, and therefore should be blocking final.

Comment 13

[Michael Yoshitaka Erlewine [:mitcho]]

Comment on attachment 480508 [details] [diff] [review]
Work in progress 1

Requesting feedback again as this has become a fx4.0 item.

Comment 14

[Ian Gilman [:iangilman]]

Comment on attachment 480508 [details] [diff] [review]
Work in progress 1

I don't actually know anything about ARIA, but here goes.

>+    iQ(this.container)
>+    .attr("aria-owns", owns)
>+    .attr("aria-activedescendant", descendant)
>+    .attr("aria-selected", selected);

Nit: Those .attr lines should be indented one.

>   setActiveGroupItem: function GroupItems_setActiveGroupItem(groupItem) {
>-    if (this._activeGroupItem)
>-      iQ(this._activeGroupItem.container).removeClass('activeGroupItem');
>+    let previousGroupItem = this._activeGroupItem;
>+    if (previousGroupItem)
>+      iQ(previousGroupItem.container).removeClass('activeGroupItem');
> 
>     if (groupItem !== null) {
>       if (groupItem)
>         iQ(groupItem.container).addClass('activeGroupItem');
>       // if a groupItem is active, we surely are not in an orphaned tab.
>       this.setActiveOrphanTab(null);
>     }
> 
>     this._activeGroupItem = groupItem;
>+    
>+    previousGroupItem.updateAriaProperties();
>+    groupItem.updateAriaProperties();

Both groupItem and previousGroupItem can validly be null here; check before calling updateAriaProperties on them.

>-  useConsole: true, // as opposed to dump
>+  useConsole: false, // as opposed to dump

Don't forget to change this back before the final patch.

>-    <input id="searchbox" type="text"/>
>+    <input id="searchbox" type="text" aria-hidden="true" />

Do we need to update aria-hidden when we show the search box?

f+ with the above addressed (particularly the null check).

Comment 15

[Ian Gilman [:iangilman]]

Mitcho, should this be assigned to you?

Comment 16

[Michael Yoshitaka Erlewine [:mitcho]]

(In reply to comment #15)
> Mitcho, should this be assigned to you?

Probably but, as I don't foresee getting back to this in the very near future, I think it'd be safer to keep it open with a helpwanted. Let's check in again perhaps in a couple weeks if noone else has taken it up.

Comment 17

[Kevin Hanes]

Moving up to b9, as it's a blocker.

Mitcho, it's been a few weeks, any thoughts?

Comment 18

[Michael Yoshitaka Erlewine [:mitcho]]

(In reply to comment #17)
> Moving up to b9, as it's a blocker.
> 
> Mitcho, it's been a few weeks, any thoughts?

I personally am still not able to get to this. :( Thanks for checking in, though.

Comment 19

[Ian Gilman [:iangilman]]

Mitcho, in case you're not the one to get back to this, can you list what still needs to be done on it? Is it just addressing my comments, or is there more?

Comment 20

[Johnathan Nightingale [:johnath]]

Notes from the Grand Retriage: minus it provisionally, but mark it d? so beltzner can follow up with dbolter.

Comment 21

[Kevin Hanes]

bugspam (moving b9 to b10)

Comment 22

[Kevin Hanes]

bugspam (removing b9)

Comment 23

[Kevin Hanes]

Punt

Comment 24

[David Bolter [:davidb] (NeedInfo me for attention)]

Yes. I think we can communicate that since this is not critical to accessible Firefox usage we can tackle it post-FF4.

Comment 25

[Marco Zehe (:MarcoZ)]

Do we have a keyword to remind us that we should make a note in the VPAT for Firefox 4 about this as a known issue?

Comment 26

[Justin Dolske [:Dolske]]

Panorama has been removed from Firefox 45, currently in Beta and scheduled for release on March 7th. As such, I'm closing all existing Panorama bugs.

If you are still using Panorama, you will see a deprecation message in Firefox 44, and when 45 is released your tab group data will be migrated to bookmarks, with a folder for each group. There are also a few addons offering similar functionality.

See https://support.mozilla.org/en-US/kb/tab-groups-removal for more info.

We're removing Panorama because it has extremely low usage (about 0.01% of users), and has a large number of bugs and usability issues. The cost of fixing all those issues is far too high to justify, and so we'll instead be focusing our time and energy on improving other parts of Firefox.