Closed Bug 885404 Opened 11 years ago Closed 11 years ago

Fix and enhance homescreen accessibility

Categories

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

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: eeejay, Assigned: eeejay)

References

Details

Attachments

(1 file, 5 obsolete files)

I made a few changes about a year ago to have the homescreen be more screen reader friendly. Since then, some of the changes have bitrotted[1], and some became less relevant as we develop the screen reader[2]. Other regressions, such as activating app icons via screen reader have been introduced. 1. https://github.com/mozilla-b2g/gaia/commit/2d4ce6b3fb7f75d140cefeded599022a186db596 2. https://github.com/mozilla-b2g/gaia/commit/2277ce7b57f28c268a226fba1c3a90ef0a8a91f3
This puts back in place a change that was first introduced in 2d4ce6b3fb7f75d140cefeded599022a186db596.
Attachment #765438 - Flags: review?(21)
There is a much more direct way of doing this now, thanks to IndieUI. The previous range widget and keyboard listeners were a horrible idea :P
Attachment #765439 - Flags: review?(21)
For a control to be activated by a screen reader, the element needs to listen for mouse clicks directly. This patch does a small refactor to allow that. Only synthesized clicks (that the screen reader sends) are accounted for, other kinds of click events are ignored.
Attachment #765440 - Flags: review?(21)
I have only one comment that IMHO is very important, avoid adding a click listener for each icon on the home screen, maybe it could be at the level of the parent for example. thanks
(In reply to Cristian Rodriguez de la Cruz (:crdlc) from comment #4) > I have only one comment that IMHO is very important, avoid adding a click > listener for each icon on the home screen, maybe it could be at the level of > the parent for example. Could you explain why that is important?
3 pages -> 3 listeners -> 3 objects 3 pages x 16 icons -> 48 listeners -> 48 objects more allocations, IMHO better event delegation
Comment on attachment 765438 [details] [diff] [review] Reintroduce hiding non-current pages in homescreen. Christian will be the right guy to r? this. I think a particular attention should be paid to the comment that want to force pages to be considered painted followed by the |visibility: hidden| code and ensure there is no regressions here.
Attachment #765438 - Flags: review?(21) → review?(crdlc)
Comment on attachment 765439 [details] [diff] [review] Rework the a11y page controls in the homescreen. Christian will be the right guy to r? this.
Attachment #765439 - Flags: review?(21) → review?(crdlc)
Comment on attachment 765440 [details] [diff] [review] Support synthesized click events on homescreen icons. Christian will be the right guy to r? this. On a side note I feel like an explicit declaration of the input source (MOZ_SOURCE_UNKNOWN) will help for the comment.
Attachment #765440 - Flags: review?(21) → review?(crdlc)
(In reply to Cristian Rodriguez de la Cruz (:crdlc) from comment #6) > 3 pages -> 3 listeners -> 3 objects > 3 pages x 16 icons -> 48 listeners -> 48 objects > > more allocations, IMHO better event delegation sounds good. i'll change it so that we use the parent. (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #9) > Comment on attachment 765440 [details] [diff] [review] > Support synthesized click events on homescreen icons. > > Christian will be the right guy to r? this. > > On a side note I feel like an explicit declaration of the input source > (MOZ_SOURCE_UNKNOWN) will help for the comment. Will do.
Attachment #765440 - Attachment is obsolete: true
Attachment #765440 - Flags: review?(crdlc)
Attachment #768009 - Flags: review?(crdlc)
Please, I need a bit help from you to review it. Attach a pull request because for me is so important to test regressions as much as review the code then it easier for me at least if I would have a pr. Thanks a lot and sorry
Status: NEW → ASSIGNED
I would like to have a new unit test for click listener, thanks a lot
Assignee: nobody → eitan
Comment on attachment 770550 [details] [diff] [review] Bug 885404 - Add click listener test to unit tests. Remove page navbar aria value asserts. I am also working to add tests to gaia-ui-tests, since things like visibility are not easy to assert for in unit tests.
Attachment #770550 - Flags: review?(crdlc)
Thanks a lot Eitan, I added some comments on Github
Blocks: gaiaa11y
OK! I just pushed a squashed version to the branch. I removed all of the scrollrequest additions for now, since I have had conversations with folks on the DOM team, and it looks like the "wheel" event will be more appropriate. So I will be coming up with a patch for that, and a separate bug.
Actually, I am working out another possible solution for activating items. So please hold on this altogeher.
Hi Eitan, could you squash your pr and add it here as an attachment. You know that it is r+ ;) Thanks
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → All
We started with 3 changes, now we are down to one - the hiding/unhiding of pages. The other two issues were: 1. scrollrequest support. This will probably be replaced with wheel events after consulting DOM team folks. 2. click listeners, officially they are still the "right" way to have items activated, but mobile web developers are moving to touch, and we can't fix everything. So we will need to simply handle this case in the screen reader.
Attachment #765438 - Attachment is obsolete: true
Attachment #765439 - Attachment is obsolete: true
Attachment #768009 - Attachment is obsolete: true
Attachment #770550 - Attachment is obsolete: true
Attachment #765438 - Flags: review?(crdlc)
Attachment #765439 - Flags: review?(crdlc)
Attachment #768009 - Flags: review?(crdlc)
Attachment #770550 - Flags: review?(crdlc)
Attachment #776568 - Flags: review?(crdlc)
Per comment 21, are you thinking about not reviewing and landing it before resolving those issues?
(In reply to Cristian Rodriguez de la Cruz (:crdlc) from comment #22) > Per comment 21, are you thinking about not reviewing and landing it before > resolving those issues? No, they can be reviewed and landed now, since they are needed in any case.
Attachment #776568 - Flags: review?(crdlc) → review+
Thanks! I'll open a bug soon for the page-changing issue soon.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: