Bug 127065 – AX: Shadow DOM video player controls menus need aria-owns on the trigger buttons

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[James Craig]

AX: Shadow DOM video player controls menus need aria-owns on the trigger buttons

e.g. the CC button needs an aria-owns pointer to the CC track selection menu element.

[Radar WebKit Bug Importer]

<rdar://problem/15828906>

[Radar WebKit Bug Importer]

<rdar://problem/15828899>

[Aaron Chu]

Created attachment 264923 [details]
Patch

[Aaron Chu]

Created attachment 264924 [details]
Patch

[WebKit Commit Bot]

Attachment 264924 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Aaron Chu]

Created attachment 264927 [details]
Patch

[Darin Adler]

Comment on attachment 264927 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=264927&action=review

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:1650
> +        this.captionMenu.setAttribute('id', 'webkitMediaAudioTrackMenu');

Why the webkitMedia prefix here? I don’t think it has any value. If there’s a real issue with ID conflicts, then the prefix is not sufficient, and we need to solve the problem of ID conflicts. If there’s not, and the ID is scoped to the media controls shadow tree then the ID should just be 'audioTrackMenu'.

> Source/WebCore/Modules/mediacontrols/mediaControlsBase.js:1137
> +        this.captionMenu.setAttribute('id', 'webkitMediaAudioTrackMenu');

Ditto.

[Aaron Chu]

Created attachment 264997 [details]
Patch

[Aaron Chu]

Comment on attachment 264997 [details]
Patch

