Closed
Bug 1069221
Opened 10 years ago
Closed 9 years ago
Dialogs that come up are not exclusively visible to the screen reader.
Categories
(Firefox OS Graveyard :: Gaia::FMRadio, defect)
Tracking
(b2g-v2.2 fixed, b2g-master fixed)
RESOLVED
FIXED
2.2 S8 (20mar)
People
(Reporter: MarcoZ, Assigned: bugzilla)
References
Details
(Keywords: access, Whiteboard: [b2ga11y p=1])
Attachments
(3 files)
46 bytes,
text/x-github-pull-request
|
timdream
:
review+
yzen
:
a11y-review+
|
Details | Review |
46 bytes,
text/x-github-pull-request
|
kgrandon
:
review+
yzen
:
a11y-review+
bajaj
:
approval-gaia-v2.2+
|
Details | Review |
83 bytes,
text/plain
|
kgrandon
:
review+
|
Details |
Any dialogs brought up by FMRadio are not exclusively visible, other items that should be hidden from the screen reader, are not.
Updated•9 years ago
|
Assignee: nobody → bugzilla
Comment 1•9 years ago
|
||
I've been having trouble with WebIDE not showing me any of the CSS rules while debugging my Flame device ever since an update occurred around 2 weeks ago, so I'm hoping I didn't miss anything, but I think I've resolved the visibility issues. Looks like Autolander nabbed the PR so give it a look and let me know! :)
Flags: needinfo?(yzenevich)
Comment 3•9 years ago
|
||
(In reply to Ross from comment #2) > I've been having trouble with WebIDE not showing me any of the CSS rules > while debugging my Flame device ever since an update occurred around 2 weeks > ago, so I'm hoping I didn't miss anything, but I think I've resolved the > visibility issues. Looks like Autolander nabbed the PR so give it a look > and let me know! :) Yeah, WebIDE acts out somtimes, see if updating to the latest firefox nightly helps.
Flags: needinfo?(yzenevich)
Comment 4•9 years ago
|
||
Hi Ross, I added some comments in the PR. Looks pretty good, see if it makes sense now (instead of updating test HTML in 2 places) to just loadHTML just like we did in you first bug and avoid any further work when updating the tests. Thanks!
Okay, updated the PR with a new version. I tried to leave some line notes on the PR in the way that you usually do but it looks like I clicked the wrong thing -- think I was supposed to click "Files Changed" at the bottom but instead clicked the commit link. Haha, sorry about that, hopefully you can browse over to the commit page and see those points-of-reference relatively easily though.
Flags: needinfo?(yzenevich)
Comment 6•9 years ago
|
||
Left some comments, generally all looks good! thanks! mark me for the final a11y-review once you cleaned it all up.
Flags: needinfo?(yzenevich)
Okay, think we're set. I ended up just refreshing the body in all suites where new tempNodes had been declared. I think that should keep things safe for future additions to tests, etc. (should there be any).
I noticed that when Autolander auto-creates the PR attachment, I'm not given any permissions to set the flags on the attachment, so I ended up just setting the a11y-review flag on the bug itself. Not sure who that goes to however, so using NI here as well.
Flags: needinfo?(yzenevich)
Updated•9 years ago
|
Flags: needinfo?(yzenevich)
Attachment #8564461 -
Flags: review?(timdream)
Attachment #8564461 -
Flags: a11y-review+
Updated•9 years ago
|
Flags: a11y-review?
Comment 9•9 years ago
|
||
Thanks, Ross! Looks really good, forwarding the review to Timothy. I will check tomorrow your bugzilla permissions so you could set the flags without problems.
Comment 10•9 years ago
|
||
Meanwhile, Ross, let me know if you would be interested in this bug 1134097? It's something I did not have time to finish up in Gaia but it should be fun!
Assignee | ||
Comment 11•9 years ago
|
||
Hey Yura, okay, sounds good! And yeah, I checked out Bug 1134097 and it seems fine to me -- feel free to assign it to me when you get a chance (I don't appear to have appropriate permissions to "Take" bugs either at the moment) and I'll get started on it!
Comment 12•9 years ago
|
||
Hi Tim, would you have time to take a look at this one? If not, no worries, I can flag someone else.
Flags: needinfo?(timdream)
Comment 13•9 years ago
|
||
(In reply to Ross from comment #8) > I noticed that when Autolander auto-creates the PR attachment, I'm not given > any permissions to set the flags on the attachment, so I ended up just > setting the a11y-review flag on the bug itself. Not sure who that goes to > however, so using NI here as well. Kevin, are you aware of this? Where should we file autolander bugs to?
Flags: needinfo?(kgrandon)
Comment 14•9 years ago
|
||
Comment on attachment 8564461 [details] [review] [gaia] FunkTron:Bug1069221 > mozilla-b2g:master Thanks for the fix!
Flags: needinfo?(timdream)
Attachment #8564461 -
Flags: review?(timdream) → review+
Comment 15•9 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #13) > Kevin, are you aware of this? Where should we file autolander bugs to? I just found out about this =/ Currently working on this in bug 1137368. Moving forward the bug assignee will be able to edit all attachments on the bug. I'll try to get an ETA on that bug.
Flags: needinfo?(kgrandon)
Comment 16•9 years ago
|
||
We will probably want to manually land this one for now. Ross - could you squash your commits into a single, and rename the commit to contain the bug number?
Flags: needinfo?(bugzilla)
Assignee | ||
Comment 17•9 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #16) > We will probably want to manually land this one for now. Ross - could you > squash your commits into a single, and rename the commit to contain the bug > number? Hey Kevin, I went ahead and squashed the commits and included the Bug# in the commit message. Think you should be set to land, but let me know if you need anything else!
Flags: needinfo?(bugzilla)
Assignee | ||
Comment 18•9 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #14) > Thanks for the fix! Sure thing! :)
Comment 19•9 years ago
|
||
Landed in master: https://github.com/mozilla-b2g/gaia/commit/620210e07a4634008a33fe62f817f508258bd2c3 Bug 1137368 should be resolved soon which will make this workflow better in the future. Thank you for the patch.
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 20•9 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #19) > Bug 1137368 should be resolved soon which will make this workflow better in > the future. Thank you for the patch. Okay cool, sounds good! And no probs!
Comment 21•9 years ago
|
||
Comment on attachment 8564461 [details] [review] [gaia] FunkTron:Bug1069221 > mozilla-b2g:master [Approval Request Comment] This PR resolves dialog visibility issues in FM radio. [Bug caused by] (feature/regressing bug #): improvement, not a bug [User impact] if declined: if declined, screen reader users will be able to access content invisible otherwise [Testing completed]: unit tests, on device [Risk to taking this patch] (and alternatives if risky): low [String changes made]: none
Attachment #8564461 -
Flags: approval-gaia-v2.2?
Comment 22•9 years ago
|
||
Reverting this for causing regressing bug 1141330. Yura or Ross - could either of you guys take a look at this? Thanks! https://github.com/mozilla-b2g/gaia/commit/353558f34cc867e0f23b5cbe499c5f01a319d1ca
Status: RESOLVED → REOPENED
Flags: needinfo?(yzenevich)
Flags: needinfo?(bugzilla)
Resolution: FIXED → ---
Updated•9 years ago
|
Attachment #8564461 -
Flags: approval-gaia-v2.2?
Comment 23•9 years ago
|
||
Ross would you mind taking a look at this one if you have time?
Flags: needinfo?(yzenevich)
Comment 24•9 years ago
|
||
Assignee | ||
Comment 25•9 years ago
|
||
Hey Yura, So it looks like the problem was related to the #container element not being able to render properly as a result of its 'hidden' attribute. I solved the issue by going with a pre-existing CSS class that sets the 'visibility' style instead. Works on v3.0 now.
Flags: needinfo?(bugzilla) → needinfo?(yzenevich)
Comment 26•9 years ago
|
||
Comment on attachment 8575492 [details] [review] [gaia] FunkTron:Bug1069221-Fix > mozilla-b2g:master Looks good and works as expected from a11y point of view when in airplane mode. Ross, mark Kevin for a review when you update the PR with the comment. Thanks!
Flags: needinfo?(yzenevich)
Attachment #8575492 -
Flags: a11y-review+
Attachment #8575492 -
Flags: review?(kgrandon)
Comment 27•9 years ago
|
||
Comment on attachment 8575492 [details] [review] [gaia] FunkTron:Bug1069221-Fix > mozilla-b2g:master I don't have a headset with me now, so I can't easily test the FM radio app. I'm basically going to just carry the Tim's review from earlier, and trust that you have tested this and it won't regress again. Thank you for the patch!
Attachment #8575492 -
Flags: review?(kgrandon) → review+
Comment 28•9 years ago
|
||
Fixed the commit name to include the bug number and landed in master: https://github.com/mozilla-b2g/gaia/commit/2bcb2253c3a98a6f5b4c4f6360984fd61bcb9d45
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Comment 29•9 years ago
|
||
Attachment #8575630 -
Flags: review+
Comment 30•9 years ago
|
||
Comment on attachment 8575492 [details] [review] [gaia] FunkTron:Bug1069221-Fix > mozilla-b2g:master Hopefully it sticks in master , so asking for approval once more. [Approval Request Comment] This PR resolves dialog visibility issues in FM radio. [Bug caused by] (feature/regressing bug #): improvement, not a bug [User impact] if declined: if declined, screen reader users will be able to access content invisible otherwise [Testing completed]: unit tests, on device [Risk to taking this patch] (and alternatives if risky): low [String changes made]: none
Attachment #8575492 -
Flags: approval-gaia-v2.2?
Updated•9 years ago
|
Attachment #8575492 -
Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Comment 31•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/de472129d4b113f2fa6357ec5b746715f53092cf
You need to log in
before you can comment on or make changes to this bug.
Description
•