Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to enter fullscreen in Counterstrike (http://game.play-cs.com) in Safari 16.3 #18883

Conversation

aestes
Copy link
Contributor

@aestes aestes commented Oct 10, 2023

7e8c765

Unable to enter fullscreen in Counterstrike (http://game.play-cs.com) in Safari 16.3
https://bugs.webkit.org/show_bug.cgi?id=251648
rdar://104984915

Reviewed by Jer Noble.

When entering fullscreen, WKFullScreenWindowController calls
WebPageProxy::setSuppressVisibilityUpdates to suppress visibility updates while the web view moves to
the new fullscreen window, since updating visibility is unnecessary when transitioning to fullscreen
and can even cause bugs (see the comment in -[WKFullScreenWindowController enterFullScreen:]). It
ends suppression in -beganEnterFullScreenWithInitialFrame:finalFrame: and schedules an activity
state update, but this occurs at a time when AppKit considers the fullscreen window to be occluded.
As a result a `visibilitychange` event is dispatched where `document.visibilityState == 'hidden'`,
and play-cs.com responds to this event by exiting fullscreen. The upshot is that the user cannot
stay in fullscreen as the page exits this mode as soon as the user enters it.

Resolved this by extending the period where visibility updates are suppressed to when
-finishedEnterFullScreenAnimation: is called. At this point AppKit no longer considers the
fullscreen window to be occluded, so WebPageProxy detects no change in activity state and does not
dispatch any `visibilitychange` events. This matches the behavior of Firefox and Chrome on macOS,
which also do not dispatch `visiblitychange` events when entering fullscreen.

Added an API test.

* Source/WebKit/UIProcess/mac/WKFullScreenWindowController.mm:
(-[WKFullScreenWindowController enterFullScreen:]):
(-[WKFullScreenWindowController beganEnterFullScreenWithInitialFrame:finalFrame:]):
(-[WKFullScreenWindowController finishedEnterFullScreenAnimation:]):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/FullscreenDelegate.html:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/FullscreenDelegate.mm:
(-[FullscreenDelegateMessageHandler userContentController:didReceiveScriptMessage:]):
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/269150@main

6891330

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style   πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings   πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
  πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ gtk-wk2
  πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@aestes aestes requested a review from cdumez as a code owner October 10, 2023 04:42
@aestes aestes self-assigned this Oct 10, 2023
@aestes aestes added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Oct 10, 2023
@aestes aestes force-pushed the eng/Unable-to-enter-fullscreen-in-Counterstrike-httpgame-play-cs-com-in-Safari-16-3 branch from cc1ce21 to 6891330 Compare October 10, 2023 15:39
@aestes aestes added the merge-queue Applied to send a pull request to merge-queue label Oct 10, 2023
… in Safari 16.3

https://bugs.webkit.org/show_bug.cgi?id=251648
rdar://104984915

Reviewed by Jer Noble.

When entering fullscreen, WKFullScreenWindowController calls
WebPageProxy::setSuppressVisibilityUpdates to suppress visibility updates while the web view moves to
the new fullscreen window, since updating visibility is unnecessary when transitioning to fullscreen
and can even cause bugs (see the comment in -[WKFullScreenWindowController enterFullScreen:]). It
ends suppression in -beganEnterFullScreenWithInitialFrame:finalFrame: and schedules an activity
state update, but this occurs at a time when AppKit considers the fullscreen window to be occluded.
As a result a `visibilitychange` event is dispatched where `document.visibilityState == 'hidden'`,
and play-cs.com responds to this event by exiting fullscreen. The upshot is that the user cannot
stay in fullscreen as the page exits this mode as soon as the user enters it.

Resolved this by extending the period where visibility updates are suppressed to when
-finishedEnterFullScreenAnimation: is called. At this point AppKit no longer considers the
fullscreen window to be occluded, so WebPageProxy detects no change in activity state and does not
dispatch any `visibilitychange` events. This matches the behavior of Firefox and Chrome on macOS,
which also do not dispatch `visiblitychange` events when entering fullscreen.

Added an API test.

* Source/WebKit/UIProcess/mac/WKFullScreenWindowController.mm:
(-[WKFullScreenWindowController enterFullScreen:]):
(-[WKFullScreenWindowController beganEnterFullScreenWithInitialFrame:finalFrame:]):
(-[WKFullScreenWindowController finishedEnterFullScreenAnimation:]):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/FullscreenDelegate.html:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/FullscreenDelegate.mm:
(-[FullscreenDelegateMessageHandler userContentController:didReceiveScriptMessage:]):
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/269150@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Unable-to-enter-fullscreen-in-Counterstrike-httpgame-play-cs-com-in-Safari-16-3 branch from 6891330 to 7e8c765 Compare October 10, 2023 17:11
@webkit-commit-queue
Copy link
Collaborator

Committed 269150@main (7e8c765): https://commits.webkit.org/269150@main

Reviewed commits have been landed. Closing PR #18883 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 7e8c765 into WebKit:main Oct 10, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 10, 2023
@aestes aestes deleted the eng/Unable-to-enter-fullscreen-in-Counterstrike-httpgame-play-cs-com-in-Safari-16-3 branch October 30, 2023 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Bugs Unclassified bugs are placed in this component until the correct component can be determined.
Projects
None yet
4 participants