>Subversion Revision: 192096
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 44a67820f2c573fd8b4e4bdac3989e194ed5cde5..f3732e9f8ce141cb064789472c70e1f7290c8f3c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>[phone]  Aaron Chu  <[e-mail]>
>+
>+        The Closed Captions button is the only control that has a popup menu. This patch Updates createControls() 
>+        so that when it creates the Closed Captions button, it adds the aria-owns attribute references an id on
>+        the Audio Track Menu, which is also added with this patch.
>+
>+        AX: Shadow DOM video player controls menus need aria-owns on the trigger buttons
>+        https://bugs.webkit.org/show_bug.cgi?id=127065
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: media/accessibility-closed-captions-has-aria-owns.html
>+
>+        * Modules/mediacontrols/mediaControlsApple.js:
>+        (Controller.prototype.createControls):
>+        (Controller.prototype.buildCaptionMenu):
>+        * Modules/mediacontrols/mediaControlsBase.js:
>+        (Controller.prototype.createControls):
>+        (Controller.prototype.buildCaptionMenu):
>+
> 2015-11-05  Carlos Garcia Campos  <[e-mail]>
> 
>         [GStreamer] Use RunLoop::Timer instead of GMainLoopSource in video sink
>diff --git a/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js b/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js
>index 7734d1f3a2b8d9c4b25644277d87d97046395a14..b759104f545ce6e27c7b9ca6232d75472f275839 100644
>--- a/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js
>+++ b/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js
>@@ -468,6 +468,7 @@ Controller.prototype = {
>         captionButton.setAttribute('pseudo', '-webkit-media-controls-toggle-closed-captions-button');
>         captionButton.setAttribute('aria-label', this.UIString('Captions'));
>         captionButton.setAttribute('aria-haspopup', 'true');
>+        captionButton.setAttribute('aria-owns', 'audioTrackMenu');
>         this.listenFor(captionButton, 'click', this.handleCaptionButtonClicked);
> 
>         var fullscreenButton = this.controls.fullscreenButton = document.createElement('button');
>@@ -1646,6 +1647,7 @@ Controller.prototype = {
> 
>         this.captionMenu = document.createElement('div');
>         this.captionMenu.setAttribute('pseudo', '-webkit-media-controls-closed-captions-container');
>+        this.captionMenu.setAttribute('id', 'audioTrackMenu');
>         this.base.appendChild(this.captionMenu);
>         this.captionMenuItems = [];
> 
>diff --git a/Source/WebCore/Modules/mediacontrols/mediaControlsBase.js b/Source/WebCore/Modules/mediacontrols/mediaControlsBase.js
>index 2124d47cfa3826ade6a648f133adf9795fd8b2a5..7c7e2883b2f30667e500eec1e2fd13a1c34c92b6 100644
>--- a/Source/WebCore/Modules/mediacontrols/mediaControlsBase.js
>+++ b/Source/WebCore/Modules/mediacontrols/mediaControlsBase.js
>@@ -396,6 +396,7 @@ Controller.prototype = {
>         captionButton.setAttribute('pseudo', '-webkit-media-controls-toggle-closed-captions-button');
>         captionButton.setAttribute('aria-label', this.UIString('Captions'));
>         captionButton.setAttribute('aria-haspopup', 'true');
>+        captionButton.setAttribute('aria-owns', 'audioTrackMenu');
>         this.listenFor(captionButton, 'click', this.handleCaptionButtonClicked);
> 
>         var fullscreenButton = this.controls.fullscreenButton = document.createElement('button');
>@@ -1133,6 +1134,7 @@ Controller.prototype = {
> 
>         this.captionMenu = document.createElement('div');
>         this.captionMenu.setAttribute('pseudo', '-webkit-media-controls-closed-captions-container');
>+        this.captionMenu.setAttribute('id', 'audioTrackMenu');
>         this.base.appendChild(this.captionMenu);
>         this.captionMenuItems = [];
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ad086dedccf99d5110bca8163cf532b0ecbd042f..c906bfaaeaa3268d16a4c6a5db5ad260219bd463 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>[phone]  Aaron Chu  <[e-mail]>
>+
>+        AX: Shadow DOM video player controls menus need aria-owns on the trigger buttons
>+        https://bugs.webkit.org/show_bug.cgi?id=127065
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * media/accessibility-closed-captions-has-aria-owns-expected.txt: Added.
>+        * media/accessibility-closed-captions-has-aria-owns.html: Added.
>+
> 2015-11-05  Sukolsak Sakshuwong  <[e-mail]>
> 
>         Layout Test js/intl-collator.html is crashing on win 7 debug
>diff --git a/LayoutTests/media/accessibility-closed-captions-has-aria-owns-expected.txt b/LayoutTests/media/accessibility-closed-captions-has-aria-owns-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..da1ca78d61679aba2e01fd4173d0ce65a7499d76
>--- /dev/null
>+++ b/LayoutTests/media/accessibility-closed-captions-has-aria-owns-expected.txt
>@@ -0,0 +1,28 @@
>+Bug 127065: AX: Shadow DOM video player controls menus need aria-owns on the trigger buttons
>+
>+Does the `aria-owns` on the CC button exist?
>+
>+PASS captionsButtonARIAOwnsValue is not null
>+
>+
>+
>+Did the Audio Track menu show up after the CC button is clicked?
>+
>+PASS captionsTrackMenuHTMLElement is not null
>+
>+
>+
>+Does the `id` of the menu exist?
>+
>+PASS closedCaptionsTrackMenuIdValue is not null
>+
>+
>+
>+Is the `aria-own` on the CC button equal to the `id` on the menu?
>+
>+PASS captionsButtonARIAOwnsValue is closedCaptionsTrackMenuIdValue
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>+
>diff --git a/LayoutTests/media/accessibility-closed-captions-has-aria-owns.html b/LayoutTests/media/accessibility-closed-captions-has-aria-owns.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..a7f6ddfc8b9db46740c348e01e65008ec802bb60
>--- /dev/null
>+++ b/LayoutTests/media/accessibility-closed-captions-has-aria-owns.html
>@@ -0,0 +1,91 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <script src="../resources/js-test-pre.js"></script>
>+    <script src="media-file.js"></script>
>+    <script src="video-test.js"></script>
>+    <script src="media-controls.js"></script>
>+</head>
>+<body>
>+    <p> Bug <a href="https://bugs.webkit.org/show_bug.cgi?id=127065">127065</a>: AX: Shadow DOM video player controls menus need aria-owns on the trigger buttons</p>
>+    <p id="console"></p>
>+    <video id="video" controls width="500">
>+        <source src="../media/content/counting.mp4" type="video/mp4">
>+        <track src="track/captions-webvtt/captions.vtt" label="English captions" kind="captions" srclang="en-us" default>
>+    </video>
>+
>+    <script>
>+        var captionsButtonHTMLElement;
>+        var captionsButtonARIAOwnsValue;
>+        var captionsTrackMenuHTMLElement;
>+        var closedCaptionsTrackMenuIdValue;
>+        var video;
>+
>+        if(!window.testRunner || !window.internals) {
>+           failTest();
>+        }
>+
>+        testRunner.dumpAsText();
>+
>+        start();
>+
>+        function start() 
>+        {
>+
>+            video = document.getElementById('video');
>+            
>+            captionsButtonHTMLElement = mediaControlsElement(internals.shadowRoot(video).firstChild, '-webkit-media-controls-toggle-closed-captions-button');
>+
>+            checkForCCButtonARIAOwns();
>+
>+            captionsButtonHTMLElement.click();
>+
>+            ensureMenuExist();
>+
>+            checkForCCTrackMenuId();
>+
>+            confirmAriaOwnsRelationship();
>+            
>+            testRunner.notifyDone();
>+        }
>+
>+        function log(message)
>+        {
>+            document.getElementById("console").appendChild(document.createTextNode(message + '\n\n'));
>+        }
>+
>+        function checkForCCButtonARIAOwns() 
>+        {
>+            captionsButtonARIAOwnsValue = captionsButtonHTMLElement.getAttribute('aria-owns');
>+            log('Does the `aria-owns` on the CC button exist?');
>+            shouldNotBe('captionsButtonARIAOwnsValue', 'null');
>+            log('\n');
>+        }
>+
>+        function ensureMenuExist()
>+        {
>+            captionsTrackMenuHTMLElement = mediaControlsElement(internals.shadowRoot(video).firstChild, '-webkit-media-controls-closed-captions-container');
>+            log('Did the Audio Track menu show up after the CC button is clicked?');
>+            shouldNotBe('captionsTrackMenuHTMLElement','null');
>+            log('\n');
>+        }
>+
>+        function checkForCCTrackMenuId() 
>+        {
>+            closedCaptionsTrackMenuIdValue = captionsTrackMenuHTMLElement.getAttribute('id');
>+            log('Does the `id` of the menu exist?');
>+            shouldNotBe('closedCaptionsTrackMenuIdValue', 'null');
>+            log('\n');
>+        }
>+
>+        function confirmAriaOwnsRelationship()
>+        {
>+            log('Is the `aria-own` on the CC button equal to the `id` on the menu?');
>+            shouldBe('captionsButtonARIAOwnsValue', 'closedCaptionsTrackMenuIdValue');
>+        }
>+
>+    </script>
>+
>+    <script src="../resources/js-test-post.js"></script>
>+</body>
>+</html>
>\ No newline at end of file

[James Craig]

Comment on attachment 264997 [details]
Patch

CQ+

[WebKit Commit Bot]

Comment on attachment 264997 [details]
Patch

Clearing flags on attachment: 264997

Committed r192570: <http://trac.webkit.org/changeset/192570>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.