Addressed PR comments

CHANGELOG.rst

@@ -316,3 +316,6 @@ Common: Updated CodeJail.
 Common: Allow setting of authentication session cookie name.
 
 LMS: Option to email students when enroll/un-enroll them.
+
+Blades: Added WAI-ARIA markup to the video player controls. These are now fully
+accessible by screen readers. 

common/lib/xmodule/xmodule/css/video/display.scss

@@ -109,7 +109,6 @@ div.video {
         -webkit-transition: -webkit-transform 0.7s ease-in-out;
         -moz-transition: -moz-transform 0.7s ease-in-out;
         -ms-transition: -ms-transform 0.7s ease-in-out;
-        tabindex: -1;
         transition: transform 0.7s ease-in-out;
         @include transform(scaleY(0.5) translate3d(0, 50%, 0));
 

common/lib/xmodule/xmodule/js/src/video/04_video_control.js

@@ -65,23 +65,10 @@ function () {
         }
 
         // ARIA
-        // Let screen readers know that:
-
-        // These anchors behaves like buttons, named 'Play' or 'Pause', and 
-        // 'Fill screen' (the title attribute is set in video.html template).
-        state.videoControl.playPauseEl.attr({
-            'role': gettext('button'),
-            'aria-disabled': false
-        });
-
-        state.videoControl.fullScreenEl.attr({
-            'role': gettext('button'),
-            'aria-disabled': false
-        });
-        
-        // This anchor behaves as a slider named 'video slider'.
+        // Let screen readers know that this anchor, representing the slider
+        // handle, behaves as a slider named 'video slider'.
         state.videoControl.sliderEl.find('.ui-slider-handle').attr({
-            'role': gettext('slider'),
+            'role': 'slider',
             'title': gettext('video slider')
         });
     }
@@ -189,14 +176,14 @@ function () {
             this.videoControl.fullScreenState = false;
             fullScreenClassNameEl.removeClass('video-fullscreen');
             this.isFullScreen = false;
-            this.videoControl.fullScreenEl.attr('title', gettext('Fill browser'));
-            this.videoControl.fullScreenEl.text(gettext('Fill browser'));
+            this.videoControl.fullScreenEl.attr('title', gettext('Fill browser'))
+                                          .text(gettext('Fill browser'));
         } else {
             this.videoControl.fullScreenState = true;
             fullScreenClassNameEl.addClass('video-fullscreen');
             this.isFullScreen = true;
-            this.videoControl.fullScreenEl.attr('title', gettext('Exit full browser'));
-            this.videoControl.fullScreenEl.text(gettext('Exit full browser'));
+            this.videoControl.fullScreenEl.attr('title', gettext('Exit full browser'))
+                                          .text(gettext('Exit full browser'));
         }
 
         this.trigger('videoCaption.resize', null);

common/lib/xmodule/xmodule/js/src/video/05_video_quality_control.js

@@ -43,16 +43,6 @@ function () {
 
         state.videoQualityControl.el.show();
         state.videoQualityControl.quality = null;
-
-        // ARIA
-        // Let screen readers know that:
-
-        // This anchor behaves as a button named 'HD'.
-        // (the title attribute is set in video.html template).
-        state.videoQualityControl.el.attr({
-            'role': gettext('button'),
-            'aria-disabled': false
-        });
     }
 
     // function _bindHandlers(state)

common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js

