[Mac] PiP placeholder should remain visible when 'controls' attribute…

… is removed https://bugs.webkit.org/show_bug.cgi?id=159158 <rdar://problem/26727435> Reviewed by Jer Noble. Source/WebCore: No new tests, existing test updated. * Modules/mediacontrols/mediaControlsApple.js: (Controller.prototype.shouldHaveControls): Always return true when in PiP or AirPlay mode. LayoutTests: * media/controls/picture-in-picture-expected.txt: Updated. * media/controls/picture-in-picture.html: Ditto. Canonical link: https://commits.webkit.org/177260@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@202509 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jun 27, 2016 · c54ba74 · c54ba74
1 parent ffaf2e9
commit c54ba74
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 7 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2016-06-27  Eric Carlson  <eric.carlson@apple.com>
+
+        [Mac] PiP placeholder should remain visible when 'controls' attribute is removed
+        https://bugs.webkit.org/show_bug.cgi?id=159158
+        <rdar://problem/26727435>
+
+        Reviewed by Jer Noble.
+
+        * media/controls/picture-in-picture-expected.txt: Updated.
+        * media/controls/picture-in-picture.html: Ditto.
+
 2016-06-27  Antoine Quint  <graouts@apple.com>
 
         [iOS] Media controls are too cramped with small video

diff --git a/LayoutTests/media/controls/picture-in-picture-expected.txt b/LayoutTests/media/controls/picture-in-picture-expected.txt
@@ -23,5 +23,11 @@ Test for the pip placeholder visibility in pip mode
 PASS: Should be in pip mode
 PASS: Inline placeholder should be visible at this point
 
+Test for the pip placeholder visibility in pip mode without a 'controls' attribute
+
+PASS: Should still be in pip mode
+PASS: No controls attribute
+PASS: Inline placeholder should still be visible
+
 Testing finished.
 
diff --git a/LayoutTests/media/controls/picture-in-picture.html b/LayoutTests/media/controls/picture-in-picture.html
@@ -7,20 +7,20 @@
             if (window.internals)
                 window.internals.settings.setAllowsPictureInPictureMediaPlayback(true);
 
-            var tester = new ControlsTest("non-existent-media-file", "error")
+            const tester = new ControlsTest("non-existent-media-file", "error")
                 .whenReady(runTestsWithoutVideo)
                 .start();
 
             function runTestsWithoutVideo()
             {
-                var currentState = tester.currentState;
+                const currentState = tester.currentState;
                 tester.test("We are using the apple idiom")
                     .value(currentState.idiom)
                     .isEqualTo("apple");
 
                 tester.startNewSection("Test the picture-in-picture button without video", true);
 
-                var stateForPictureInPictureButton = tester.stateForControlsElement("Picture-in-picture Button");
+                const stateForPictureInPictureButton = tester.stateForControlsElement("Picture-in-picture Button");
                 tester.test("Picture-in-picture button should not be visible if there's no video")
                     .value(stateForPictureInPictureButton.className)
                     .contains("hidden");
@@ -34,7 +34,7 @@
             {
                 tester.startNewSection("Test the picture-in-picture button with valid video");
 
-                var stateForPictureInPictureButton = tester.stateForControlsElement("Picture-in-picture Button", true);
+                const stateForPictureInPictureButton = tester.stateForControlsElement("Picture-in-picture Button", true);
                 tester.test("Picture-in-picture button should be visible if there's video")
                     .value(stateForPictureInPictureButton.className)
                     .doesNotContain("hidden");
@@ -43,7 +43,7 @@
                     .value(stateForPictureInPictureButton.parentElement)
                     .isNotEqualTo(undefined);
 
-                var stateForPlaceholder = tester.stateForControlsElement("Inline playback placeholder", true);
+                const stateForPlaceholder = tester.stateForControlsElement("Inline playback placeholder", true);
                 tester.test("Inline placeholder should not be visible at this point")
                     .value(stateForPlaceholder.className)
                     .contains("hidden");
@@ -62,12 +62,36 @@
                     .value(tester.media.webkitPresentationMode)
                     .isEqualTo("picture-in-picture");
 
-                var stateForPlaceholder = tester.stateForControlsElement("Inline playback placeholder", true);
+                const stateForPlaceholder = tester.stateForControlsElement("Inline playback placeholder", true);
                 tester.test("Inline placeholder should be visible at this point")
                     .value(stateForPlaceholder.className)
                     .doesNotContain("hidden");
 
+                const controlsObserver = new MutationObserver(controlsDidChange);
+                controlsObserver.observe(tester.media, { attributes: true, attributeFilter: ['controls'] });
+
+                tester.media.removeAttribute('controls');
+            }
+
+            function controlsDidChange()
+            {
+                tester.startNewSection("Test for the pip placeholder visibility in pip mode without a 'controls' attribute");
+
+                tester.test("Should still be in pip mode")
+                    .value(tester.media.webkitPresentationMode)
+                    .isEqualTo("picture-in-picture");
+
+                tester.test("No controls attribute")
+                    .value(tester.media.hasAttribute('controls'))
+                    .isFalse();
+
+                const stateForPlaceholder = tester.stateForControlsElement("Inline playback placeholder", true);
+                tester.test("Inline placeholder should still be visible")
+                    .value(stateForPlaceholder.className)
+                    .doesNotContain("hidden");
+
                 tester.media.webkitSetPresentationMode("inline");
+
                 tester.end();
             }
         </script>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2016-06-27  Eric Carlson  <eric.carlson@apple.com>
+
+        [Mac] PiP placeholder should remain visible when 'controls' attribute is removed
+        https://bugs.webkit.org/show_bug.cgi?id=159158
+        <rdar://problem/26727435>
+
+        Reviewed by Jer Noble.
+
+        No new tests, existing test updated.
+
+        * Modules/mediacontrols/mediaControlsApple.js:
+        (Controller.prototype.shouldHaveControls): Always return true when in PiP or AirPlay mode.
+
 2016-06-27  Oliver Hunt  <oliver@apple.com>
 
         Update ATS WebContent exception for more robust framework information

diff --git a/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js b/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js
@@ -306,7 +306,10 @@ Controller.prototype = {
         if (!this.isAudio() && !this.host.allowsInlineMediaPlayback)
             return true;
 
-        return this.video.controls || this.isFullScreen();
+        if (this.isFullScreen() || this.presentationMode() === 'picture-in-picture' || this.currentPlaybackTargetIsWireless())
+            return true;
+
+        return this.video.controls;
     },
 
     setNeedsTimelineMetricsUpdate: function()