Closed
Bug 995035
Opened 11 years ago
Closed 10 years ago
Invisible UI elements are visible to screen reader
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(blocking-b2g:2.1+, b2g-v2.1 fixed, b2g-v2.2 fixed)
People
(Reporter: eeejay, Assigned: eeejay)
References
Details
(Keywords: access, Whiteboard: [b2ga11y p=1] )
Attachments
(2 files)
46 bytes,
text/x-github-pull-request
|
azasypkin
:
review+
fabrice
:
approval-gaia-v2.1+
|
Details | Review |
4.92 MB,
video/webm
|
Details |
When swiping through the interface with the screen reader, I am able to delete, deselect, select all buttons that are invisible at first. They need to be properly hidden.
Assignee | ||
Updated•11 years ago
|
Whiteboard: [b2ga11y 2.0]
Assignee | ||
Updated•11 years ago
|
Whiteboard: [b2ga11y 2.0] → [b2ga11y p=2]
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #8436002 -
Flags: review?(felash)
Comment 3•11 years ago
|
||
Comment on attachment 8436002 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
Adding Arnau for the BB change.
Attachment #8436002 -
Flags: review?(arnau)
Comment 4•11 years ago
|
||
Comment on attachment 8436002 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
Sorry Eitan, this doesn't work for the large panels :(
I guess we'll need to use aria-hidden here...
Attachment #8436002 -
Flags: review?(felash) → review-
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #4)
> Comment on attachment 8436002 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
>
> Sorry Eitan, this doesn't work for the large panels :(
>
> I guess we'll need to use aria-hidden here...
How doesn't it work? Does it show/hide elements when it shouldn't? Performance?
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to Eitan Isaacson [:eeejay] from comment #5)
> (In reply to Julien Wajsberg [:julienw] from comment #4)
> > Comment on attachment 8436002 [details] [review]
> > Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
> >
> > Sorry Eitan, this doesn't work for the large panels :(
> >
> > I guess we'll need to use aria-hidden here...
>
> How doesn't it work? Does it show/hide elements when it shouldn't?
> Performance?
Ah, just saw the PR comment..
Comment on attachment 8436002 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
Cancelling review
Attachment #8436002 -
Flags: review?(arnau)
Assignee | ||
Comment 8•10 years ago
|
||
Re-aligning priorities with 2.1 accessibility goals.
Whiteboard: [b2ga11y p=2] → [b2ga11y p=1]
Comment 9•10 years ago
|
||
Hey Milan,
when testing this patch, we see that when the slide animation starts (say: we want to display a thread, so the "inbox" panel and the "thread" panel move left), the panel that was not displayed (in that case: the thread) stays white for some time, whereas it should be displayed really at the start of the animation.
My initial guess was that the "will-change: scroll-position;" CSS property is doing this, but I tried removing it and there was no effect for this issue.
Do you think there is something we can do in Gaia? That there is an issue in Gecko?
Flags: needinfo?(milan)
Comment 10•10 years ago
|
||
Comment 11•10 years ago
|
||
Is this still the original bug? Anyway, just looking at the video, it's probably a performance issue, but I'd really like to see what Flame does with this. If the thing behind is not around anymore, either we have to display white and show the motion right away, or delay the motion before we render what we need to show. I think showing white with the motion right away is a better way, but UX may want to chime in.
Flags: needinfo?(milan)
Comment 12•10 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #11)
> Is this still the original bug?
Sorry, I don't follow the question. If the question is "does the proposed patch fix the issue in comment 0?" then yes :)
> Anyway, just looking at the video, it's
> probably a performance issue, but I'd really like to see what Flame does
> with this. If the thing behind is not around anymore, either we have to
> display white and show the motion right away, or delay the motion before we
> render what we need to show. I think showing white with the motion right
> away is a better way, but UX may want to chime in.
I think that displaying white looks broken, but delaying the motion can be too long and will look jerky (because I think we'll skip the steps instead of just delaying, right?).
IMO the long-term fix will be haida (separate windows should be faster to repaint), the short-term fix is to use aria-hidden.
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Julien Wajsberg [:julienw] (PTO 08/20 -> 09/15; contact schung instead) from comment #12)
> IMO the long-term fix will be haida (separate windows should be faster to
> repaint), the short-term fix is to use aria-hidden.
Our support for aria-hidden is about to get an upgrade, thanks to bug 1054454. I am going to have another go at this.. sorry for the delay.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → eitan
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8436002 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
I updated to PR to use aria-hidden
Attachment #8436002 -
Flags: review- → review?(felash)
Comment 15•10 years ago
|
||
Comment on attachment 8436002 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
Redirecting review request to Oleg as I'm in holidays soon and won't be able to deal with follow-ups.
Oleg, please especially check that we see both panels during the sliding transition; I'm afraid that since the patch still uses "visibility" we'd still have the issue outlined in comment 9.
Attachment #8436002 -
Flags: review?(felash) → review?(azasypkin)
Assignee | ||
Comment 16•10 years ago
|
||
(In reply to Julien Wajsberg [:julienw] (PTO 08/19 -> 09/15; contact schung for SMS matters) from comment #15)
> Comment on attachment 8436002 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
>
> Redirecting review request to Oleg as I'm in holidays soon and won't be able
> to deal with follow-ups.
>
> Oleg, please especially check that we see both panels during the sliding
> transition; I'm afraid that since the patch still uses "visibility" we'd
> still have the issue outlined in comment 9.
We only use visibility for the edit header/buttons, not for the panel. So I don't think it should be an issue.
Comment 17•10 years ago
|
||
Sorry, still didn't have time to look at this patch, but it's on the top of my list for Monday.
Comment 18•10 years ago
|
||
Comment on attachment 8436002 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
Everything looks good to me and I don't see any issues while transitioning. I've just left one question on GitHub, once we sort out it, I think we're good to go.
Also, please rebase your PR on the latest master, seems like we have some merge conflicts.
Thanks!
Attachment #8436002 -
Flags: review?(azasypkin)
Assignee | ||
Comment 19•10 years ago
|
||
Comment on attachment 8436002 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
Alrite.. I took your suggestion in the PR. here is another version.
Attachment #8436002 -
Flags: review?(azasypkin)
Comment 20•10 years ago
|
||
Comment on attachment 8436002 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
Eitan, now it looks good to me, except that screen reader doesn't see Report and Participants panels. I've left comment on Github about that + few tiny stylistic nits.
Thanks!
Attachment #8436002 -
Flags: review?(azasypkin)
Assignee | ||
Comment 21•10 years ago
|
||
Comment on attachment 8436002 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
Oops, I forgot to reflag this for review. Just rebased again, and corrected the issues you mentioned in the PR.
Attachment #8436002 -
Flags: review?(azasypkin)
Comment 22•10 years ago
|
||
Comment on attachment 8436002 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
Looks good now, r=me, just few indentation nits on Github.
Thanks!
Attachment #8436002 -
Flags: review?(azasypkin) → review+
Assignee | ||
Comment 23•10 years ago
|
||
[Blocking Requested - why for this release]:
This accounts for a round of accessibility improvements for the SMS app that are required for the screen reader. The changes here are critical for basic functionality. The review process started a while ago, but slipped the branching deadline.
blocking-b2g: --- → 2.1?
Assignee | ||
Comment 24•10 years ago
|
||
Assignee | ||
Comment 26•10 years ago
|
||
Comment on attachment 8436002 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20160
[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: SMS app is unusable to screen reader users.
[Testing completed]: yes.
[Risk to taking this patch] (and alternatives if risky): Low risk for general, non-screen reader users since this is mostly done only with aria-hidden which has no impact on layout.
[String changes made]:
Attachment #8436002 -
Flags: approval-gaia-v2.1?
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Attachment #8436002 -
Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Comment 27•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•