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: WCO crash on bad titlebarStyle #34140

Merged
merged 1 commit into from May 17, 2022
Merged

Conversation

codebytere
Copy link
Member

Description of Change

Closes #34137.

Fixes an issue where calling setTitlebarOverlay with an initially invalid titleBarStyle on Windows would result in a crash. Ideally, we'd throw in the constructor, but that causes a whole host of issues, so we instead no-op if we attempt to call setTitleBarStyle and the overlay was never correctly initialized.

Checklist

Release Notes

Notes: Fixes an issue where calling setTitlebarOverlay with an initially invalid titleBarStyle on Windows would result in a crash.

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/19-x-y labels May 9, 2022
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label May 9, 2022
@codebytere codebytere changed the title fix: WCO crash on bad titlebarStyle fix: WCO crash on bad titlebarStyle May 9, 2022
@codebytere
Copy link
Member Author

@jkleinsc it looks like the WOA failure is real - would you be able to potentially help verify on your local machine?

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label May 10, 2022
@MarshallOfSound
Copy link
Member

MarshallOfSound commented May 11, 2022

Looks like both (x64 + WOA) windows failures are legit @codebytere

@codebytere
Copy link
Member Author

codebytere commented May 12, 2022

I see two options:

  1. Try to fix the newly added functionality test as part of this PR (thereby expanding scope since this PR only fixes the crash and that test correctly passes)
  2. Remove the non-crash test and open a new PR to add it and fix the failure, since it's not actually testing the issue at hand.

Thoughts? @jkleinsc @MarshallOfSound

@jkleinsc
Copy link
Contributor

@codebytere I'd pick option 2.

@codebytere codebytere force-pushed the fix-wco-titlebarstyle-crash branch from 842ffdd to 5b7719d Compare May 13, 2022 07:37
@codebytere
Copy link
Member Author

@jkleinsc done!

@codebytere codebytere force-pushed the fix-wco-titlebarstyle-crash branch from 5b7719d to 0b94727 Compare May 17, 2022 07:38
@codebytere codebytere force-pushed the fix-wco-titlebarstyle-crash branch from 0b94727 to 3755fc8 Compare May 17, 2022 12:49
@jkleinsc jkleinsc merged commit 97c9451 into main May 17, 2022
@jkleinsc jkleinsc deleted the fix-wco-titlebarstyle-crash branch May 17, 2022 15:50
@release-clerk
Copy link

release-clerk bot commented May 17, 2022

Release Notes Persisted

Fixes an issue where calling setTitlebarOverlay with an initially invalid titleBarStyle on Windows would result in a crash.

@trop
Copy link
Contributor

trop bot commented May 17, 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 20, 2022

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

@miniak
Copy link
Contributor

miniak commented Jul 22, 2022

/trop run backport-to 18-x-y

@trop
Copy link
Contributor

trop bot commented Jul 22, 2022

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

@trop
Copy link
Contributor

trop bot commented Jul 22, 2022

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

khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
fix: WCO crash on bad titlebarStyle
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]: Crash on Windows when calling win.setTitleBarOverlay on a window that has titleBarStyle != 'hidden'
8 participants