From c54ba749fa891fd6be8547ae03e28a890fe04145 Mon Sep 17 00:00:00 2001 From: Eric Carlson Date: Mon, 27 Jun 2016 20:37:48 +0000 Subject: [PATCH] [Mac] PiP placeholder should remain visible when 'controls' attribute is removed https://bugs.webkit.org/show_bug.cgi?id=159158 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 --- LayoutTests/ChangeLog | 11 ++++++ .../controls/picture-in-picture-expected.txt | 6 ++++ .../media/controls/picture-in-picture.html | 36 +++++++++++++++---- Source/WebCore/ChangeLog | 13 +++++++ .../mediacontrols/mediaControlsApple.js | 5 ++- 5 files changed, 64 insertions(+), 7 deletions(-) diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index a9e2ef46d320..2bbb6471e06c 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,14 @@ +2016-06-27 Eric Carlson + + [Mac] PiP placeholder should remain visible when 'controls' attribute is removed + https://bugs.webkit.org/show_bug.cgi?id=159158 + + + Reviewed by Jer Noble. + + * media/controls/picture-in-picture-expected.txt: Updated. + * media/controls/picture-in-picture.html: Ditto. + 2016-06-27 Antoine Quint [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 index 3c85f45097f3..a073ff8735c1 100644 --- 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 index d881e7ecba64..ef8c7639a496 100644 --- 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(); } diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 6eab6c408fce..0a515cf4a61a 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,16 @@ +2016-06-27 Eric Carlson + + [Mac] PiP placeholder should remain visible when 'controls' attribute is removed + https://bugs.webkit.org/show_bug.cgi?id=159158 + + + 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 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 index 778760925e23..0b794a662a5e 100644 --- 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()