@@ -56,11 +56,12 @@ function () {
         state.videoProgressSlider.handle = state.videoProgressSlider.el.find('.ui-slider-handle');
         
         // ARIA
-        // Let screen readers know that:
-
-        // This anchor behaves as a button named 'video position'.
+        // We just want the knob to be selectable with keyboard
+        state.videoProgressSlider.el.attr('tabindex', -1);
+        // Let screen readers know that this anchor, representing the slider
+        // handle, behaves as a slider named 'video position'.
         state.videoProgressSlider.handle.attr({
-            'role': gettext('slider'),
+            'role': 'slider',
             'title': 'video position',
             'aria-disabled': false,
             'aria-valuetext': getTimeDescription(state.videoProgressSlider.slider.slider('option', 'value'))

common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js

@@ -67,25 +67,29 @@ function () {
         // Let screen readers know that:
 
         // This anchor behaves as a button named 'Volume'.
-        // (the title attribute is set in video.html template).
-        var buttonStr = state.videoVolumeControl.currentVolume === 0 ? gettext('Volume muted') : gettext('Volume');
-        state.videoVolumeControl.buttonEl.attr({
-            'role': gettext('button'),
-            'aria-label': buttonStr, // Doesn't read the title attribute, why? 
-            'aria-disabled': false
-        });
-
-        // The anchor representing the slider handle behaves as a slider named
-        // volume.
-        state.videoVolumeControl.volumeSliderHandleEl = state.videoVolumeControl.volumeSliderEl.find('.ui-slider-handle');
+        var buttonStr = gettext(
+                state.videoVolumeControl.currentVolume === 0
+                ? 'Volume muted'
+                : 'Volume'
+            );
+        // We add the aria-label attribute because the title attribute cannot be
+        // read. 
+        state.videoVolumeControl.buttonEl.attr('aria-label', buttonStr);
+
+        // Let screen readers know that this anchor, representing the slider
+        // handle, behaves as a slider named 'volume'.
+        var volumeSlider = state.videoVolumeControl.slider;
+        state.videoVolumeControl.volumeSliderHandleEl = state.videoVolumeControl
+                                                             .volumeSliderEl
+                                                             .find('.ui-slider-handle');
         state.videoVolumeControl.volumeSliderHandleEl.attr({
-            'role': gettext('slider'),
+            'role': 'slider',
             'title': 'volume',
             'aria-disabled': false,
-            'aria-valuemin': state.videoVolumeControl.slider.slider('option', 'min'),
-            'aria-valuemax': state.videoVolumeControl.slider.slider('option', 'max'),
-            'aria-valuenow': state.videoVolumeControl.slider.slider('option', 'value'),
-            'aria-valuetext': getVolumeDescription(state.videoVolumeControl.slider.slider('option', 'value'))
+            'aria-valuemin': volumeSlider.slider('option', 'min'),
+            'aria-valuemax': volumeSlider.slider('option', 'max'),
+            'aria-valuenow': volumeSlider.slider('option', 'value'),
+            'aria-valuetext': getVolumeDescription(volumeSlider.slider('option', 'value'))
         });
     }
 
@@ -180,7 +184,9 @@ function () {
         });
        
         this.videoVolumeControl.buttonEl.attr(
-            'aria-label', this.videoVolumeControl.currentVolume === 0 ? gettext('Volume muted') : gettext('Volume')
+            'aria-label', this.videoVolumeControl.currentVolume === 0
+                          ? gettext('Volume muted')
+                          : gettext('Volume')
         );
     }
 

common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js

@@ -79,16 +79,6 @@ function () {
         });
 
         state.videoSpeedControl.setSpeed(state.speed);
-
-         // ARIA
-        // Let screen readers know that:
-
-        // This anchor behaves as a button named 'Speeds'.
-        // (the title attribute is set in video.html template).
-        state.videoSpeedControl.el.children('a').attr({
-            'role': gettext('button'),
-            'aria-disabled': false
-        });
     }
 
     /**

common/lib/xmodule/xmodule/js/src/video/09_video_caption.js

@@ -105,16 +105,6 @@ function () {
             this.videoCaption.hideCaptions(true);
             this.videoCaption.hideSubtitlesEl.hide();
         }
-
-        // ARIA
-        // Let screen readers know that:
-
-        // This anchor behaves as a button named 'CC'.
-        // (the title attribute is set in video.html template).
-        this.videoCaption.hideSubtitlesEl.attr({
-            'role': gettext('button'),
-            'aria-disabled': 'false'
-        });
     }
 
     // function bindHandlers()

lms/templates/video.html

@@ -46,27 +46,27 @@
 
                 <div>
                     <ul class="vcr">
-                        <li><a class="video_control" href="#" title="${_('Play')}"></a></li>
+                        <li><a class="video_control" href="#" title="${_('Play')}" role="button" aria-disabled="false"></a></li>
                         <li><div class="vidtime">0:00 / 0:00</div></li>
                     </ul>
                     <div class="secondary-controls">
                         <div class="speeds">
-                            <a href="#" title="Speeds">
+                            <a href="#" title="Speeds" role="button" aria-disabled="false">
                                 <h3>${_('Speed')}</h3>
                                 <p class="active"></p>
                             </a>
                             <ol class="video_speeds"></ol>
                         </div>
                         <div class="volume">
-                            <a href="#" title="Volume"></a>
+                            <a href="#" title="Volume" role="button" aria-disabled="false"></a>
                             <div class="volume-slider-container">
                                 <div class="volume-slider"></div>
                             </div>
                         </div>
-                        <a href="#" class="add-fullscreen" title="${_('Fill browser')}">${_('Fill browser')}</a>
-                        <a href="#" class="quality_control" title="${_('HD')}">${_('HD')}</a>
+                        <a href="#" class="add-fullscreen" title="${_('Fill browser')}" role="button" aria-disabled="false">${_('Fill browser')}</a>
+                        <a href="#" class="quality_control" title="${_('HD')}" role="button" aria-disabled="false">${_('HD')}</a>
 
-                        <a href="#" class="hide-subtitles" title="${_('Turn off captions')}">${_('Turn off captions')}</a>
+                        <a href="#" class="hide-subtitles" title="${_('Turn off captions')}" role="button" aria-disabled="false">${_('Turn off captions')}</a>
                     </div>
                 </div>
             </section>