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

fix: esc not working on Windows during fullscreen #34317

Merged
merged 2 commits into from May 25, 2022

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented May 23, 2022

Description of Change

Closes #34301.

Cleans up fullscreen control logic cross-platform.

  1. We don't want to call fullscreen_controller()->EnterFullscreenModeForTab in WebContents::EnterFullscreenModeForTab, becuase there's a chance that the user will deny permission via session.setPermissionRequestHandler. Only call this in OnlyEnterFullscreenModeForTab after the user has allowed fullscreening.
  2. We also need to ensure that NotifyExclusiveTabAccessLost is called on all platforms in FullscreenController::ExitFullscreenModeInternal() and not just macOS, since Electron's native window impls report state change fairly instantly as well, and so pressing escape won't work on Linux or Windows to un-fullscreen in some circumstances without this change.

Checklist

Release Notes

Notes: Fixed an issue where pressing escape would not un-fullscreen on Windows or Linux in some circumstances.

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/18-x-y labels May 23, 2022
@codebytere codebytere requested review from a team as code owners May 23, 2022 11:07
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label May 23, 2022
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label May 24, 2022
@VerteDinde
Copy link
Member

WOA failure is unrelated

@VerteDinde VerteDinde merged commit 7bc4b91 into main May 25, 2022
@VerteDinde VerteDinde deleted the fix-fullscreen-escape-windows branch May 25, 2022 04:38
@release-clerk
Copy link

release-clerk bot commented May 25, 2022

Release Notes Persisted

Fixed an issue where pressing escape would not un-fullscreen on Windows or Linux in some circumstances.

@trop
Copy link
Contributor

trop bot commented May 25, 2022

I was unable to backport this PR to "18-x-y" cleanly;
you will need to perform this backport manually.

@trop trop bot removed the target/18-x-y label May 25, 2022
@trop
Copy link
Contributor

trop bot commented May 25, 2022

I was unable to backport this PR to "19-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented May 26, 2022

@codebytere has manually backported this PR to "19-x-y", please check out #34359

@trop
Copy link
Contributor

trop bot commented May 26, 2022

@codebytere has manually backported this PR to "18-x-y", please check out #34361

khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
* fix: esc not working on Windows during fullscreen

* chore: fix lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
3 participants