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: set macOS crypto keychain name earlier #34683

Merged
merged 2 commits into from Sep 23, 2022

Conversation

MarshallOfSound
Copy link
Member

Fixes #34614

Notes: Usage of safeStorage now consistently uses the correct service name on macOS regardless of timing with browser window construction

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jun 21, 2022
@MarshallOfSound MarshallOfSound added target/18-x-y semver/patch backwards-compatible bug fixes and removed new-pr 🌱 PR opened in the last 24 hours labels Jun 21, 2022
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jun 21, 2022
Copy link

@mayfield mayfield left a comment

Choose a reason for hiding this comment

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

This all looks correct to me.

@mayfield
Copy link

I'm sure it goes without saying, but one point of caution with this change is that anyone who depended on the previous behavior (however erroneous) will now get errors following this patch.

@RaisinTen
Copy link
Member

We should probably also add the test mentioned in #34614 (comment), to make sure that this never regresses in the future.

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jun 22, 2022
Copy link
Contributor

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

@MarshallOfSound MarshallOfSound dismissed jkleinsc’s stale review September 23, 2022 19:14

fixed the arm64 failure

@MarshallOfSound MarshallOfSound merged commit 324db14 into main Sep 23, 2022
@MarshallOfSound MarshallOfSound deleted the init-mac-keychain-early branch September 23, 2022 19:32
@release-clerk
Copy link

release-clerk bot commented Sep 23, 2022

Release Notes Persisted

Usage of safeStorage now consistently uses the correct service name on macOS regardless of timing with browser window construction

@MarshallOfSound MarshallOfSound added the target/21-x-y PR should also be added to the "21-x-y" branch. label Sep 23, 2022
@trop
Copy link
Contributor

trop bot commented Sep 23, 2022

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

@trop
Copy link
Contributor

trop bot commented Sep 23, 2022

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

@trop
Copy link
Contributor

trop bot commented Sep 23, 2022

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

khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
* fix: set macOS crypto keychain name earlier

* spec: ensure arm64 mac tests are cleaned up
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 target/21-x-y PR should also be added to the "21-x-y" branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: safeStorage use is invalid prior use of a BrowserWindow
7 participants