1209062 - [Accessibility] Ensure system events are well labelled and announced.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

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

This includes:

* Ensuring app opening is clear
* Caller id number is announced on call
* Hardware events are announced (USB, SDCard etc)

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] yzen:bug-1209062-statusbar > mozilla-b2g:master

Comment 2

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

Comment on attachment 8668415 [details] [review]
[gaia] yzen:bug-1209062-statusbar > mozilla-b2g:master

Statusbar icons PR.

Comment 3

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

Statusbar icons - https://github.com/mozilla-b2g/gaia/commit/0f43446e7544cf89b8d8881a8f90aa206e74461b

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] yzen:bug-1209062-callscreen > mozilla-b2g:master

Comment 5

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

Comment on attachment 8669022 [details] [review]
[gaia] yzen:bug-1209062-callscreen > mozilla-b2g:master

Tim, could you review the System app part. The title that is hidden for attention window is still accessible via the screen reader, which is a bug.

Tamara, could you review the callscreen bits. Thanks.

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] yzen:bug-1209062-appchrome > mozilla-b2g:master

Comment 7

[Tamara Hills [:thills]]

Comment on attachment 8669022 [details] [review]
[gaia] yzen:bug-1209062-callscreen > mozilla-b2g:master

Hi Yura,

It looks good.  One doubt I had was around the .css change in /system.  I left a comment there.  Can double check with system peer what else this might impact aside from callscreen?

Thanks,

-tamara

Comment 8

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

https://github.com/mozilla-b2g/gaia/commit/76c23972a3b4b04b7a2a54efc76a371976474a07

Comment 9

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

https://github.com/mozilla-b2g/gaia/commit/4ac904057dc4506ff49dc667518fd72698701a5c

Comment 10

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

I believe we need it for RedTai/RedSquare

Comment 11

[GitHub Autolander Bot]

Attachment: [gaia] yzen:bug-1209062-statusbar-v2.2r > mozilla-b2g:v2.2r

Comment 12

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

Comment on attachment 8673299 [details] [review]
[gaia] yzen:bug-1209062-statusbar-v2.2r > mozilla-b2g:v2.2r

v2.2r PR

Comment 13

[GitHub Autolander Bot]

Attachment: [gaia] yzen:bug-1209062-callscreen-v2.2r > mozilla-b2g:v2.2r

Comment 14

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

Comment on attachment 8673306 [details] [review]
[gaia] yzen:bug-1209062-callscreen-v2.2r > mozilla-b2g:v2.2r

v2.2r PR

Comment 15

[GitHub Autolander Bot]

Attachment: [gaia] yzen:bug-1209062-appchrome-v2.2r > mozilla-b2g:v2.2r

Comment 16

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

Comment on attachment 8673313 [details] [review]
[gaia] yzen:bug-1209062-appchrome-v2.2r > mozilla-b2g:v2.2r

v2.2r PR

Comment 17

[Carsten Book [:Tomcat]]

for v2.2r:

https://github.com/mozilla-b2g/gaia/commit/cca3a23868755374e0eb60550c8eee2af89a867b
https://github.com/mozilla-b2g/gaia/commit/ec3fc32888d0bf8fb94b9ed7e2d5cc788da86eea
https://github.com/mozilla-b2g/gaia/commit/c1ec9277a56ecf10bb9b625873c84bdd9d1b138d

Comment 18

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

Will close this when the dependency is resolved.

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Seeing failures like https://treeherder.mozilla.org/logviewer.html#?job_id=10017&repo=mozilla-b2g37_v2_2r and https://treeherder.mozilla.org/logviewer.html#?job_id=10021&repo=mozilla-b2g37_v2_2r on 2.2r. Any ideas?

Comment 20

[GitHub Autolander Bot]

Attachment: [gaia] yzen:bug-1209062-unit-tests > mozilla-b2g:v2.2r

Comment 21

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

Comment on attachment 8674010 [details] [review]
[gaia] yzen:bug-1209062-unit-tests > mozilla-b2g:v2.2r

This should fix the bugs I believe.

Comment 22

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

