Merge pull request #2381 from edx/jmclaus/feature_video_speed_control_improved_a11y

Keyboard events and ARIA markup added to speed control

common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js

@@ -53,12 +53,6 @@
                         'aria-disabled': 'false'
                     });
                 });
-
-                it('bind to change video speed link', function () {
-                    expect($('.video_speeds a')).toHandleWith(
-                        'click', state.videoSpeedControl.changeVideoSpeed
-                    );
-                });
             });
 
             describe('when running on touch based device', function () {
@@ -73,71 +67,184 @@
             });
 
             describe('when running on non-touch based device', function () {
+                var speedControl, speedEntries,
+                    KEY = $.ui.keyCode,
+
+                    keyPressEvent = function(key) {
+                        return $.Event('keydown', {keyCode: key});
+                    },
+
+                    tabBackPressEvent = function() {
+                        return $.Event('keydown',
+                                       {keyCode: KEY.TAB, shiftKey: true});
+                    },
+
+                    tabForwardPressEvent = function() {
+                        return $.Event('keydown',
+                                       {keyCode: KEY.TAB, shiftKey: false});
+                    },
+
+                    // Get previous element in array or cyles back to the last
+                    // if it is the first.
+                    previousSpeed = function(index) {
+                        return speedEntries.eq(index < 1 ?
+                                               speedEntries.length - 1 :
+                                               index - 1);
+                    },
+
+                    // Get next element in array or cyles back to the first if
+                    // it is the last.
+                    nextSpeed = function(index) {
+                        return speedEntries.eq(index >= speedEntries.length-1 ?
+                                               0 :
+                                               index + 1);
+                    };
+
                 beforeEach(function () {
                     state = jasmine.initializePlayer();
+                    speedControl = $('div.speeds');
+                    speedEntries = speedControl.children('a');
+                    spyOn($.fn, 'focus').andCallThrough();
                 });
 
-                it('open the speed toggle on hover', function () {
-                    $('.speeds').mouseenter();
-                    expect($('.speeds')).toHaveClass('open');
-
-                    $('.speeds').mouseleave();
-                    expect($('.speeds')).not.toHaveClass('open');
-                });
-
-                it('close the speed toggle on mouse out', function () {
-                    $('.speeds').mouseenter().mouseleave();
-
-                    expect($('.speeds')).not.toHaveClass('open');
-                });
-
-                it('close the speed toggle on click', function () {
-                    $('.speeds').mouseenter().click();
-
-                    expect($('.speeds')).not.toHaveClass('open');
-                });
-
-                // Tabbing depends on the following order:
-                // 1. Play anchor
-                // 2. Speed anchor
-                // 3. A number of speed entry anchors
-                // 4. Volume anchor
-                // If another focusable element is inserted or if the order is
-                // changed, things will malfunction as a flag,
-                // state.previousFocus, is set in the 1,3,4 elements and is
-                // used to determine the behavior of foucus() and blur() for
-                // the speed anchor.
-                it(
-                    'checks for a certain order in focusable elements in ' +
-                    'video controls',
-                    function ()
-                {
-                    var foundFirst = false,
-                        playIndex, speedIndex, firstSpeedEntry, lastSpeedEntry,
-                        volumeIndex;
-
-                    $('.video-controls').find('a, :focusable').each(
-                        function (index)
-                    {
-                        if ($(this).hasClass('video_control')) {
-                            playIndex = index;
-                        } else if ($(this).parent().hasClass('speeds')) {
-                            speedIndex = index;
-                        } else if ($(this).hasClass('speed_link')) {
-                            if (!foundFirst) {
-                                firstSpeedEntry = index;
-                                foundFirst = true;
-                            }
-
-                            lastSpeedEntry = index;
-                        } else if ($(this).parent().hasClass('volume')) {
-                            volumeIndex = index;
-                        }
-                    });
+                it('open/close the speed menu on mouseenter/mouseleave',
+                   function () {
+                    speedControl.mouseenter();
+                    expect(speedControl).toHaveClass('open');
+                    speedControl.mouseleave();
+                    expect(speedControl).not.toHaveClass('open');
+                });
+
+                it('do not close the speed menu on mouseleave if a speed ' +
+                    'entry has focus', function () {
+                    // Open speed meenu. Focus is on last speed entry.
+                    speedControl.trigger(keyPressEvent(KEY.ENTER));
+                    speedControl.mouseenter().mouseleave();
+                    expect(speedControl).toHaveClass('open');
+                });
+
+                it('close the speed menu on click', function () {
+                    speedControl.mouseenter().click();
+                    expect(speedControl).not.toHaveClass('open');
+                });
+
+                it('close the speed menu on outside click', function () {
+                    speedControl.trigger(keyPressEvent(KEY.ENTER));
+                    $(window).click();
+                    expect(speedControl).not.toHaveClass('open');
+                });
+
+                it('open the speed menu on ENTER keydown', function () {
+                    speedControl.trigger(keyPressEvent(KEY.ENTER));
+                    expect(speedControl).toHaveClass('open');
+                    expect(speedEntries.last().focus).toHaveBeenCalled();
+                });
+
+                it('open the speed menu on SPACE keydown', function () {
+                    speedControl.trigger(keyPressEvent(KEY.SPACE));
+                    expect(speedControl).toHaveClass('open');
+                    expect(speedEntries.last().focus).toHaveBeenCalled();
+                });
+
+                it('open the speed menu on UP keydown', function () {
+                    speedControl.trigger(keyPressEvent(KEY.UP));
+                    expect(speedControl).toHaveClass('open');
+                    expect(speedEntries.last().focus).toHaveBeenCalled();
+                });
+
+                it('close the speed menu on ESCAPE keydown', function () {
+                    speedControl.trigger(keyPressEvent(KEY.ESCAPE));
+                    expect(speedControl).not.toHaveClass('open');
+                });
+
+                it('UP and DOWN keydown function as expected on speed entries',
+                   function () {
+                    // Iterate through list in both directions and check if
+                    // things wrap up correctly.
+                    var lastEntry = speedEntries.length-1, i;
+
+                    // First open menu
+                    speedControl.trigger(keyPressEvent(KEY.UP));
+
+                    // Iterate with UP key until we have looped.
+                    for (i = lastEntry; i >= 0; i--) {
+                        speedEntries.eq(i).trigger(keyPressEvent(KEY.UP));
+                    }
+
+                    // Iterate with DOWN key until we have looped.
+                    for (i = 0; i <= lastEntry; i++) {
+                        speedEntries.eq(i).trigger(keyPressEvent(KEY.DOWN));
+                    }
+                    // Test if each element has been called twice.
+                    expect($.fn.focus.calls.length)
+                        .toEqual(2*speedEntries.length);
+                });
+
+                it('ESC keydown on speed entry closes menu', function () {
+                    // First open menu. Focus is on last speed entry.
+                    speedControl.trigger(keyPressEvent(KEY.UP));
+                    speedEntries.last().trigger(keyPressEvent(KEY.ESCAPE));
 
-                    expect(playIndex+1).toEqual(speedIndex);
-                    expect(speedIndex+1).toEqual(firstSpeedEntry);
-                    expect(lastSpeedEntry+1).toEqual(volumeIndex);
+                    // Menu is closed and focus has been returned to speed
+                    // control.
+                    expect(speedControl).not.toHaveClass('open');
+                    expect(speedControl.focus).toHaveBeenCalled();
+                });
+
+                it('ENTER keydown on speed entry selects speed and closes menu',
+                   function () {
+                    // First open menu.
+                    speedControl.trigger(keyPressEvent(KEY.UP));
+                    // Focus on 1.50x speed
+                    speedEntries.eq(1).focus();
+                    speedEntries.eq(1).trigger(keyPressEvent(KEY.ENTER));
+
+                    // Menu is closed, focus has been returned to speed
+                    // control and video speed is 1.50x.
+                    expect(speedControl.focus).toHaveBeenCalled();
+                    expect($('.video_speeds li[data-speed="1.50"]'))
+                        .toHaveClass('active');
+                    expect($('.speeds p.active')).toHaveHtml('1.50x');
+                });
+
+                it('SPACE keydown on speed entry selects speed and closes menu',
+                   function () {
+                    // First open menu.
+                    speedControl.trigger(keyPressEvent(KEY.UP));
+                    // Focus on 1.50x speed
+                    speedEntries.eq(1).focus();
+                    speedEntries.eq(1).trigger(keyPressEvent(KEY.SPACE));
+
+                    // Menu is closed, focus has been returned to speed
+                    // control and video speed is 1.50x.
+                    expect(speedControl.focus).toHaveBeenCalled();
+                    expect($('.video_speeds li[data-speed="1.50"]'))
+                        .toHaveClass('active');
+                    expect($('.speeds p.active')).toHaveHtml('1.50x');
+                });
+
+                it('TAB + SHIFT keydown on speed entry closes menu and gives ' +
+                    'focus to Play/Pause control', function () {
+                    // First open menu. Focus is on last speed entry.
+                    speedControl.trigger(keyPressEvent(KEY.UP));
+                    speedEntries.last().trigger(tabBackPressEvent());
+
+                    // Menu is closed and focus has been given to Play/Pause
+                    // control.
+                    expect(state.videoControl.playPauseEl.focus)
+                        .toHaveBeenCalled();
+                });
+
+                it('TAB keydown on speed entry closes menu and gives focus ' +
+                   'to Volume control', function () {
+                    // First open menu. Focus is on last speed entry.
+                    speedControl.trigger(keyPressEvent(KEY.UP));
+                    speedEntries.last().trigger(tabForwardPressEvent());
+
+                    // Menu is closed and focus has been given to Volume
+                    // control.
+                    expect(state.videoVolumeControl.buttonEl.focus)
+                        .toHaveBeenCalled();
                 });
             });
         });
