Bug 238462 - [macOS] Muted video is sometimes paused when entering fullscreen
Summary: [macOS] Muted video is sometimes paused when entering fullscreen
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-03-28 11:26 PDT by Eric Carlson
Modified: 2022-03-29 11:32 PDT (History)
13 users (show)

See Also:


Attachments
Patch (15.64 KB, patch)
2022-03-28 11:37 PDT, Eric Carlson
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (15.70 KB, patch)
2022-03-28 12:05 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch for landing (15.12 KB, patch)
2022-03-28 15:54 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2022-03-28 11:26:23 PDT
Muted video is sometimes paused when entering fullscreen
Comment 1 Eric Carlson 2022-03-28 11:27:09 PDT
rdar://89104216
Comment 2 Eric Carlson 2022-03-28 11:37:26 PDT
Created attachment 455933 [details]
Patch
Comment 3 Eric Carlson 2022-03-28 12:05:01 PDT
Created attachment 455937 [details]
Patch
Comment 4 Jer Noble 2022-03-28 14:07:56 PDT
Comment on attachment 455937 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:6058
> +#if ENABLE(FULLSCREEN_API)
> +    auto& fullscreenManager = document().fullscreenManager();
> +    if (isVideo() && fullscreenManager.isFullscreen() && fullscreenManager.currentFullscreenElement())
> +        return false;
> +#endif
> +
> +    if (m_videoFullscreenMode != VideoFullscreenModeNone)
> +        return false;
> +
> +    return document().hidden();

I think this is incorrect long-term, insofar as element fullscreen on a background space (on macOS) shouldn't always be "visible", but we should file a bug to follow up later.

> Source/WebCore/html/HTMLMediaElement.cpp:8344
> +        queueTaskKeepingObjectAlive(*this, TaskSource::MediaElement, [this] {
> +            if (isContextStopped())
> +                return;
> +            mediaSession().isVisibleInViewportChanged();
> +            updateShouldAutoplay();
> +            schedulePlaybackControlsManagerUpdate();
> +        });

Is this necessary?
Comment 5 Eric Carlson 2022-03-28 15:48:34 PDT
Comment on attachment 455937 [details]
Patch

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

>> Source/WebCore/html/HTMLMediaElement.cpp:6058
>> +    return document().hidden();
> 
> I think this is incorrect long-term, insofar as element fullscreen on a background space (on macOS) shouldn't always be "visible", but we should file a bug to follow up later.

I filed bug 238472

>> Source/WebCore/html/HTMLMediaElement.cpp:8344
>> +        });
> 
> Is this necessary?

Oops!
Comment 6 Eric Carlson 2022-03-28 15:54:37 PDT
Created attachment 455964 [details]
Patch for landing
Comment 7 Eric Carlson 2022-03-29 10:17:52 PDT
Comment on attachment 455964 [details]
Patch for landing

The failing tests don't seem to be related.
Comment 8 EWS 2022-03-29 11:32:04 PDT
Committed r292049 (248986@main): <https://commits.webkit.org/248986@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455964 [details].