Closed
Bug 1115829
Opened 10 years ago
Closed 10 years ago
Exploring status bar icons by touch again broken for the screen reader
Categories
(Firefox OS Graveyard :: Gaia::System, defect)
Tracking
(blocking-b2g:2.2+, tracking-b2g:backlog, b2g-v2.1 unaffected, b2g-v2.2 verified, b2g-master verified)
VERIFIED
FIXED
2.2 S4 (23jan)
Tracking | Status | |
---|---|---|
b2g-v2.1 | --- | unaffected |
b2g-v2.2 | --- | verified |
b2g-master | --- | verified |
People
(Reporter: MarcoZ, Assigned: apastor)
References
Details
(Keywords: access, regression, Whiteboard: [b2ga11y p=1][systemsfe])
Attachments
(1 file, 1 obsolete file)
46 bytes,
text/x-github-pull-request
|
gmarty
:
review+
etienne
:
feedback+
bajaj
:
approval-gaia-v2.2+
|
Details | Review |
[Tracking Requested - why for this release]:
Exploring the status bar icons with the screen reader is once again broken in 2.2 nightly builds. They can only be swiped to by navigational swipes left and right. To reproduce:
1. Configure your device.
2. Turn on screen reader by rocking the volume up, then down 3 times, then 3 times more after you hear the announcement.
3. On the home screen, touch the area where the status bar icons are.
Expected: You should hear an icon, and it should be highlighted.
Actual: Nothing is found.
5. Touch the address bar.
6. Perform a swipe to the left a few times and listen.
Result: Now, the status bar icons are spoken.
So they can be swiped to, but not touched.
Comment 1•10 years ago
|
||
Yes, it could be reproduced in 2.2[1].
And I could not reproduce this in 2.1[2], so it is a regression issue.
[1]: The environment:
Build ID 20141229160214
Gaia Revision 322ef5116a5827a30c9a3cd9b842449a9c66a5b3
Gaia Date 2014-12-29 18:03:02
Gecko Revision https://hg.mozilla.org/mozilla-central/rev/2c0ef7315830
Gecko Version 37.0a1
Device Name flame
Firmware(Release) 4.4.2
Firmware(Incremental) eng.cltbld.20141229.192338
Firmware Date Mon Dec 29 19:23:49 EST 2014
Bootloader L1TC00011880
[2]: The environment:
Build ID 20141229161203
Gaia Revision 73be51f998031f06db0cd660c0e388fa621c9f4c
Gaia Date 2014-12-26 16:54:56
Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/ea426e47bfc4
Gecko Version 34.0
Device Name flame
Firmware(Release) 4.4.2
Firmware(Incremental) eng.cltbld.20141229.192338
Firmware Date Mon Dec 29 19:23:49 EST 2014
Bootloader L1TC00011880
Keywords: regressionwindow-wanted
Updated•10 years ago
|
Comment 2•10 years ago
|
||
b2g-inbound regression window:
Last Working Environmental Variables:
Device: Flame
BuildID: 20141218042802
Gaia: 99be66baf4146835bed70e8105c2b8934af5cb7d
Gecko: 577aefbf76c3
Version: 37.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
First Broken Environmental Variables:
Device: Flame
BuildID: 20141218050403
Gaia: a1ec2268e7b467aa0ff07a2a3cbd1b9abc4e4dd1
Gecko: 32d360316ff3
Version: 37.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
First Broken Gaia & Last Working Gecko - issue DOES repro
Gaia: a1ec2268e7b467aa0ff07a2a3cbd1b9abc4e4dd1
Gecko: 577aefbf76c3
First Broken Gecko & Last Working Gaia - issue does NOT repro
Gaia: 99be66baf4146835bed70e8105c2b8934af5cb7d
Gecko: 32d360316ff3
https://github.com/mozilla-b2g/gaia/compare/99be66baf4146835bed70e8105c2b8934af5cb7d...a1ec2268e7b467aa0ff07a2a3cbd1b9abc4e4dd1
Caused by Bug 1074332.
Comment 3•10 years ago
|
||
Caused by bug 1074332 - Alberto can you take a look please?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado) → needinfo?(apastor)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → apastor
Flags: needinfo?(apastor)
Comment 4•10 years ago
|
||
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8544455 [details] [review]
[PullReq] albertopq:1115829-screen-reader to mozilla-b2g:master
I would like to hear Etienne's opinion here. We are using mozelements now for the statusbar, so the screenreader can't access to the icons. This patch uses the real statusbar when the screenreader is active, but I'm not sure if is the best solution. Any idea?
Attachment #8544455 -
Flags: feedback?(etienne)
Comment 6•10 years ago
|
||
Comment on attachment 8544455 [details] [review]
[PullReq] albertopq:1115829-screen-reader to mozilla-b2g:master
The change looks okay, but I want to make sure it's really needed before adding a screenreader exception.
The way Vivien made it work originally was to display the in-app status bar (moz-element) during transitions and edge gestures but to always make the real "canonical" status bar visible while the UI was at rest.
Looks like bug 1074332 changed this, but was it done deliberately to fix something else or were we just missing an integration test? :)
Attachment #8544455 -
Flags: feedback?(etienne)
Assignee | ||
Comment 7•10 years ago
|
||
It was intentionally done in order to have 1 statusbar per app (and avoid all the icons color issues that we had with only 1 statusbar). The other main reason was to, when opening the utility tray on an app with different icon colors, being able to transition between those 2 colors correctly.
May be we need to go back to only one statusbar, and only apply the shadows on the gestures and utility tray, but my impression is that we'll go back to icon colors exceptions again.
Comment 8•10 years ago
|
||
(In reply to Alberto Pastor [:albertopq] from comment #7)
> It was intentionally done in order to have 1 statusbar per app (and avoid
> all the icons color issues that we had with only 1 statusbar).
Not sure I follow this part (since the "before" also had 1 statusbar per app), do you mean 1 moz-element + filter or a completely separate dom?
Assignee | ||
Comment 9•10 years ago
|
||
Yes, the filter was applied to the statusbar, not to the mozelement. Now we always hide the real statusbar, and show the mozelement of that specific app, with the filter applied to it. Does that make sense?
Comment 10•10 years ago
|
||
(In reply to Alberto Pastor [:albertopq] from comment #9)
> Yes, the filter was applied to the statusbar, not to the mozelement. Now we
> always hide the real statusbar, and show the mozelement of that specific
> app, with the filter applied to it. Does that make sense?
Yep.
Kinda sad the original code wasn't commented/tested better :/
The accessibility regression shouldn't have been a surprise since this new approach removed code that was here solely for accessibility purpose.
Anyway, onward.
I don't know enough about the icon color exceptions we had in the past (how bad, numerous they were) to make a judgment here, but I trust yours!
Let's make sure we have a solid integration test for this screenreader-only code path.
Comment 11•10 years ago
|
||
(In reply to Alberto Pastor [:albertopq] from comment #5)
> Comment on attachment 8544455 [details] [review]
> [PullReq] albertopq:1115829-screen-reader to mozilla-b2g:master
>
> I would like to hear Etienne's opinion here. We are using mozelements now
> for the statusbar, so the screenreader can't access to the icons. This patch
> uses the real statusbar when the screenreader is active, but I'm not sure if
> is the best solution. Any idea?
I would strongly argue against using screenreader specific code path. This patch uses .screenreader as an implementation approach where the intention was to use it where screen reader truly requires alternative interactions (such as edge gestures). Ideally, we should have something similar to what used to be prior to this regression, otherwise we are setting ourselves up for another regression as soon as statusbar/utility tray/app titlebar stuff is revisited.
Assignee | ||
Comment 12•10 years ago
|
||
Thanks for the feedback guys. I'll work on going back to show the real statusbar always but when transitioning. I'll try to get the color filter working with the app window titlebar instead of the statusbar as well, so we get the best thing of both approaches.
Updated•10 years ago
|
Whiteboard: [b2ga11y p=1] → [b2ga11y p=1][systemsfe]
Assignee | ||
Comment 13•10 years ago
|
||
Attachment #8544455 -
Attachment is obsolete: true
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8545222 [details] [review]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/27190
Is basically a backout of the statusbar.css and app_titlebar.css of bug 1074332. I restored the tests again in order to check the real statusbar and not the shadows. Everything is passing but 1 error with the FTU (which seems to fail in master currently)
Attachment #8545222 -
Flags: review?(etienne)
Assignee | ||
Comment 15•10 years ago
|
||
I think the test that is failing is related to Bug 1118361
Comment 16•10 years ago
|
||
Comment on attachment 8545222 [details] [review]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/27190
The change looks good, please update the commit message to explicitly say that this is a "partial backout of the statusbar changes from bug XXXX".
Guillaume might have ideas about the permared test so pinging him and redirecting the final review while I'm at it :)
Attachment #8545222 -
Flags: review?(gmarty)
Attachment #8545222 -
Flags: review?(etienne)
Attachment #8545222 -
Flags: feedback+
Updated•10 years ago
|
blocking-b2g: 2.2? → 2.2+
Comment 17•10 years ago
|
||
The test in the FTU is failing because the minimised status bar is showing instead of the maximised.
Alberto, can you fix this?
Updated•10 years ago
|
Attachment #8545222 -
Flags: review?(gmarty)
Assignee | ||
Updated•10 years ago
|
Attachment #8545222 -
Flags: review?(gmarty)
Comment 18•10 years ago
|
||
Comment on attachment 8545222 [details] [review]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/27190
Looks good to me! You need to rebase though.
Attachment #8545222 -
Flags: review?(gmarty) → review+
Assignee | ||
Comment 19•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 21•10 years ago
|
||
This missed the v2.2 branching. Please request Gaia v2.2 approval for this when you get a chance.
Assignee | ||
Comment 22•10 years ago
|
||
Comment on attachment 8545222 [details] [review]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/27190
[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 1074332
[User impact] if declined: Screenreader doesn't work properly on the statusbar
[Testing completed]: Restored all the ui tests, for checking all the statusbar visibility cases.
[Risk to taking this patch] (and alternatives if risky): This is a partial backout. This is the less risky option to make the screenreader work again.
[String changes made]: -
Flags: needinfo?(apastor)
Attachment #8545222 -
Flags: approval-gaia-v2.2?
Updated•10 years ago
|
Attachment #8545222 -
Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Comment 23•10 years ago
|
||
Comment 24•10 years ago
|
||
This issue is verified fixed on Flame Master.
Result: The status bar icons are selected properly when touched, and the screen reader can be heard.
Device: Flame Master (319mb, full flash)
BuildID: 20150121010204
Gaia: 5e98dc164b17fd6decb48a9eaddef0e55b82e249
Gecko: 540077a30866
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 ( Master)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Comment 25•10 years ago
|
||
Yeojin, can you please verify 2.2 as well?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage-]
Flags: needinfo?(ktucker) → needinfo?(ychung)
Comment 26•10 years ago
|
||
This issue is verified fixed on Flame 2.2.
Result: The status bar icons are selected properly when touched, and the screen reader can be heard.
Environmental Variables:
Device: Flame 2.2 (319mb, full flash)
BuildID: 20150121002607
Gaia: e4f9b5da3751798f9cc5d95f302c30722cc11fca
Gecko: 75a462a58d7a
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
QA Whiteboard: [QAnalyst-Triage-] → [QAnalyst-Triage?]
Flags: needinfo?(ychung) → needinfo?(ktucker)
Updated•10 years ago
|
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.
Description
•