Merge pull request #14098 from edx/caption-link-role

fix invalid role definition on interactive captions

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

@@ -74,18 +74,18 @@
     width: 0px;
     height: 0px;
   }
-  
+
     .downloads-heading {
         margin: 1em 0 0 0;
     }
-  
+
     .wrapper-downloads {
         display: flex;
-        
+
         .hd {
             margin: 0;
         }
-        
+
         .wrapper-download-video,
         .wrapper-download-transcripts,
         .wrapper-handouts,
@@ -95,27 +95,27 @@
             @include padding-right($baseline);
             vertical-align: top;
         }
-        
+
         .wrapper-download-video {
-            
+
             .video-sources {
                 margin: 0;
             }
         }
-        
+
         .wrapper-download-transcripts {
-            
+
             .list-download-transcripts {
                 margin: 0;
                 padding: 0;
                 list-style: none;
-                
+
                 .transcript-option {
                     margin: 0;
                 }
             }
         }
-        
+
         .branding {
             @include padding-right(0);
 
@@ -681,9 +681,9 @@
       }
     }
   }
-  
+
   &.video-fullscreen {
-      
+
       .closed-captions {
           width: 60%;
       }
@@ -705,7 +705,7 @@
     .subtitles-menu {
       height: 100%;
       margin: 0;
-      padding: 0;
+      padding: 0 3px;
       list-style: none;
 
       li {
@@ -716,6 +716,10 @@
         color: #0074b5; // AA compliant
         line-height: lh();
 
+        span {
+          display: block;
+        }
+
         &.current {
           color: #333;
           font-weight: 700;
@@ -734,10 +738,10 @@
         &:empty {
           margin-bottom: 0;
         }
-        
+
         &.spacing:last-of-type {
             position: relative;
-            
+
             .transcript-end {
                 position: absolute;
                 bottom: 0;

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

@@ -161,7 +161,7 @@
                     mousewheel: this.onMovement,
                     DOMMouseScroll: this.onMovement
                 })
-                .on(events, 'li[data-index]', this.onCaptionHandler);
+                .on(events, 'span[data-index]', this.onCaptionHandler);
                 this.container.on({
                     mouseenter: this.onContainerMouseEnter,
                     mouseleave: this.onContainerMouseLeave
@@ -739,16 +739,16 @@
             */
             buildCaptions: function(container, start, captions) {
                 var process = function(text, index) {
-                    var liEl = $('<li>', {
+                    var $spanEl = $('<span>', {
                         'role': 'link',
                         'data-index': index,
                         'data-start': start[index],
                         'tabindex': 0
                     });
 
-                    HtmlUtils.setHtml($(liEl), HtmlUtils.HTML(text.toString()));
+                    HtmlUtils.setHtml($($spanEl), HtmlUtils.HTML(text.toString()));
 
-                    return liEl[0];
+                    return $spanEl.wrap('<li>').parent()[0]; // safe-lint: disable=javascript-jquery-insertion
                 };
 
                 return AsyncProcess.array(captions, process).done(function(list) {
@@ -912,20 +912,21 @@
             */
             captionFocus: function(event) {
                 var caption = $(event.target),
+                    container = caption.parent(),
                     captionIndex = parseInt(caption.attr('data-index'), 10);
                 // If the focus comes from a mouse click, hide the outline, turn on
                 // automatic scrolling and set currentCaptionIndex to point outside of
                 // caption list (ie -1) to disable mouseenter, mouseleave behavior.
                 if (this.isMouseFocus) {
                     this.autoScrolling = true;
-                    caption.removeClass('focused');
+                    container.removeClass('focused');
                     this.currentCaptionIndex = -1;
                 }
                 // If the focus comes from tabbing, show the outline and turn off
                 // automatic scrolling.
                 else {
                     this.currentCaptionIndex = captionIndex;
-                    caption.addClass('focused');
+                    container.addClass('focused');
                     // The second and second to last elements turn automatic scrolling
                     // off again as it may have been enabled in captionBlur.
                     if (
@@ -945,9 +946,10 @@
             */
             captionBlur: function(event) {
                 var caption = $(event.target),
+                    container = caption.parent(),
                     captionIndex = parseInt(caption.attr('data-index'), 10);
 
-                caption.removeClass('focused');
+                container.removeClass('focused');
                 // If we are on first or last index, we have to turn automatic scroll
                 // on again when losing focus. There is no way to know in what
                 // direction we are tabbing. So we could be on the first element and
@@ -1057,11 +1059,12 @@
                         }
 
                         this.subtitlesEl
-                            .find("li[data-index='" + newIndex + "']")
+                            .find("span[data-index='" + newIndex + "']")
+                            .parent()
                             .addClass('current');
 
                         this.currentIndex = newIndex;
-                        this.captionDisplayEl.text(this.subtitlesEl.find("li[data-index='" + newIndex + "']").text());
+                        this.captionDisplayEl.text(this.subtitlesEl.find("span[data-index='" + newIndex + "']").text());
                         this.scrollCaption();
                     }
                 }

common/test/acceptance/pages/lms/video/video.py

@@ -32,8 +32,8 @@ CSS_CLASS_NAMES = {
     'captions_closed': '.video.closed',
     'captions_rendered': '.video.is-captions-rendered',
     'captions': '.subtitles',
-    'captions_text': '.subtitles li',
-    'captions_text_getter': '.subtitles li[role="link"][data-index="1"]',
+    'captions_text': '.subtitles li span',
+    'captions_text_getter': '.subtitles li span[role="link"][data-index="1"]',
     'closed_captions': '.closed-captions',
     'error_message': '.video .video-player .video-error',
     'video_container': '.video',

common/test/acceptance/pages/studio/video/video.py

@@ -277,7 +277,7 @@ class VideoComponentPage(VideoPage):
             line_number (int): caption line number
 
         """
-        caption_line_selector = ".subtitles li[data-index='{index}']".format(index=line_number - 1)
+        caption_line_selector = ".subtitles li span[data-index='{index}']".format(index=line_number - 1)
         self.q(css=caption_line_selector).results[0].send_keys(Keys.ENTER)
 
     def is_caption_line_focused(self, line_number):
@@ -288,10 +288,9 @@ class VideoComponentPage(VideoPage):
             line_number (int): caption line number
 
         """
-        caption_line_selector = ".subtitles li[data-index='{index}']".format(index=line_number - 1)
-        attributes = self.q(css=caption_line_selector).attrs('class')
-
-        return 'focused' in attributes
+        caption_line_selector = ".subtitles li span[data-index='{index}']".format(index=line_number - 1)
+        caption_container = self.q(css=caption_line_selector).results[0].find_element_by_xpath('..')
+        return 'focused' in caption_container.get_attribute('class').split()
 
     @property
     def is_slider_range_visible(self):