Closed Bug 1045017 Opened 5 years ago Closed 5 years ago

Can no longer touch status bar icons with the screen reader

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S4 (12sep)
Tracking Status
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: MarcoZ, Assigned: yzen)

References

Details

(Keywords: access, regression)

Attachments

(2 files, 2 obsolete files)

This is a regression that was introduced during the two weeks of my vacation. The status bar icons can still be swiped to, but no longer touched, and a two-finger swipe down does not activate the notification tray any more.
Sorry, I am on a Flame on a current nightly. Without the screen reader, the status bar icons work as expected.
Blocks: 1042083
As mentioned in bug 1042083: there are now 2 issues: statusbar being offscreen and the use of moz-element as background for app-titlebar
I started something on top of bug 1042105. The idea is to show the real statusbar for the current active apps and replaced it instantly by the statusbar shadow (the -moz-element one) once the user starts an edge gesture. This will be the same behavior if the screen reader is turned on or off.

It should help with 2 things:
 - Accessibility - the real statusbar will be back on the screen
 - Performance - Once some pixels are repainted for one particular icon, we won't need to repaint the whole statusbar because of -moz-element. 

The current patch is really in a wip state. I'm going on PTO tonight for one or 2 weeks and will likely not have the time to finish it before. If someone wants to steal the work, and continue it, it will be really appreciated!

