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: transparent window max/unmax event firing #32643

Merged
merged 3 commits into from Jan 31, 2022

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Jan 27, 2022

Description of Change

Closes #32633.

This PR fixes an issue with transparent windows failing to fire the maximize and unmaximize events on Windows.

We've made a few fixes to transparent window size events on Windows of late, including #28207 and #26586. As of now, we set transparent window to maximized by calling SetBounds, which means that NativeWindowViews::HandleSizeEvent is not triggered with SIZE_MAXIMIZED as it should be. This also presents an issue with unmaximization for similar reasons, and caused the maximize and unmaximize events not to fire on Windows. We can fix this issue by calling NotifyWindowMaximize() and NotifyWindowUnmaximize() in the transparency-specific logic blocks of Maximize() and Unmaximize().

cc @dsanders11

Checklist

Release Notes

Notes: Fixed an issue with transparent windows failing to fire the maximize and unmaximize events on Windows.

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/15-x-y labels Jan 27, 2022
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jan 27, 2022
@dsanders11
Copy link
Member

Thanks for jumping on fixing this. 😄

Couple small things:

  • I think the "Notes" for this PR got garbled
  • Can you update the "can check transparent window maximization" test to use the max/unmax events instead of 'resize'? My understanding is that was a workaround to these events not firing on Windows, so now that they do, that workaround can be removed.
  • Do the added tests need to be restricted to Windows? These events should fire for transparent windows across all platforms, right? I think the tests should run on all platforms to prevent any potential regressions.

@dsanders11
Copy link
Member

The Linux CI build is having issues, but I've checked out and built this branch on Ubuntu 20.04 and confirmed the new tests work there (with the changes from #32575 to re-enable event tests), after one small change I've suggested above.

Co-authored-by: David Sanders <dsanders11@ucsbalum.com>
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jan 28, 2022
@codebytere codebytere merged commit f5dc2a6 into main Jan 31, 2022
@codebytere codebytere deleted the fix-maximize-unmaximize-firing branch January 31, 2022 21:10
@release-clerk
Copy link

release-clerk bot commented Jan 31, 2022

Release Notes Persisted

Fixed an issue with transparent windows failing to fire the maximize and unmaximize events on Windows.

@trop
Copy link
Contributor

trop bot commented Jan 31, 2022

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

@trop
Copy link
Contributor

trop bot commented Jan 31, 2022

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

@trop
Copy link
Contributor

trop bot commented Jan 31, 2022

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

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]: On Windows, Transparent Windows Don't Fire "maximize"/"unmaximize" Events
4 participants