Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 1132191 - improved overall accessibility of the video app. #28342

Merged
merged 1 commit into from Mar 9, 2015
Merged

Bug 1132191 - improved overall accessibility of the video app. #28342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
118 changes: 74 additions & 44 deletions apps/video/index.html
View file
Open in desktop
Expand Up @@ -64,59 +64,85 @@ <h1 id="thumbnails-number-selected"></h1>

<!-- This element gets positioned over different view elements -->
<!-- It gets styled differently depending on what it is over -->
<ul id="thumbnails"></ul> <!-- Thumbnails inserted here -->
<!-- The real thumbnails list is actually inside this ul. We need
to remove the semantics of this list to be less verbose, if the
screen reader is on. -->
<ul role="presentation" id="thumbnails"></ul> <!-- Thumbnails inserted here -->
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be helpful to include a comment regarding the "presentation" role and why it's being used here. The comment can be written such that it can be used to understand why the "presentation" role is being used on other elements in the file so that the same comment doesn't need to be included wherever the role is used.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"in the screen reader is on" --> "if the screen reader is on"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, ill clean that up


<footer id="thumbnails-bottom" class="thumbnails-list">
<a id="thumbnails-video-button" class="button" data-icon="video"></a>
<a id="thumbnails-select-button" class="button" data-icon="select"></a>
<a id="thumbnails-video-button" class="button" data-icon="video"
role="button" data-l10n-id="video-button"></a>
<a id="thumbnails-select-button" class="button" data-icon="select"
role="button" data-l10n-id="select-button"></a>
</footer>
<footer id="thumbnails-select-bottom" class="thumbnails-select">
<a id="thumbnails-delete-button" class="button" data-icon="delete"></a>
<a id="thumbnails-share-button" class="button" data-icon="share"></a>
<a id="thumbnails-delete-button" class="button" data-icon="delete"
role="button" data-l10n-id="delete-button"></a>
<a id="thumbnails-share-button" class="button" data-icon="share"
role="button" data-l10n-id="share-button"></a>
</footer>
</section>

<!-- (maximized) video player -->
<section role="region" id="player-view">
<!-- display this at startup while we create thumbnails -->
<div id="spinner-overlay" class="hidden"><div id="spinner"></div></div>
<div id="video-container">
<video src="about:blank" id="player"></video>
<div id="spinner-overlay" class="hidden" role="progress"
data-l10n-id="spinner-overlay"><div id="spinner"></div></div>
<!-- video controls header -->
<gaia-header id="player-header" action="back" class="video-controls">
<h1 id="video-title"></h1>
<button id="options" data-icon="more"
data-l10n-id="options-more"></button>
<button id="picker-done" data-l10n-id="done">Done</button>
</gaia-header>
<!-- In order for the screen reader to be able to activate the container
to toggle controls, it must have the semantics of the button -->
<div id="video-container" role="button">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a comment about how the video container is acting like a button.

<!-- Hiding the video element from the screen reader since the
controls are implemented elsewhere. -->
<video src="about:blank" id="player" aria-hidden="true"></video>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recognizing my ARIA knowledge is limited, what I've read says the "aria-hidden" attribute should be set to "false" or removed when the element is presented. I don't see that happening for the elements for which 'aria-hidden="true"' has been added.

</div>
<!-- video controls -->
<div id="videoControls">
<section role="region">
<gaia-header id="player-header" action="back">
<h1 id="video-title"></h1>
<button id="options" data-icon="more"></button>
<button id="picker-done" data-l10n-id="done">Done</button>
</gaia-header>
<footer id="videoBar">
<div id="timeSlider">
<span id="elapsed-text"></span>
<div id="slider-wrapper">
<div id="elapsedTime" class="progress"></div>
<div id="bufferedTime" class="progress"></div>
<div id="timeBackground" class="progress"></div>
<button id="playHead"></button>
</div>
<span id="duration-text"></span>
</div>
<div id="fullscreen-button"></div>
</footer>
<footer id="videoControlBar">
<div id="videoToolBar">
<button id="seek-backward" class="player-controls-button" data-icon="skip-back"></button>
<button id="play" class="player-controls-button" data-icon="pause"></button>
<button id="seek-forward" class="player-controls-button" data-icon="skip-forward"></button>
<!-- video controls footer toolbar -->
<div role="toolbar" class="video-controls bottom">
<!-- It is simpler and less verbose (for the screen reader) to not
expose that this block is a footer and consider it part of
its toolbar container. -->
<footer id="videoBar" role="presentation">
<div id="timeSlider" role="slider">
<!-- We need to use aria hideen for the text nodes in the slider
subtree because the screen reader treats sliders as atomic
elements and does not step inside their subtree. All the
information is conveyed via the slider attributes instead -->
<span id="elapsed-text" aria-hidden="true"></span>
<div id="slider-wrapper" aria-hidden="true">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use 'aria-hidden=true' here and on the other time slider children?

