FKA: No way to get focus from DOM to shadow DOM components (Was: HTML5 media controls...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 May 2016 21:01:32 +0000 (21:01 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 May 2016 21:01:32 +0000 (21:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=117857

Reviewed by Jer Noble.

Source/WebCore:

The bug was caused by hasCustomFocusLogic returning true on media elements.

Fix the bug by removing this function so that FocusController will walk into the shadow tree of media elements
to look for focusable elements. This will allow AT such as Voice Over to iterate through controls.

We don't seem to draw focus rings inside the media elements but that could be tweaked in a separate patch.

Test: media/tab-focus-inside-media-elements.html

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::hasCustomFocusLogic): Deleted.
* html/HTMLMediaElement.h:

LayoutTests:

Added a regression test for moving focus into media elements by pressing tab key.

* media/tab-focus-inside-media-elements-expected.txt: Added.
* media/tab-focus-inside-media-elements.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@200520 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/media/tab-focus-inside-media-elements-expected.txt [new file with mode: 0644]
LayoutTests/media/tab-focus-inside-media-elements.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/html/HTMLMediaElement.h

index 61cc42a..e12e2e9 100644 (file)
@@ -1,3 +1,15 @@
+2016-05-06  Ryosuke Niwa  <rniwa@webkit.org>
+
+        FKA: No way to get focus from DOM to shadow DOM components (Was: HTML5 media controls not keyboard accessible)
+        https://bugs.webkit.org/show_bug.cgi?id=117857
+
+        Reviewed by Jer Noble.
+
+        Added a regression test for moving focus into media elements by pressing tab key.
+
+        * media/tab-focus-inside-media-elements-expected.txt: Added.
+        * media/tab-focus-inside-media-elements.html: Added.
+
 2016-05-06  Filip Pizlo  <fpizlo@apple.com>
 
         JS Function removed after parsing
diff --git a/LayoutTests/media/tab-focus-inside-media-elements-expected.txt b/LayoutTests/media/tab-focus-inside-media-elements-expected.txt
new file mode 100644 (file)
index 0000000..eae05b4
--- /dev/null
@@ -0,0 +1,39 @@
+
+
+Tests for moving the focus onto controls inside an audio element and a video element.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.body.focus(); eventSender.keyDown("\t"); document.activeElement is mediaElements[0]
+PASS mediaElements[0] instanceof HTMLAudioElement is true
+PASS mediaElements[0].controls is true
+PASS eventSender.keyDown("\t"); document.activeElement is mediaElements[0]
+PASS !!internals.shadowRoot(mediaElements[0]).activeElement /* play button */ is true
+PASS eventSender.keyDown("\t"); document.activeElement is mediaElements[0]
+PASS !!internals.shadowRoot(mediaElements[0]).activeElement /* rewind button */ is true
+PASS eventSender.keyDown("\t"); document.activeElement is mediaElements[0]
+PASS !!internals.shadowRoot(mediaElements[0]).activeElement /* volume slider */ is true
+PASS eventSender.keyDown("\t"); document.activeElement is mediaElements[0]
+PASS !!internals.shadowRoot(mediaElements[0]).activeElement /* mute button */ is true
+PASS eventSender.keyDown("\t"); document.activeElement is mediaElements[1]
+PASS mediaElements[1] instanceof HTMLVideoElement is true
+PASS mediaElements[1].controls is true
+FAIL !!internals.shadowRoot(mediaElements[1]).activeElement /* play button */ should be true. Was false.
+PASS eventSender.keyDown("\t"); document.activeElement is mediaElements[1]
+PASS !!internals.shadowRoot(mediaElements[1]).activeElement /* rewind button */ is true
+PASS eventSender.keyDown("\t"); document.activeElement is mediaElements[1]
+PASS !!internals.shadowRoot(mediaElements[1]).activeElement /* volume slider */ is true
+PASS eventSender.keyDown("\t"); document.activeElement is mediaElements[1]
+PASS !!internals.shadowRoot(mediaElements[1]).activeElement /* mute button */ is true
+FAIL eventSender.keyDown("\t"); document.activeElement should be [object HTMLAudioElement]. Was [object HTMLVideoElement].
+PASS mediaElements[2] instanceof HTMLAudioElement is true
+PASS mediaElements[2].controls is false
+PASS eventSender.keyDown("\t"); document.activeElement is mediaElements[3]
+PASS mediaElements[3] instanceof HTMLVideoElement is true
+PASS mediaElements[3].controls is false
+PASS eventSender.keyDown("\t"); document.activeElement is document.querySelector("div")
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/media/tab-focus-inside-media-elements.html b/LayoutTests/media/tab-focus-inside-media-elements.html
new file mode 100644 (file)
index 0000000..ce8d061
--- /dev/null
@@ -0,0 +1,61 @@
+<!DOCTYPE html!>
+<html>
+<body>
+<audio controls></audio><video controls></video><br>
+<audio tabindex="0"></audio><video tabindex="0"></video>
+<div tabindex="0"></div>
+<div id="console"></div>
+<script src="../resources/js-test-pre.js"></script>
+<script>
+
+description('Tests for moving the focus onto controls inside an audio element and a video element.');
+
+var mediaElements = document.querySelectorAll("audio,video");
+
+if (window.testRunner)
+    runTests();
+else
+    log('This test requires eventSender');
+
+function runTests()
+{
+    testRunner.overridePreference("WebKitTabToLinksPreferenceKey", 1);
+
+    shouldBe('document.body.focus(); eventSender.keyDown("\\t"); document.activeElement', 'mediaElements[0]');
+    shouldBeTrue('mediaElements[0] instanceof HTMLAudioElement');
+    shouldBeTrue('mediaElements[0].controls');
+    shouldBe('eventSender.keyDown("\\t"); document.activeElement', 'mediaElements[0]');
+    shouldBeTrue('!!internals.shadowRoot(mediaElements[0]).activeElement /* play button */');
+    shouldBe('eventSender.keyDown("\\t"); document.activeElement', 'mediaElements[0]');
+    shouldBeTrue('!!internals.shadowRoot(mediaElements[0]).activeElement /* rewind button */');
+    shouldBe('eventSender.keyDown("\\t"); document.activeElement', 'mediaElements[0]');
+    shouldBeTrue('!!internals.shadowRoot(mediaElements[0]).activeElement /* volume slider */');
+    shouldBe('eventSender.keyDown("\\t"); document.activeElement', 'mediaElements[0]');
+    shouldBeTrue('!!internals.shadowRoot(mediaElements[0]).activeElement /* mute button */');
+
+    shouldBe('eventSender.keyDown("\\t"); document.activeElement', 'mediaElements[1]');
+    shouldBeTrue('mediaElements[1] instanceof HTMLVideoElement');
+    shouldBeTrue('mediaElements[1].controls');
+    shouldBeTrue('!!internals.shadowRoot(mediaElements[1]).activeElement /* play button */');
+    shouldBe('eventSender.keyDown("\\t"); document.activeElement', 'mediaElements[1]');
+    shouldBeTrue('!!internals.shadowRoot(mediaElements[1]).activeElement /* rewind button */');
+    shouldBe('eventSender.keyDown("\\t"); document.activeElement', 'mediaElements[1]');
+    shouldBeTrue('!!internals.shadowRoot(mediaElements[1]).activeElement /* volume slider */');
+    shouldBe('eventSender.keyDown("\\t"); document.activeElement', 'mediaElements[1]');
+    shouldBeTrue('!!internals.shadowRoot(mediaElements[1]).activeElement /* mute button */');
+
+    shouldBe('eventSender.keyDown("\\t"); document.activeElement', 'mediaElements[2]');
+    shouldBeTrue('mediaElements[2] instanceof HTMLAudioElement');
+    shouldBeFalse('mediaElements[2].controls');
+
+    shouldBe('eventSender.keyDown("\\t"); document.activeElement', 'mediaElements[3]');
+    shouldBeTrue('mediaElements[3] instanceof HTMLVideoElement');
+    shouldBeFalse('mediaElements[3].controls');
+
+    shouldBe('eventSender.keyDown("\\t"); document.activeElement', 'document.querySelector("div")');
+}
+
+</script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
index 24ff743..098088b 100644 (file)
@@ -1,3 +1,23 @@
+2016-05-06  Ryosuke Niwa  <rniwa@webkit.org>
+
+        FKA: No way to get focus from DOM to shadow DOM components (Was: HTML5 media controls not keyboard accessible)
+        https://bugs.webkit.org/show_bug.cgi?id=117857
+
+        Reviewed by Jer Noble.
+
+        The bug was caused by hasCustomFocusLogic returning true on media elements.
+
+        Fix the bug by removing this function so that FocusController will walk into the shadow tree of media elements
+        to look for focusable elements. This will allow AT such as Voice Over to iterate through controls.
+
+        We don't seem to draw focus rings inside the media elements but that could be tweaked in a separate patch.
+
+        Test: media/tab-focus-inside-media-elements.html
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::hasCustomFocusLogic): Deleted.
+        * html/HTMLMediaElement.h:
+
 2016-05-06  Anders Carlsson  <andersca@apple.com>
 
         Tidy up the LinkRelAttribute code
index d101039..1713286 100644 (file)
@@ -659,11 +659,6 @@ void HTMLMediaElement::resumeFromDocumentSuspension()
 }
 #endif
 
-bool HTMLMediaElement::hasCustomFocusLogic() const
-{
-    return true;
-}
-
 bool HTMLMediaElement::supportsFocus() const
 {
     if (document().isMediaDocument())
index d6293ba..b214731 100644 (file)
@@ -496,7 +496,6 @@ private:
     // FIXME: Shadow DOM spec says we should be able to create shadow root on audio and video elements
     bool canHaveUserAgentShadowRoot() const final { return true; }
 
-    bool hasCustomFocusLogic() const override;
     bool supportsFocus() const override;
     bool isMouseFocusable() const override;
     bool rendererIsNeeded(const RenderStyle&) override;