Closed Bug 1132191 Opened 7 years ago Closed 7 years ago

Improve accessibility of the video app.

Categories

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

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S8 (20mar)
Tracking Status
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: yzen, Assigned: yzen)

References

Details

(Keywords: late-l10n, Whiteboard: [b2ga11y p=1])

Attachments

(1 file)

There are several controls that are inaccessible or not labelled for the screen reader.
Attachment #8567264 - Flags: review?(rnicoletti)
Comment on attachment 8567264 [details] [review]
[gaia] yzen:bug-1132191 > mozilla-b2g:master

Hi Yura,

Thanks for the very thorough PR.

I'm giving r-, mostly because I have some questions about the use of the existing video-container element and the new hide-controls element. I'd like to understand the need for those changes before approving.
Attachment #8567264 - Flags: review?(rnicoletti) → review-
Comment on attachment 8567264 [details] [review]
[gaia] yzen:bug-1132191 > mozilla-b2g:master

Hi Russ,
I think I got all your comments/concerns addressed. I ended up with the original markup (e.g. wihtout introducing the hide-button element). The only thing I had to do is to switch the order video container, header and footers, so the markup is consistent with presentation (Similar to the previous PR). Hopefully that works for you, thanks!
Attachment #8567264 - Flags: review- → review?(rnicoletti)
Comment on attachment 8567264 [details] [review]
[gaia] yzen:bug-1132191 > mozilla-b2g:master

Hi Yura, the changes look good. I have put some questions in the PR. 

There is one issue that I see that needs to be resolved. When toggling the video controls when the video does not fill the entire screen, the background -- not covered by the video -- flashes blue. This can be seen by changing the orientation of the device to landscape when viewing a video taking in portrait orientation or vice-versa.

I'm going to leave the review r? until this is resolved.
Flags: needinfo?(yzenevich)
Comment on attachment 8567264 [details] [review]
[gaia] yzen:bug-1132191 > mozilla-b2g:master

Ok, I updated the pull request and it fixed the issue when video container was getting :active background change.
Flags: needinfo?(yzenevich)
Comment on attachment 8567264 [details] [review]
[gaia] yzen:bug-1132191 > mozilla-b2g:master

Looks good, thanks for the changes. I have one last comment in the PR regarding 'video-controls-bottom'.
Attachment #8567264 - Flags: review?(rnicoletti) → review+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment on attachment 8567264 [details] [review]
[gaia] yzen:bug-1132191 > mozilla-b2g:master

[Approval Request Comment] This PR makes video app awesome for the screen reader
[Bug caused by] (feature/regressing bug #): improvement, not a bug
[User impact] if declined: if declined the video app will stay inaccessible to the screen reader
[Testing completed]: unit + on device
[Risk to taking this patch] (and alternatives if risky): some markup and css changes across the app
[String changes made]: https://github.com/mozilla-b2g/gaia/pull/28342/files#diff-55698edbdb20279da9494aafdf4c3d48
Attachment #8567264 - Flags: approval-gaia-v2.2?
Attachment #8567264 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
You need to log in before you can comment on or make changes to this bug.