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: {exit|enter}-html-fullscreen emitted after esc in webview #30537

Merged
merged 1 commit into from Aug 17, 2021

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Aug 16, 2021

Description of Change

Closes #30509.

Fixes an issue present in webView where the leave-html-full-screen event is not emitted if the user exits fullscreen with esc instead of by clicking into the webView.

This was happening because when the user exits by clicking into the webview, we call into ExitFullScreenModeFromTab on the webcontents of the webview - meaning the event is emitted correctly to the webview. When we do so from esc, however, this happens from the wrong webContents and so then go and loop through each child webview to ensure they all exit properly with api_web_contents->SetHtmlApiFullscreen(false);. Since the emit happens in ExitFullScreenModeFromTab though, leave-html-full-screen is not emitted again.

This fixes the issue by centralizing calls to Emit("enter-html-full-screen") and owner_window_->NotifyWindow{Enter|Leave}HtmlFullScreen().

Checklist

Release Notes

Notes: Fixes an issue present in webView where the leave-html-full-screen event is not emitted if the user exits fullscreen with esc instead of by clicking into the webView.

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/13-x-y labels Aug 16, 2021
@codebytere codebytere requested a review from zcbenz August 16, 2021 06:50
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Aug 16, 2021
@codebytere
Copy link
Member Author

codebytere commented Aug 16, 2021

@zcbenz it looks like the Escape inputEvent does work on other platforms - it did so locally and in this test as well 🤔 would you like me to update the other spec which only runs on windows?

@zcbenz
Copy link
Member

zcbenz commented Aug 16, 2021

Hmm the other specs could not run on Windows for me, I probably did something wrong. I think you can try enabling them for Windows in another PR.

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Aug 17, 2021
@codebytere codebytere merged commit a9a90fa into main Aug 17, 2021
@codebytere codebytere deleted the fix-esc-fullscreen-webview branch August 17, 2021 07:03
@release-clerk
Copy link

release-clerk bot commented Aug 17, 2021

Release Notes Persisted

Fixes an issue present in webView where the leave-html-full-screen event is not emitted if the user exits fullscreen with esc instead of by clicking into the webView.

@trop
Copy link
Contributor

trop bot commented Aug 17, 2021

I have automatically backported this PR to "13-x-y", please check out #30560

@trop
Copy link
Contributor

trop bot commented Aug 17, 2021

I have automatically backported this PR to "14-x-y", please check out #30561

@trop
Copy link
Contributor

trop bot commented Aug 17, 2021

I have automatically backported this PR to "15-x-y", please check out #30562

@miniak
Copy link
Contributor

miniak commented Aug 24, 2021

/trop run backport-to 12-x-y

@trop
Copy link
Contributor

trop bot commented Aug 24, 2021

The backport process for this PR has been manually initiated - sending your PR to 12-x-y!

@trop
Copy link
Contributor

trop bot commented Aug 24, 2021

I have automatically backported this PR to "12-x-y", please check out #30669

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
Development

Successfully merging this pull request may close these issues.

[Bug]: Webview not emitting leave-html-full-screen when using ESC key to exit webview fullscreen
3 participants