Skip to content

Commit

Permalink
Fullscreen video size/position is incorrect on YouTube when minimumEf…
Browse files Browse the repository at this point in the history
…fectiveDeviceWidth is set

https://bugs.webkit.org/show_bug.cgi?id=255556
rdar://101557743

Reviewed by Tim Horton.

Entering fullscreen on youtube.com for a `WKWebView` with a non-zero
`minimumEffectiveDeviceWidth` currently results in portions of the video
becoming clipped, the top of the video being incorrectly offset from the top of
the window.

In fullscreen, YouTube explicitly sets the width/height of the video element to
the `clientWidth`/`clientHeight` of the fullscreen element, inside the
`fullscreenchange` event. The fullscreen element is a wrapper `<div>` containing
the video element, with width/height: 100%.

Currently, the `WKWebView`'s frame is adjusted prior to dispatching the
`fullscreenchange` event, but the `minimumEffectiveDeviceWidth` is not. This
results in the viewport state continuing to incorporate the
`minimumEffectiveDeviceWidth`, which may be larger than the width of the
fullscreen window. The larger size also results in an offset from the top of the
window, as the video element has `object-fit: contain`. The video is smaller
than its element's size, and is vertically centered relative to the larger size.

To fix, ensure `minimumEffectiveDeviceWidth` is unset prior to dispatching the
`fullscreenchange` event. However, simply updating the value in the UI process
prior to dispatching `willEnterFullscreen` is not enough, as the visible content
rect update is asynchronous in both the UI and web processes. This means that
there is no guarantee that the viewport state in the web process will be correct
at the time of `fullscreenchange` event dispatch. To fix the secondary issue,
introduce `-[WKWebView _doAfterNextVisibleContentRectAndPresentationUpdate]` to
ensure the correct viewport state is reflected in the web process prior to
dispatching the `fullscreenchange` event.

* Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _doAfterNextVisibleContentRectAndPresentationUpdate:]):
* Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h:

Introduce `_doAfterNextVisibleContentRectAndPresentationUpdate`, used to ensure
ensure that the next visible content rect update is accurately reflected in
the web process, prior to performing the update block.

* Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:
(-[WKFullScreenWindowController enterFullScreen:]):

Unset `minimumEffectiveDeviceWidth` prior to dispatching the `fullscreenchange`
event, so that the viewport state accurately reflects the size of the fullscreen
window.

The original value is already stored `WKWebViewState`, and is restored on
fullscreen exit.

Canonical link: https://commits.webkit.org/263082@main
  • Loading branch information
pxlcoder committed Apr 18, 2023
1 parent 2a0a146 commit b42eaa6
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 4 deletions.
7 changes: 7 additions & 0 deletions Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Expand Up @@ -1639,6 +1639,13 @@ - (void)_internalDoAfterNextPresentationUpdate:(void (^)(void))updateBlock witho
});
}

- (void)_doAfterNextVisibleContentRectAndPresentationUpdate:(void (^)(void))updateBlock
{
[self _doAfterNextVisibleContentRectUpdate:makeBlockPtr([strongSelf = retainPtr(self), updateBlock = makeBlockPtr(updateBlock)] {
[strongSelf _doAfterNextPresentationUpdate:updateBlock.get()];
}).get()];
}

- (void)_recalculateViewportSizesWithMinimumViewportInset:(CocoaEdgeInsets)minimumViewportInset maximumViewportInset:(CocoaEdgeInsets)maximumViewportInset throwOnInvalidInput:(BOOL)throwOnInvalidInput
{
auto frame = WebCore::FloatSize(self.frame.size);
Expand Down
2 changes: 2 additions & 0 deletions Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h
Expand Up @@ -336,6 +336,8 @@ struct PerWebProcessState {

- (void)_internalDoAfterNextPresentationUpdate:(void (^)(void))updateBlock withoutWaitingForPainting:(BOOL)withoutWaitingForPainting withoutWaitingForAnimatedResize:(BOOL)withoutWaitingForAnimatedResize;

- (void)_doAfterNextVisibleContentRectAndPresentationUpdate:(void (^)(void))updateBlock;

- (void)_recalculateViewportSizesWithMinimumViewportInset:(CocoaEdgeInsets)minimumViewportInset maximumViewportInset:(CocoaEdgeInsets)maximumViewportInset throwOnInvalidInput:(BOOL)throwOnInvalidInput;

- (void)_showSafeBrowsingWarning:(const WebKit::SafeBrowsingWarning&)warning completionHandler:(CompletionHandler<void(std::variant<WebKit::ContinueUnsafeLoad, URL>&&)>&&)completionHandler;
Expand Down
Expand Up @@ -649,6 +649,7 @@ - (void)enterFullScreen:(CGSize)videoDimensions

[webView setAutoresizingMask:(UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight)];
[webView setFrame:[_window bounds]];
[webView _setMinimumEffectiveDeviceWidth:0];
[webView _overrideLayoutParametersWithMinimumLayoutSize:[_window bounds].size maximumUnobscuredSizeOverride:[_window bounds].size];
[_window insertSubview:webView.get() atIndex:0];
[webView setNeedsLayout];
Expand All @@ -671,13 +672,21 @@ - (void)enterFullScreen:(CGSize)videoDimensions
return;
}

if (auto* manager = [protectedSelf _manager]) {
manager->willEnterFullScreen();
if (![protectedSelf _manager]) {
ASSERT_NOT_REACHED();
[self _exitFullscreenImmediately];
return;
}

ASSERT_NOT_REACHED();
[self _exitFullscreenImmediately];
[self._webView _doAfterNextVisibleContentRectAndPresentationUpdate:makeBlockPtr([protectedSelf] {
if (auto* manager = [protectedSelf _manager]) {
manager->willEnterFullScreen();
return;
}

ASSERT_NOT_REACHED();
[protectedSelf _exitFullscreenImmediately];
}).get()];
});

[CATransaction commit];
Expand Down

0 comments on commit b42eaa6

Please sign in to comment.