<div id="elapsedTime" class="progress"></div>
<div id="bufferedTime" class="progress"></div>
<div id="timeBackground" class="progress"></div>
<button id="playHead"></button>
</div>
</footer>
</section>
<span id="duration-text" aria-hidden="true"></span>
</div>
<div id="fullscreen-button" role="button"
data-l10n-id="fullscreen-button"></div>
</footer>
<!-- It is simpler and less verbose (for the screen reader) to not
expose that this block is a footer and consider it part of
its toolbar container. -->
<footer id="videoControlBar" role="presentation">
<div id="videoToolBar">
<button id="seek-backward" class="player-controls-button"
data-icon="skip-back" data-l10n-id="seek-backward"></button>
<button id="play" class="player-controls-button" data-icon="pause"></button>
<button id="seek-forward" class="player-controls-button"
data-icon="skip-forward" data-l10n-id="seek-forward"></button>
</div>
</footer>
</div>
</section>
</div>

<form id="info-view" role="dialog" data-type="confirm" class="hidden">
<form id="info-view" role="dialog" data-type="confirm" class="hidden"
data-l10n-id="info-view">
<section>
<dl>
<dt data-l10n-id="name-label"></dt>
Expand All @@ -139,7 +165,8 @@ <h1 id="video-title"></h1>
</menu>
</form>

<form id="options-view" role="dialog" data-type="action" class="hidden">
<form id="options-view" role="dialog" data-type="action" class="hidden"
data-l10n-id="options-view">
<menu>
<button type="button" class="single-share-button"><span class="gi gi-text" data-l10n-id="share">Share</span></button>
<button type="button" class="single-info-button"><span class="gi gi-text" data-l10n-id="more-info">More info</span></button>
Expand All @@ -151,18 +178,20 @@ <h1 id="video-title"></h1>
<!-- This element is the template of each thumbnail item -->
<div id="thumbnail-group-header" class="hidden">
<!--
<li>
<div class="thumbnail-group-header"></div>
<ul class="thumbnail-group-container"></ul>
<li role="presentation">
<div class="thumbnail-group-header" role="heading"
aria-level="1"></div>
<ul class="thumbnail-group-container" role="listbox"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why 'aria-level' is used here in the thumbnail-group-header template but not in the thumbnail template below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So here the header is for the group and it makes sense to expose it semantically as such. The thumbnail is represented as an option (actionable list item within a listbox). Our screen reader utters options in one sentence and treats them as atomic elements so we opt out for being terse there.

data-l10n-id="videos-list" aria-multiselectable="true"></ul>
</li>
-->
</div>
<div id="thumbnail-template" class="hidden">
<!--
<li class="thumbnail">
<li class="thumbnail" role="option">
<div class="inner">
<div class="unwatched"></div>
<div class="img"></div>
<div class="img" role="presentation"></div>
<div class="details">
<span class="title">${title}</span>
<span class="duration-text after line-break"> ${duration-text}</span>
Expand All @@ -189,7 +218,8 @@ <h1 id="overlay-title"></h1>
<button type="button" id="overlay-action-button" class="full"></button>
</menu>
</form>
<form id="confirm-dialog" role="dialog" data-type="confirm" class="hidden">
<form id="confirm-dialog" role="dialog" data-type="confirm" class="hidden"
data-l10n-id="confirm-dialog">
<section>
<p id="confirm-msg"></p>
</section>
Expand Down