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: Windows 7 frame showing for frameless non-resizable windows #35365

Merged
merged 1 commit into from Oct 13, 2022

Conversation

rzhao271
Copy link
Contributor

Description of Change

Fixes #30024.
It turns out there's some Chromium code that was removing the WS_CAPTION style for frameless windows, just for Electron to add it back. Removing that if statement fixed the Windows 7 frame issue for both resizable and non-resizable frameless windows.

Downstream ref microsoft/vscode#158065.

CC @deepak1556

Checklist

Release Notes

Notes: Fixed an issue where frameless non-resizable windows showed a Windows 7 frame during startup.

@rzhao271 rzhao271 requested review from a team as code owners August 17, 2022 21:49
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Aug 17, 2022
Copy link
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with minor style change.

@deepak1556 deepak1556 requested a review from zcbenz August 18, 2022 00:36
@deepak1556 deepak1556 added semver/patch backwards-compatible bug fixes target/19-x-y target/21-x-y PR should also be added to the "21-x-y" branch. labels Aug 18, 2022
@rzhao271 rzhao271 force-pushed the rzhao271/frameless-caption-patch branch from 9fff3a3 to ec51e87 Compare August 18, 2022 02:35
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Aug 18, 2022
@rzhao271
Copy link
Contributor Author

This PR fails the minimum size test case on Windows. Will have to look into whether that test is relying on the WS_CAPTION style being taken off for a split second.

@rzhao271 rzhao271 force-pushed the rzhao271/frameless-caption-patch branch from ec51e87 to fb563c7 Compare August 19, 2022 21:04
Copy link
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: Understand why frame change notification is required.

@rzhao271
Copy link
Contributor Author

For more context, Deepak and I also got back a response from a SWE on the Windows UI team.

The brief summary is that because Chromium and Electron were toggling on and off the WS_CAPTION or WS_THICKFRAME Window styles, WM_DWMNCRENDERINGCHANGED messages were being queued up. Those messages decide whether DWM (the newer compositor) or uxtheme (the older painter) worked on the non-client area. When the window showed, the WM_DWMNCRENDERINGCHANGED:false message caused DWM to delegate the task of drawing the frame to uxtheme, which drew the frame in an older style. Then, when the real application came up, it just painted over that frame.

The SWE's response aligns with my investigations where I found out that playing around with WS_THICKFRAME or WS_CAPTION can prevent the WM_DWMNCRENDERINGCHANGED:false message from being sent.

This PR also aligns with their recommendation of making sure the windows styles are set properly in the first place. In particular, it considers how Electron sets the WS_CAPTION style, and prevents that style from being unset and reset in some cases.

@rzhao271 rzhao271 requested review from deepak1556 and removed request for zcbenz August 31, 2022 21:43
@rzhao271 rzhao271 force-pushed the rzhao271/frameless-caption-patch branch 2 times, most recently from d50aa00 to eb04728 Compare September 9, 2022 20:18
@rzhao271 rzhao271 requested review from codebytere and deepak1556 and removed request for deepak1556 and codebytere September 13, 2022 00:07
@jkleinsc jkleinsc added the target/22-x-y PR should also be added to the "22-x-y" branch. label Sep 28, 2022
@rzhao271 rzhao271 force-pushed the rzhao271/frameless-caption-patch branch from eb04728 to 8b112ff Compare September 29, 2022 05:40
@rzhao271 rzhao271 requested a review from a team as a code owner September 29, 2022 05:40
@rzhao271 rzhao271 requested review from deepak1556 and removed request for codebytere September 29, 2022 05:40
@rzhao271 rzhao271 force-pushed the rzhao271/frameless-caption-patch branch from 8b112ff to a13dcad Compare October 4, 2022 18:58
@jkleinsc jkleinsc merged commit ce138fe into electron:main Oct 13, 2022
@release-clerk
Copy link

release-clerk bot commented Oct 13, 2022

Release Notes Persisted

Fixed an issue where frameless non-resizable windows showed a Windows 7 frame during startup.

@trop
Copy link
Contributor

trop bot commented Oct 13, 2022

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

@trop
Copy link
Contributor

trop bot commented Oct 13, 2022

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

@trop trop bot added needs-manual-bp/21-x-y and removed target/21-x-y PR should also be added to the "21-x-y" branch. labels Oct 13, 2022
@trop
Copy link
Contributor

trop bot commented Oct 13, 2022

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

@trop trop bot added in-flight/22-x-y and removed target/22-x-y PR should also be added to the "22-x-y" branch. labels Oct 13, 2022
@trop trop bot added merged/22-x-y PR was merged to the "22-x-y" branch. and removed in-flight/22-x-y labels Oct 13, 2022
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged/22-x-y PR was merged to the "22-x-y" branch. needs-manual-bp/21-x-y semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Windows 7 frame is shown on load with frame: false and resizable: false
5 participants