Remember that the wip is on top of bug 1042105, which will hopefully land soon.
Attached patch one-statusbar.for.all.patch (obsolete) — Splinter Review
It will actually be better if I attached the wip...
(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #4)
> Created attachment 8469990 [details] [diff] [review]
> one-statusbar.for.all.patch
> 
> It will actually be better if I attached the wip...

The only thing I think is missing is having a visibility property set correctly along with opacity.
(In reply to Yura Zenevich [:yzen] from comment #5)
> (In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails,
> needinfo? please) from comment #4)
> > Created attachment 8469990 [details] [diff] [review]
> > one-statusbar.for.all.patch
> > 
> > It will actually be better if I attached the wip...
> 
> The only thing I think is missing is having a visibility property set
> correctly along with opacity.

In app_titlebar.css that is
Vivien, any movement on this? There are a couple of statusbar bugs that will be fixed here, and I'd love to get this in for 2.1.
Flags: needinfo?(21)
Attached patch bug1045017.patch (obsolete) — Splinter Review
Here an updated version.
Attachment #8469990 - Attachment is obsolete: true
Flags: needinfo?(21)
Attached patch bug1045017.patchSplinter Review
Here the latest version.
Attachment #8482246 - Attachment is obsolete: true
(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #9)
> Created attachment 8482304 [details] [diff] [review]
> bug1045017.patch
> 
> Here the latest version.

Hi Vivien, thanks for the patch! I tried it and I only had to only slightly modify it to work: in addition to opacity styling for maximized/minimized bars, I also added an appropriate visibility: hidden/false (depending on the value of opacity). The reason for that the screen reader's explore by touch behavior will not be able to access elements that are underneath completely opaque ones (even though visible otherwise). 

Otherwise, this patch makes status bar work again both in homescreen/lockscreen and apps!

Let me know how you'd like to proceed. It would definitely be a big plus in 2.1!
Flags: needinfo?(21)
(In reply to Yura Zenevich [:yzen] from comment #10)
> (In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails,
> needinfo? please) from comment #9)
> > Created attachment 8482304 [details] [diff] [review]
> > bug1045017.patch
> > 
> > Here the latest version.
> 
> Hi Vivien, thanks for the patch! I tried it and I only had to only slightly
> modify it to work: in addition to opacity styling for maximized/minimized
> bars, I also added an appropriate visibility: hidden/false (depending on the
> value of opacity). The reason for that the screen reader's explore by touch
> behavior will not be able to access elements that are underneath completely
> opaque ones (even though visible otherwise). 
> 
> Otherwise, this patch makes status bar work again both in
> homescreen/lockscreen and apps!
> 
> Let me know how you'd like to proceed. It would definitely be a big plus in
> 2.1!

Yura, thank you for looking into this! Do you have enough information to carry the patch forward from here? If so, I will make sure we find a suitable reviewer. This is actually blocking a bunch of graphical related work, so it would be wonderful to have.
Flags: needinfo?(yzenevich)
Yes, I can take it from here.
Assignee: nobody → yzenevich
Flags: needinfo?(yzenevich)
Attached file Github pull request.
I added a11y specific fixes on top of what Vivien had + added tests for the changes.
Attachment #8486669 - Flags: review?(alive)
Comment on attachment 8486669 [details] [review]
Github pull request.

r=me iif appChrome change has test.
Attachment #8486669 - Flags: review?(alive) → review+
Comment on attachment 8486669 [details] [review]
Github pull request.

Asking for a quick re-review. I had to change this line: https://github.com/mozilla-b2g/gaia/pull/23873/files#diff-889fcaf0800608d5aaa275a452c7492fR23 
from
#statusbar.light
to
#screen:not(.rocketbar-focused) #statusbar.light
Because of the rocketbar not being visible (and regressions in the gaia ui funtional browser tests)
Attachment #8486669 - Flags: review+ → review?(alive)
Also, alive, I did add the tests you requested.
(In reply to Yura Zenevich [:yzen] from comment #16)
> Also, alive, I did add the tests you requested.

I don't see app_chrome_test.js changed in https://github.com/mozilla-b2g/gaia/pull/23873/files..
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #17)
> (In reply to Yura Zenevich [:yzen] from comment #16)
> > Also, alive, I did add the tests you requested.
> 
> I don't see app_chrome_test.js changed in
> https://github.com/mozilla-b2g/gaia/pull/23873/files..

Oh I found it. sorry!
Attachment #8486669 - Flags: review?(alive) → review+
https://github.com/mozilla-b2g/gaia/commit/7c5014a7b040c526022126c4db09a8f065aa4275
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(21)
Resolution: --- → FIXED
Comment on attachment 8486669 [details] [review]
Github pull request.

This patch brings back status bar accessibility that was broken during v2.1 sprint. Also it is a blocker for several other non a11y bugs listed in description.

[Bug caused by] (feature/regressing bug #): bug 1042083

[User impact] if declined: I can speak for the screen reader user. The status bar becomes inaccessible from what it used to be in 2.0. The screen reader user will not be able to easily get any information about battery, signal, wifi etc.

[Testing completed]: Added unit tests and manually tested with screen reader on device.

[Risk to taking this patch] (and alternatives if risky): It is fairly risky. Changes to appchrome propagate to other components that use it.

[String changes made]: None
Attachment #8486669 - Flags: approval-gaia-v2.1?
Duplicate of this bug: 1054220
Attachment #8486669 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Depends on: 1071942
Depends on: 1071983
Depends on: 1071159
Depends on: 1072001
No longer depends on: 1072001
Depends on: 1081319
This issue is verified fixed on Flame 2.1 and 2.2.

Result: The screen reader reads status bar icons when the user touches the icons.

Device: Flame 2.1 (319mb, KK, Shallow Flash)
BuildID: 20141113001200
Gaia: 569a299ca446f714cd98d5881cc058fd6f6e257b
Gecko: d188e92aa5a6
Version: 34.0 (2.1)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 (319mb, KK, Shallow Flash)
BuildID: 20141113040205
Gaia: be8b0151d2f9a4c41fc63952128e0b723cd1161d
Gecko: ab137ddd3746
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
======================================================
Leaving the overall status as RESOLVED as one dependency is still NEW (bug 1094506).
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
(In reply to Yeojin Chung [:YeojinC] from comment #23)
> ======================================================
> Leaving the overall status as RESOLVED as one dependency is still NEW (bug
> 1094506).

Correction: The dependency does not seem to be related to screen reader. Marking the bug as VERIFIED, and I will check back with the open dependency later.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.