Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bbc.co.uk: Video goes black with only audio playing when exiting PiP …
…mode

https://bugs.webkit.org/show_bug.cgi?id=257711
rdar://108304377

Reviewed by Jer Noble.

Only fire the fullscreen change event once we did enter fullscreen.
This gives time for the UI process to complete exiting PiP in the UI process.

The process for exiting PiP and go back into fullscreen is a complicated
(and unnecessary) dance between the content and the UI process.
didStopPictureInPicture (UI) -> requestRestoreFullScreen (CP) -> EnterFullScreen (UI)
 -> WillEnterFullscreen (CP) -> beganEnterFullScreen (UI) -> didEnterFullScreen (UI)
 -> didEnterFullScreen (CP)

Previously, the events `fullscreenchange` was fired in WillEnterFullscreen.
if a JS event listener was set, and attempted to exit PiP then (as BBC website is doing)
it would have left VideoFullscreenInterfaceAVKit in a broken state
once it received `beganEnterFullScreen` as it's not an handled chained of event.

By letting the UI process complete the exit of PiP back to fullscreen,
we can avoid the problem from occurring alltogether, and this is simply
done by firing the events and resolving the promise in didEnterFullScreen.

We limit this behaviour to BBC.com

This entire code is in serious need of a rewrite.

Manually tested under all possible interface scenarios:
- Entering PiP, Exiting PiP using BBC main player control.
- Entering PiP, Exiting PiP using PiP controller button.
- Entering fullscreen via BBC control, entering PiP using PiP button, exiting PiP using BBC main player control
- As above but using PiP controller button as last step.
- Going into Auto-Pip by swiping home and exiting PiP.
- Entering PiP, quitting PiP (X button)

* Source/WebCore/dom/FullscreenManager.cpp:
(WebCore::FullscreenManager::willEnterFullscreen):
(WebCore::FullscreenManager::didEnterFullscreen):
* Source/WebCore/page/Quirks.cpp:
(WebCore::Quirks::shouldDelayFullscreenEventWhenExitingPictureInPictureQuirk const):
* Source/WebCore/page/Quirks.h:

Canonical link: https://commits.webkit.org/264974@main
  • Loading branch information
jyavenard committed Jun 8, 2023
1 parent c38822c commit ba39716
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 1 deletion.
7 changes: 6 additions & 1 deletion Source/WebCore/dom/FullscreenManager.cpp
Expand Up @@ -43,6 +43,7 @@
#include "Page.h"
#include "PseudoClassChangeInvalidation.h"
#include "QualifiedName.h"
#include "Quirks.h"
#include "Settings.h"
#include <wtf/LoggerHelper.h>

Expand Down Expand Up @@ -517,13 +518,17 @@ bool FullscreenManager::willEnterFullscreen(Element& element)
if (is<HTMLIFrameElement>(element))
element.setIFrameFullscreenFlag(true);

notifyAboutFullscreenChangeOrError();
if (!document().quirks().shouldDelayFullscreenEventWhenExitingPictureInPictureQuirk())
notifyAboutFullscreenChangeOrError();

return true;
}

bool FullscreenManager::didEnterFullscreen()
{
if (document().quirks().shouldDelayFullscreenEventWhenExitingPictureInPictureQuirk())
notifyAboutFullscreenChangeOrError();

if (!m_fullscreenElement) {
ERROR_LOG(LOGIDENTIFIER, "No fullscreenElement; bailing");
return false;
Expand Down
19 changes: 19 additions & 0 deletions Source/WebCore/page/Quirks.cpp
Expand Up @@ -1404,6 +1404,25 @@ bool Quirks::shouldDisableEndFullscreenEventWhenEnteringPictureInPictureFromFull
#endif
}

bool Quirks::shouldDelayFullscreenEventWhenExitingPictureInPictureQuirk() const
{
#if ENABLE(VIDEO_PRESENTATION_MODE)
// This quirk delay the "webkitstartfullscreen" and "fullscreenchange" event when a video exits picture-in-picture
// to fullscreen.
if (!needsQuirks())
return false;

if (!m_shouldDelayFullscreenEventWhenExitingPictureInPictureQuirk) {
auto domain = RegistrableDomain(m_document->topDocument().url());
m_shouldDelayFullscreenEventWhenExitingPictureInPictureQuirk = domain == "bbc.com"_s;
}

return *m_shouldDelayFullscreenEventWhenExitingPictureInPictureQuirk;
#else
return false;
#endif
}

// teams.live.com rdar://88678598
// teams.microsoft.com rdar://90434296
bool Quirks::shouldAllowNavigationToCustomProtocolWithoutUserGesture(StringView protocol, const SecurityOriginData& requesterOrigin)
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/page/Quirks.h
Expand Up @@ -136,6 +136,7 @@ class Quirks {
WEBCORE_EXPORT bool blocksReturnToFullscreenFromPictureInPictureQuirk() const;
WEBCORE_EXPORT bool blocksEnteringStandardFullscreenFromPictureInPictureQuirk() const;
bool shouldDisableEndFullscreenEventWhenEnteringPictureInPictureFromFullscreenQuirk() const;
bool shouldDelayFullscreenEventWhenExitingPictureInPictureQuirk() const;

#if ENABLE(TRACKING_PREVENTION)
static bool isMicrosoftTeamsRedirectURL(const URL&);
Expand Down Expand Up @@ -208,6 +209,7 @@ class Quirks {
mutable std::optional<bool> m_blocksReturnToFullscreenFromPictureInPictureQuirk;
mutable std::optional<bool> m_blocksEnteringStandardFullscreenFromPictureInPictureQuirk;
mutable std::optional<bool> m_shouldDisableEndFullscreenEventWhenEnteringPictureInPictureFromFullscreenQuirk;
mutable std::optional<bool> m_shouldDelayFullscreenEventWhenExitingPictureInPictureQuirk;
#if PLATFORM(IOS) || PLATFORM(VISION)
mutable std::optional<bool> m_allowLayeredFullscreenVideos;
#endif
Expand Down

0 comments on commit ba39716

Please sign in to comment.