@@ -163,30 +270,6 @@
                     expect(state.videoSpeedControl.currentSpeed).toEqual(0.75);
                 });
             });
-
-            describe(
-                'make sure the speed control gets the focus afterwards',
-                function ()
-            {
-                var anchor;
-
-                beforeEach(function () {
-                    state = jasmine.initializePlayer();
-                    anchor= $('.speeds > a').first();
-                    state.videoSpeedControl.setSpeed(1.0);
-                    spyOnEvent(anchor, 'focus');
-                });
-
-                it('when the speed is the same', function () {
-                    $('li[data-speed="1.0"] a').click();
-                    expect('focus').toHaveBeenTriggeredOn(anchor);
-                });
-
-                it('when the speed is not the same', function () {
-                    $('li[data-speed="0.75"] a').click();
-                    expect('focus').toHaveBeenTriggeredOn(anchor);
-                });
-            });
         });
 
         describe('onSpeedChange', function () {

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

@@ -117,6 +117,142 @@ function () {
         state.el.find('div.speeds').hide();
     }
 
+    // Get previous element in array or cyles back to the last if it is the
+    // first.
+    function _previousSpeedLink(speedLinks, index) {
+        return $(speedLinks.eq(index < 1 ? speedLinks.length - 1 : index - 1));
+    }
+
+    // Get next element in array or cyles back to the first if it is the last.
+    function _nextSpeedLink(speedLinks, index) {
+        return $(speedLinks.eq(index >= speedLinks.length - 1 ? 0 : index + 1));
+    }
+
+    function _speedLinksFocused(state) {
+        var speedLinks = state.videoSpeedControl.videoSpeedsEl
+                                                .find('a.speed_link');
+        return speedLinks.is(':focus');
+    }
+
+    function _openMenu(state) {
+        // When speed entries have focus, the menu stays open on
+        // mouseleave. A clickHandler is added to the window
+        // element to have clicks close the menu when they happen
+        // outside of it.
+        $(window).on('click.speedMenu', _clickHandler.bind(state));
+        state.videoSpeedControl.el.addClass('open');
+    }
+
+    function _closeMenu(state) {
+        // Remove the previously added clickHandler from window element.
+        $(window).off('click.speedMenu');
+        state.videoSpeedControl.el.removeClass('open');
+    }
+
+    // Various event handlers. They all return false to stop propagation and
+    // prevent default behavior.
+    function _clickHandler(event) {
+        var target = $(event.currentTarget);
+
+        this.videoSpeedControl.el.removeClass('open');
+        if (target.is('a.speed_link')) {
+            this.videoSpeedControl.changeVideoSpeed.call(this, event);
+        }
+
+        return false;
+    }
+
+    // We do not use _openMenu and _closeMenu in the following two handlers
+    // because we do not want to add an unnecessary clickHandler to the window
+    // element.
+    function _mouseEnterHandler(event) {
+        this.videoSpeedControl.el.addClass('open');
+
+        return false;
+    }
+
+    function _mouseLeaveHandler(event) {
+        // Only close the menu is no speed entry has focus.
+        if (!_speedLinksFocused(this)) {
+            this.videoSpeedControl.el.removeClass('open');
+        }
+                
+        return false;
+    }
+
+    function _keyDownHandler(event) {
+        var KEY = $.ui.keyCode,
+            keyCode = event.keyCode,
+            target = $(event.currentTarget),
+            speedButtonLink = this.videoSpeedControl.el.children('a'),
+            speedLinks = this.videoSpeedControl.videoSpeedsEl
+                                               .find('a.speed_link'),
+            index;
+
+        if (target.is('a.speed_link')) {
+
+            index = target.parent().index();
+
+            switch (keyCode) {
+                // Scroll up menu, wrapping at the top. Keep menu open.
+                case KEY.UP:
+                    _previousSpeedLink(speedLinks, index).focus();
+                    break;
+                // Scroll down  menu, wrapping at the bottom. Keep menu
+                // open.
+                case KEY.DOWN:
+                    _nextSpeedLink(speedLinks, index).focus();
+                    break;
+                // Close menu.
+                case KEY.TAB:
+                    _closeMenu(this);
+                    // Set focus to previous menu button in menu bar
+                    // (Play/Pause button)
+                    if (event.shiftKey) {
+                        this.videoControl.playPauseEl.focus();
+                    }
+                    // Set focus to next menu button in menu bar
+                    // (Volume button)
+                    else {
+                        this.videoVolumeControl.buttonEl.focus();
+                    }
+                    break;
+                // Close menu, give focus to speed control and change
+                // speed.
+                case KEY.ENTER:
+                case KEY.SPACE:
+                    _closeMenu(this);
+                    speedButtonLink.focus();
+                    this.videoSpeedControl.changeVideoSpeed.call(this, event);
+                    break;
+                // Close menu and give focus to speed control.
+                case KEY.ESCAPE:
+                    _closeMenu(this);
+                    speedButtonLink.focus();
+                    break;
+            }
+            return false;
+        }
+        else {
+            switch(keyCode) {
+                // Open menu and focus on last element of list above it.
+                case KEY.ENTER:
+                case KEY.SPACE:
+                case KEY.UP:
+                    _openMenu(this);
+                    speedLinks.last().focus();
+                    break;
+                // Close menu.
+                case KEY.ESCAPE:
+                    _closeMenu(this);
+                    break;
+            }
+            // We do not stop propagation and default behavior on a TAB
+            // keypress.
+            return event.keyCode === KEY.TAB;
+        }
+    }
+
     /**
      * @desc Bind any necessary function callbacks to DOM events (click,
      *     mousemove, etc.).
@@ -133,125 +269,21 @@ function () {
      * @returns {undefined}
      */
     function _bindHandlers(state) {
-        var speedLinks;
-
-        state.videoSpeedControl.videoSpeedsEl.find('a')
-            .on('click', state.videoSpeedControl.changeVideoSpeed);
-
-        if (state.isTouch) {
-            state.videoSpeedControl.el.on('click', function (event) {
-                // So that you can't highlight this control via a drag
-                // operation, we disable the default browser actions on a
-                // click event.
-                event.preventDefault();
-
-                state.videoSpeedControl.el.toggleClass('open');
-            });
-        } else {
-            state.videoSpeedControl.el
-                .on('mouseenter', function () {
-                    state.videoSpeedControl.el.addClass('open');
-                })
-                .on('mouseleave', function () {
-                    state.videoSpeedControl.el.removeClass('open');
-                })
-                .on('click', function (event) {
-                    // So that you can't highlight this control via a drag
-                    // operation, we disable the default browser actions on a
-                    // click event.
-                    event.preventDefault();
-
-                    state.videoSpeedControl.el.removeClass('open');
-                });
-
-            // ******************************
-            // The tabbing will cycle through the elements in the following
-            // order:
-            // 1. Play control
-            // 2. Speed control
-            // 3. Fastest speed called firstSpeed
-            // 4. Intermediary speed called otherSpeed
-            // 5. Slowest speed called lastSpeed
-            // 6. Volume control
-            // This field will keep track of where the focus is coming from.
-            state.previousFocus = '';
-
-            // ******************************
-            // Attach 'focus', and 'blur' events to the speed control which
-            // either brings up the speed dialog with individual speed entries,
-            // or closes it.
-            state.videoSpeedControl.el.children('a')
-                .on('focus', function () {
-                    // If the focus is coming from the first speed entry
-                    // (tabbing backwards) or last speed entry (tabbing forward)
-                    // hide the speed entries dialog.
-                    if (state.previousFocus === 'firstSpeed' ||
-                        state.previousFocus === 'lastSpeed') {
-                         state.videoSpeedControl.el.removeClass('open');
-                    }
-                })
-                .on('blur', function () {
-                    // When the focus leaves this element, the speed entries
-                    // dialog will be shown.
-
-                    // If we are tabbing forward (previous focus is play
-                    // control), we open the dialog and set focus on the first
-                    // speed entry.
-                    if (state.previousFocus === 'playPause') {
-                        state.videoSpeedControl.el.addClass('open');
-                        state.videoSpeedControl.videoSpeedsEl
-                        .find('a.speed_link:first')
-                        .focus();
-                    }
-
-                    // If we are tabbing backwards (previous focus is volume
-                    // control), we open the dialog and set focus on the
-                    // last speed entry.
-                    if (state.previousFocus === 'volume') {
-                        state.videoSpeedControl.el.addClass('open');
-                        state.videoSpeedControl.videoSpeedsEl
-                        .find('a.speed_link:last')
-                        .focus();
-                    }
-
-                });
-
-            // ******************************
-            // Attach 'blur' event to elements which represent individual speed
-            // entries and use it to track the origin of the focus.
-            speedLinks = state.videoSpeedControl.videoSpeedsEl
-                .find('a.speed_link');
-
-            speedLinks.first().on('blur', function () {
-                // The previous focus is a speed entry (we are tabbing
-                // backwards), the dialog will close, set focus on the speed
-                // control and track the focus on first speed.
-                if (state.previousFocus === 'otherSpeed') {
-                    state.previousFocus = 'firstSpeed';
-                    state.videoSpeedControl.el.children('a').focus();
-                }
-            });
-
-            // Track the focus on intermediary speeds.
-            speedLinks
-                .filter(function (index) {
-                    return index === 1 || index === 2;
-                })
-                .on('blur', function () {
-                    state.previousFocus = 'otherSpeed';
-                });
-
-            speedLinks.last().on('blur', function () {
-                // The previous focus is a speed entry (we are tabbing forward),
-                // the dialog will close, set focus on the speed control and
-                // track the focus on last speed.
-                if (state.previousFocus === 'otherSpeed') {
-                    state.previousFocus = 'lastSpeed';
-                    state.videoSpeedControl.el.children('a').focus();
-                }
-            });
+        var speedButton = state.videoSpeedControl.el,
+            videoSpeeds = state.videoSpeedControl.videoSpeedsEl;
+
+        // Attach various events handlers to the speed menu button.
+        speedButton.on({
+            'mouseenter': _mouseEnterHandler.bind(state),
+            'mouseleave': _mouseLeaveHandler.bind(state),
+            'click': _clickHandler.bind(state),
+            'keydown': _keyDownHandler.bind(state)
+        });
 
-        }
+        // Attach click and keydown event handlers to the individual speed
+        // entries.
+        videoSpeeds.on('click', 'a.speed_link', _clickHandler.bind(state))
+                   .on('keydown', 'a.speed_link', _keyDownHandler.bind(state));
     }
 
     // ***************************************************************
@@ -289,9 +321,6 @@ function () {
                 this.videoSpeedControl.currentSpeed
             );
         }
-        // When a speed entry has been selected, we want the speed control to
-        // regain focus.
-        parentEl.parent().siblings('a').focus();
     }
 
     function reRender(params) {
@@ -304,9 +333,9 @@ function () {
         $.each(this.videoSpeedControl.speeds, function (index, speed) {
             var link, listItem;
 
-            link = '<a class="speed_link" href="#">' + speed + 'x</a>';
+            link = '<a class="speed_link" href="#" role="menuitem">' + speed + 'x</a>';
 
-            listItem = $('<li data-speed="' + speed + '">' + link + '</li>');
+            listItem = $('<li data-speed="' + speed + '" role="presentation">' + link + '</li>');
 
             if (speed === params.currentSpeed) {
                 listItem.addClass('active');

lms/templates/video.html

@@ -71,7 +71,7 @@
                               <h3>${_('Speed')}</h3>
                               <p class="active"></p>
                           </a>
-                          <ol class="video_speeds"></ol>
+                          <ol class="video_speeds" role="menu"></ol>
                       </div>
                       <div class="volume">
                           <a href="#" title="${_('Volume')}" role="button" aria-disabled="false"></a>