(In reply to Yura Zenevich [:yzen] from comment #21)
> Comment on attachment 8674010 [details] [review]
> [gaia] yzen:bug-1209062-unit-tests > mozilla-b2g:v2.2r
> 
> This should fix the bugs I believe.

Sorry the tests.

Comment 23

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/75087e2f879632a89f778b3ec972a23867024675

Comment 24

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

Has failing test, reverted: https://github.com/mozilla-b2g/gaia/commit/c39024c919fbde295ea779ad9c5641a2bea5444a

Comment 25

[GitHub Autolander Bot]

Attachment: [gaia] yzen:bug-1209062-follow-up-unit-tests > mozilla-b2g:v2.2r

Comment 26

[Carsten Book [:Tomcat]]

landed as https://github.com/mozilla-b2g/gaia/commit/98e302bd63bf1a903bf7ccff5a6d887f7622f9cf

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/f8ecaec7fa12 is still failing

Comment 28

[GitHub Autolander Bot]

Attachment: [gaia] yzen:bug-1209062-follow-up-unit-tests > mozilla-b2g:v2.2r

Comment 29

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

Ok added a follow up PR for 2.2r, id wait until treeherder runs, but it looks like it's jammed atm.

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Looks green to me (Treeherder has trouble rendering individual revisions with the full revision SHA until some patches land and get pushed to production): https://treeherder.mozilla.org/#/jobs?repo=gaia&fromchange=71eda00bbf794579fd6cb05c75b481127afc1ba2&tochange=71eda00bbf794579fd6cb05c75b481127afc1ba2

Comment 31

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

And the test is passing on v2.2r!

Comment 32

[Carsten Book [:Tomcat]]

wes landed this as https://github.com/mozilla-b2g/gaia/commit/4440d266df1939fe2228f6dca498408d3a453a60

Comment 33

[GitHub Autolander Bot]

Attachment: [gaia] yzen:bug-1209062-announce-app > mozilla-b2g:master

Comment 34

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

Comment on attachment 8682271 [details] [review]
[gaia] yzen:bug-1209062-announce-app > mozilla-b2g:master

This should be the last patch for this bug. App name will be announced when it is opened, including homescreen.

Comment 35

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8682271 [details] [review]
[gaia] yzen:bug-1209062-announce-app > mozilla-b2g:master

I think this is fine if you have enough tests to cover the sequences you would like to support, but without a master promise queue I wonder if we could have speeches overlap with one another, or a then() callback unset a properly at the wrong time for other speeches?

It looks like you have just convert it into promises so if you think the current code is in risk of the problem describe above, adding an promise queue to protect that should be quite simple.

Please also noted a promise without a |.catch((e) => console.error(e))| at the very end implies JS error will be consumed without ever being printed out. It also means .then() followed will be skipped too.

Feel free to ask me to review again if you make additional changes and need extra pair of eyes to look at.

Comment 36

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

https://github.com/mozilla-b2g/gaia/commit/cf0703f147e17bea1b18b58ada82625151238220

Thanks Tim, I added catch clauses and double checked that there's no speech interrupt.

Comment 37

[GitHub Autolander Bot]

Attachment: [gaia] yzen:bug-1209062-announce-app-v2.2r > mozilla-b2g:v2.2r

Comment 38

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

In case threeherder is green this should be the last one for this bug for v2.2r

Comment 39

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/b12374ef1457cab0ae9b330171a373c7160c031a for the 2.2r checkin

Comment 40

[David Flanagan [:djf]]

This patch has broken the Music app on master, at least. Music will no longer play in the background. See bug 1222143.

Please back this out of master if you can't fix it quickly. I don't know about 2.2r. I didn't test there.

Is the screen reader using the "content" channel? If so, it means that it will pause other apps that are using that channel.  I'm not sure why music doesn't automatically resume.  Maybe that is a music app bug.

Is there a higher-priority channel that would allow the screen reader to just talk over the music rather than pause it?

Comment 41

[David Flanagan [:djf]]

I don't understand why the screen reader is on by default. I haven't turned it on, and I don't see any setting for it anywhere in the settings app. Why it is touching the audio hardware in any way if it is off?

Comment 42

[David Flanagan [:djf]]

Just to be clear, it is the last commit, the announce app names part, that broke the Music app.

Comment 43

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

(In reply to David Flanagan [:djf] from comment #40)
> This patch has broken the Music app on master, at least. Music will no
> longer play in the background. See bug 1222143.
> 
> Please back this out of master if you can't fix it quickly. I don't know
> about 2.2r. I didn't test there.
> 
> Is the screen reader using the "content" channel? If so, it means that it
> will pause other apps that are using that channel.  I'm not sure why music
> doesn't automatically resume.  Maybe that is a music app bug.
> 
> Is there a higher-priority channel that would allow the screen reader to
> just talk over the music rather than pause it?

Ideally yes there should be but there isn't at the moment.


(In reply to David Flanagan [:djf] from comment #41)
> I don't understand why the screen reader is on by default. I haven't turned
> it on, and I don't see any setting for it anywhere in the settings app. Why
> it is touching the audio hardware in any way if it is off?

The underlying bug is audio bug, this bug only exposes it even more. However you are right and screen reader should not do anything if it is off, which is what bug 1222249 is fixing. Once bug 1222249 is landed, bug 1222143 will be fixed. You can try the PR as it seems to work for me.