Closed
Bug 1135805
Opened 9 years ago
Closed 9 years ago
[Accessibility] App chrome urlbar title is visible to the screen reader user even though it is hidden off screen.
Categories
(Firefox OS Graveyard :: Gaia::System, defect)
Tracking
(b2g-v2.2 fixed, b2g-master fixed)
RESOLVED
FIXED
2.2 S7 (6mar)
People
(Reporter: yzen, Assigned: yzen)
References
Details
(Keywords: access, Whiteboard: [b2ga11y p=1][b2ga11y stability])
Attachments
(1 file, 2 obsolete files)
46 bytes,
text/x-github-pull-request
|
kgrandon
:
review+
bajaj
:
approval-gaia-v2.2+
|
Details | Review |
Screen reader user should not be able to swipe to the titlebar in full screen apps for example. Steps to reproduce with screen reader: * Open a Gallery app * Swipe all the way right until the screen reader reaches the end. * Expected: screen reader should stop on the last visible item on the screen. * Observed: screen reader steps into the invisible "Search the web" item.
Comment 1•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8568581 -
Flags: review?(alive)
Assignee | ||
Updated•9 years ago
|
Whiteboard: [b2ga11y p=1] → [b2ga11y p=1][b2ga11y stabilitiy]
Assignee | ||
Updated•9 years ago
|
Whiteboard: [b2ga11y p=1][b2ga11y stabilitiy] → [b2ga11y p=1][b2ga11y stability]
Comment 2•9 years ago
|
||
Comment on attachment 8568581 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master Looks good to me, but I am delivering this review to sysFE team since they are taking care app chrome now.
Attachment #8568581 -
Flags: review?(alive) → review?(mhenretty)
Comment 3•9 years ago
|
||
Comment on attachment 8568581 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master Change looks good, things seem more consistent now that we add/remove the visible class both when showing and hiding. I tested the patch, and couldn't find anything broken with the date in the utility. Glad to see tests for this too, so we don't break a11y so easily.
Attachment #8568581 -
Flags: review?(mhenretty) → review+
Comment 4•9 years ago
|
||
Comment on attachment 8568581 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master Whoops, that was for the wrong bug. I'm reviewing this now.
Attachment #8568581 -
Flags: review+ → review?(mhenretty)
Comment 5•9 years ago
|
||
Comment on attachment 8568581 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master Code looks reasonable, and it seems to work well too. However, I don't feel entirely comfortable reviewing app_chrome just yet. Forwarding to Kevin for super review.
Attachment #8568581 -
Flags: review?(mhenretty)
Attachment #8568581 -
Flags: review?(kgrandon)
Attachment #8568581 -
Flags: feedback+
Comment 6•9 years ago
|
||
Comment on attachment 8568581 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master Looks good to me. Thanks!
Attachment #8568581 -
Flags: review?(kgrandon) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Updated•9 years ago
|
Keywords: checkin-needed
Comment 7•9 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/e4390a6fb59b58da21a21a9702c17884528f26ca
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8568581 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master [Approval Request Comment] This pull request fixes screen reader visibility issues around chrome bar. [Bug caused by] (feature/regressing bug #): improvement, not a bug [User impact] if declined: if declined, screen reader users will be able to land on content hidden otherwise (in chrome bar) [Testing completed]: unit tests + on device [Risk to taking this patch] (and alternatives if risky): low [String changes made]: none
Attachment #8568581 -
Flags: approval-gaia-v2.2?
Comment 9•9 years ago
|
||
Comment on attachment 8568581 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master Clearing for now for causing bug 1137074?
Attachment #8568581 -
Flags: approval-gaia-v2.2?
Comment 10•9 years ago
|
||
Backed out for causing bug 1137074: https://github.com/mozilla-b2g/gaia/commit/9c0dcfb27be96e09453a447dd2714e7b1d991e7a Let's fix and re-land so we don't have to uplift multiple things.
Comment 11•9 years ago
|
||
Yura - please create a new patch for this bug + the lockscreen fix and flag me for review. Thanks!
Flags: needinfo?(yzenevich)
Updated•9 years ago
|
Attachment #8568581 -
Attachment is obsolete: true
Comment 12•9 years ago
|
||
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8569640 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master Merged the fix into the original PR.
Flags: needinfo?(yzenevich)
Attachment #8569640 -
Flags: review?(kgrandon)
Comment 14•9 years ago
|
||
Comment on attachment 8569640 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master This seems fine to me again. Thanks!
Attachment #8569640 -
Flags: review?(kgrandon) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Updated•9 years ago
|
Keywords: checkin-needed
Comment 15•9 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/2769db0e5fe75986ff9b16b39f63ec328eff62ff
Updated•9 years ago
|
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 16•9 years ago
|
||
Comment on attachment 8569640 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master [Approval Request Comment] This pull request fixes screen reader visibility issues around chrome bar. [Bug caused by] (feature/regressing bug #): improvement, not a bug [User impact] if declined: if declined, screen reader users will be able to land on content hidden otherwise (in chrome bar) [Testing completed]: unit tests + on device [Risk to taking this patch] (and alternatives if risky): low [String changes made]: none
Attachment #8569640 -
Flags: approval-gaia-v2.2?
Updated•9 years ago
|
Attachment #8569640 -
Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Comment 17•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/158ca75143288df269137ae751d2600d625e4160
status-b2g-v2.2:
--- → fixed
Target Milestone: --- → 2.2 S7 (6mar)
Comment 18•9 years ago
|
||
reverted for causing bug 1138688. master: https://github.com/mozilla-b2g/gaia/commit/67aa028e7e41f2216b0aafc163333ca77474efc8 v2.2: https://github.com/mozilla-b2g/gaia/commit/3d188c414e30acc392253d5389a42352fcfbc183
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•9 years ago
|
Target Milestone: 2.2 S7 (6mar) → ---
Comment 19•9 years ago
|
||
If we don't write tests here, let's make sure we file some follow-ups to write tests for *everything* that this has broken that we didn't catch in CI.
Depends on: 1137074
Updated•9 years ago
|
Attachment #8569640 -
Flags: approval-gaia-v2.2+
Assignee | ||
Updated•9 years ago
|
Attachment #8569640 -
Attachment is obsolete: true
Comment 20•9 years ago
|
||
Assignee | ||
Comment 21•9 years ago
|
||
Comment on attachment 8571981 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master Ensuring that the urlbar visibility is fully consistent with the rules described for its container (https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/chrome/chrome.css#L63-L70)
Attachment #8571981 -
Flags: review?(kgrandon)
Comment 22•9 years ago
|
||
Comment on attachment 8571981 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master The selectors are starting to get a little too crazy/duplicated for me. I'd like to see if we can explore a more clean way of doing things. Yura - I left a comment on github. Would it be possible to only have an override for the fullscreen-app case? Also why is it bad that the screenreader hits the rocketbar when it's offscreen?
Flags: needinfo?(yzenevich)
Attachment #8571981 -
Flags: review?(kgrandon)
Assignee | ||
Comment 23•9 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #22) > Comment on attachment 8571981 [details] [review] > [gaia] yzen:bug-1135805 > mozilla-b2g:master > > The selectors are starting to get a little too crazy/duplicated for me. I'd > like to see if we can explore a more clean way of doing things. > > Yura - I left a comment on github. Would it be possible to only have an > override for the fullscreen-app case? > > Also why is it bad that the screenreader hits the rocketbar when it's > offscreen? The general rule is that screen reader must only have access to elements that are currently present on screen: to avoid unexpected interactions, etc. Also screen readers are not necessarily used by completely blind people meaning that it will be highly confusing to reach things that they can't see.
Flags: needinfo?(yzenevich)
Assignee | ||
Comment 24•9 years ago
|
||
Comment on attachment 8571981 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master Styling just the fullscreen-app chrome did that trick, thanks for the suggestion!
Attachment #8571981 -
Flags: review?(kgrandon)
Comment 25•9 years ago
|
||
Comment on attachment 8571981 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master I think this looks fine to me, thanks! It appears that there was some kind of rebase error here. It looks like the author is currently labeled as "Autolander" instead of yourself. It's probably ok to land as-is, but you might want to take a minute to fix it.
Attachment #8571981 -
Flags: review?(kgrandon) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Updated•9 years ago
|
Keywords: checkin-needed
Comment 26•9 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/8fd2a21c98fe230e76a92f4f8bada70424b1e4f1
Updated•9 years ago
|
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 27•9 years ago
|
||
Comment on attachment 8571981 [details] [review] [gaia] yzen:bug-1135805 > mozilla-b2g:master Re-requesting once more, hopefully for good: [Approval Request Comment] This pull request fixes screen reader visibility issues around chrome bar. [Bug caused by] (feature/regressing bug #): improvement, not a bug [User impact] if declined: if declined, screen reader users will be able to land on content hidden otherwise (in chrome bar) [Testing completed]: unit tests + on device [Risk to taking this patch] (and alternatives if risky): low [String changes made]: none
Attachment #8571981 -
Flags: approval-gaia-v2.2?
Updated•9 years ago
|
Attachment #8571981 -
Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Comment 28•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/35bd03b2a4ccb242edb3ce8c2b2989a0e8508ce7
Target Milestone: --- → 2.2 S7 (6mar)
You need to log in
before you can comment on or make changes to this bug.
Description
•