895438 - Make status bar icons accessible

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Marco Zehe (:MarcoZ)]

The indicators for battery, WiFi, mobile network etc. are not accessible. They can neither be found, nor do they talk,  when swiping or exploring.

Comment 1

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

Depends on changes to output and traversal in bug 1001628

Comment 2

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

Attachment: Github pull request.

Comment 3

[Eitan Isaacson [:eeejay]]

Comment on attachment 8412919 [details] [review]
Github pull request.

We discussed this in the previous bug.

Comment 4

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

Updated the pull request to work with newer traversal rules in the scree reader.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

My question: do we use data-l10n-id in screen-reader? what if it doesn't need translate? can screen reader use alternative attribute?

Comment 6

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8412919 [details] [review]
Github pull request.

I need to know how screen reader works with data-l10n-id.
If you put data-l10n-id="statusbar" in statusbar div, there might be unwanted "Statusbar" shown in statusbar. How do we resolve this?

Comment 7

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

(In reply to Alive Kuo [:alive][NEEDINFO!][OOO 4/30 - 5/4] from comment #6)
> Comment on attachment 8412919 [details] [review]
> Github pull request.
> 
> I need to know how screen reader works with data-l10n-id.
> If you put data-l10n-id="statusbar" in statusbar div, there might be
> unwanted "Statusbar" shown in statusbar. How do we resolve this?

So in the .properties file we use statusbar.ariaLabel which in turn populate the aria-label attribute and not the inner text. It will then only be visible to the screen reader.

Comment 8

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Yura Zenevich [:yzen] from comment #7)
> (In reply to Alive Kuo [:alive][NEEDINFO!][OOO 4/30 - 5/4] from comment #6)
> > Comment on attachment 8412919 [details] [review]
> > Github pull request.
> > 
> > I need to know how screen reader works with data-l10n-id.
> > If you put data-l10n-id="statusbar" in statusbar div, there might be
> > unwanted "Statusbar" shown in statusbar. How do we resolve this?
> 
> So in the .properties file we use statusbar.ariaLabel which in turn populate
> the aria-label attribute and not the inner text. It will then only be
> visible to the screen reader.

Interesting...btw you have lots of "icon is undefined" failure, please fix it and ask review again.

Comment 9

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

Comment on attachment 8412919 [details] [review]
Github pull request.

Travis is happy now!

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8412919 [details] [review]
Github pull request.

r+ but let me get pike noticed

Comment 11

[Axel Hecht [:Pike]]

Comment on attachment 8412919 [details] [review]
Github pull request.

Please don't insert entries between comments and the entry that comment is for.

Also, I'm very concerned about the various string compositions, notably, that we're doing all kinds of Nouns plus " on", and then trailing stuff which might be empty.

I'm concerned that languages will need different grammar depending on the noun there, etc.

Just make those single entries instead, please.

Comment 12

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

Comment on attachment 8412919 [details] [review]
Github pull request.

Addressed the feedback comments; marking for a quick re-review, feedback to make sure all's good (Travis's green).

Comment 13

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8412919 [details] [review]
Github pull request.

Let pike do final check.

Comment 14

[Axel Hecht [:Pike]]

Comment on attachment 8412919 [details] [review]
Github pull request.

Thanks, this is much easier to read.

I think we should still have a comment for the section clarifying that all of these strings are for a11y only.

Two strings make my eyebrows rise:
"Media playing on" isn't really English, but maybe it's a11y jargon that makes more sense than English?
"Searching… Emergency calls only", the ellipsis in the middle of the sentence, does that make sense to screenreaders? It's certainly nothing that'd show up in English UI.

Denoting this with an f-, to get a comment in. I don't think I need to make another pass over the patch, though.

Comment 15

[Marco Zehe (:MarcoZ)]

(In reply to Axel Hecht [:Pike] from comment #14)
> "Media playing on" isn't really English, but maybe it's a11y jargon that
> makes more sense than English?

I agree that this can be worded better. It is not an indicator that a feature is enabled, but that media is currently playing. So the label should probably be changed to "Media playing" and the "on" removed.

> "Searching… Emergency calls only", the ellipsis in the middle of the
> sentence, does that make sense to screenreaders? It's certainly nothing
> that'd show up in English UI.

I think this is OK. The ellipsis indicates that this search is currently in progress, and that currently only emergency calls are possible. Since we don't have a spinner or something, this ellipsis is a common way of saying "this is on-going".

Looking over the other labels, they look OK to me.

Comment 16

[Marco Zehe (:MarcoZ)]

Comment on attachment 8412919 [details] [review]
Github pull request.

Pike, with comment 15, does this look OK now? Note that yzen is PTO this week, so trying to move this forward from this end.

Comment 17

[Axel Hecht [:Pike]]

Comment on attachment 8412919 [details] [review]
Github pull request.

Still need a comment.

This would be good in general for the a11y strings we add. Most localizers will have a tendency by now to shorten strings as much as possible, and the a11y strings shouldn't fall under that, so adding comments or making it clear through the IDs helps to change the mental context of localizers when they're hitting these.

The ones with .ariaLabel are easy, but the other entries are hard to discover that they're a11y, even in the file context. Not to mention the poor folks working in form-based tools like pootle.

Comment 18

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

Added the comment and changed the media string.

Comment 19

[Eitan Isaacson [:eeejay]]

I would merge, if travis weren't failing (also, stay away from bugzilla when you take time off :)

Comment 20

[Carsten Book [:Tomcat]]

(In reply to Eitan Isaacson [:eeejay] from comment #19)
> I would merge, if travis weren't failing (also, stay away from bugzilla when
> you take time off :)

https://travis-ci.org/mozilla-b2g/gaia/builds/25101801 is the travis link. Could you look into this and request checkin-needed again ?

Comment 21

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

\o/ https://travis-ci.org/mozilla-b2g/gaia/builds/25617201

Comment 22

[Eitan Isaacson [:eeejay]]

https://github.com/mozilla-b2g/gaia/commit/6178a58b21ace4085d0307ec6c063bc46fbf2c2b

Comment 23

[Francesco Lodolo [:flod]]

One doubt about a couple of strings

> statusbarIconOnActive-tethering=Tethering on, active
> statusbarIconOnActive-bluetooth=Bluetooth on, active

Does this mean that the tethering/bluetooth is on, and there's a transfer ongoing?

Comment 24

[Francesco Lodolo [:flod]]

Removing also keyword for checkin, since this already landed on Gaia.

Comment 25

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

(In reply to Francesco Lodolo [:flod] from comment #23)
> One doubt about a couple of strings
> 
> > statusbarIconOnActive-tethering=Tethering on, active

Means that tethering is enabled and there are connected clients.

> > statusbarIconOnActive-bluetooth=Bluetooth on, active

Means that the Bluetooth is enabled and there is a connection.

> 
> Does this mean that the tethering/bluetooth is on, and there's a transfer
> ongoing?

Basically yes, hope that helps.

Comment 26

[Peter Dolanjski [:pdol]]

Removing the target 2.1 off of this meta bug.  We're going to instead only set that flag for individual accessibility features instead of the overall meta.