1069004 - Edit view: brightness slider is not accessible

Status: RESOLVED FIXED

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

Description

[Eitan Isaacson [:eeejay]]

The slider is a bunch of labels, it should be unified to a widget that is perceivable and operable by a screen reader user.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/27649

Comment 2

[Punam Dahiya [:pdahiya]]

Comment on attachment 8554063 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/27649

Hi Eitan
The aria-value attributes part of the patch looks good, however I am not confident reviewing keypress handler for slider. Please see my query in github and set the review flag again with your inputs. Thanks!

Comment 3

[Eitan Isaacson [:eeejay]]

Comment on attachment 8554063 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/27649

Addressed question in PR.

Comment 4

[Punam Dahiya [:pdahiya]]

Hi Eitan

Please confirm if you are able to test screen reader swipe up and down gesture on slider. It still doesn't works for me on latest m-c Flame-kk. A nit noted in github to add comment explaining  key up/down. Thanks!

Comment 5

[Eitan Isaacson [:eeejay]]

Yes! You can test this now both with m-c and gecko37.

Comment 6

[Punam Dahiya [:pdahiya]]

Tried on m-c with today's build 20150217074221, flame-kk , accessibility experience has definitely improved but I still fail to use swipe up and down gesture on slider. Attaching screenshot of exposure slider screen with a yellow highlight shown on touching slider. However navigating up and down both on the yellow highlight and slider fails to change the value. 
If it helps, please let me know and I can attach video of the experience seen. Thanks

Comment 7

[Punam Dahiya [:pdahiya]]

Attachment: Screen Shot of exposure screen with screen reader enabled

Comment 8

[Eitan Isaacson [:eeejay]]

I tested this with m-c gecko and master gaia:

https://www.dropbox.com/s/z92sfqhtql1wk2q/2015-02-18%2018.33.20.mp4?dl=0

Comment 9

[Punam Dahiya [:pdahiya]]

Comment on attachment 8554063 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/27649

Thanks Eitan for uploading video, it helps a lot and I am able to use slider with screen reader enabled. I guess the key is to do it slow and give gesture enough time so that it doesn't change focus to preview image.

Please add comment explaining screen reader slider swipe up and down code and it's good to land.

Comment 10

[Eitan Isaacson [:eeejay]]

https://github.com/mozilla-b2g/gaia/commit/1c2fb4e76bc879cbb85c2390b8dc7456b3a8e478

Comment 11

[Eitan Isaacson [:eeejay]]

Comment on attachment 8554063 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/27649

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Brightness slider won't be accessible to screen reader users.
[Testing completed]: Yes, this has been on master for a while.
[Risk to taking this patch] (and alternatives if risky): None.
[String changes made]: None.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/445ebd59a4bfa36099dfc635a67706f2b73b0998