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: potential crash when setting vibrancy #29677

Merged
merged 1 commit into from Jun 16, 2021
Merged

fix: potential crash when setting vibrancy #29677

merged 1 commit into from Jun 16, 2021

Conversation

codebytere
Copy link
Member

Description of Change

Closes #29653.

Fixes a potential crash when a value is passed in for the vibrancy type which is not valid either owing to operating system version or the string itself. This will now simply no-op.

Checklist

Release Notes

Notes: Fixes a potential crash when setting vibrancy on macOS.

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

Build error due to unused variable after the change:

../../electron/shell/browser/native_window_mac.mm -o obj/electron/electron_lib/native_window_mac.o
../../electron/shell/browser/native_window_mac.mm:1393:26: error: unused variable 'vibrancyType' [-Werror,-Wunused-variable]
NSVisualEffectMaterial vibrancyType;

On line 1363 (vibrancy_type_ = type;) the input value is saved off, and looks like it's checked if that's non-empty in NativeWindowMac::UpdateVibrancyRadii. Does that logic need to change? With this PR line 1363 will still save off the input to NativeWindowMac::SetVibrancy even if it is a no-op due to it being an unrecognized/unsupported value.

@codebytere codebytere force-pushed the fix-vibrancy-crash branch 2 times, most recently from bf27332 to 7605bc8 Compare June 14, 2021 20:14
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jun 15, 2021
@zcbenz zcbenz merged commit 2ae7e04 into main Jun 16, 2021
@zcbenz zcbenz deleted the fix-vibrancy-crash branch June 16, 2021 06:39
@release-clerk
Copy link

release-clerk bot commented Jun 16, 2021

Release Notes Persisted

Fixes a potential crash when setting vibrancy on macOS.

@trop
Copy link
Contributor

trop bot commented Jun 16, 2021

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

@trop
Copy link
Contributor

trop bot commented Jun 16, 2021

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

@trop
Copy link
Contributor

trop bot commented Jun 16, 2021

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

@trop
Copy link
Contributor

trop bot commented Aug 11, 2021

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

BlackHole1 pushed a commit to BlackHole1/electron that referenced this pull request Aug 30, 2021
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]: Application crashes on macOS < Catalina, related to Window Vibrancy
4 participants