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

feat: expose system preferences to utility process #42203

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

georgexu99
Copy link
Contributor

@georgexu99 georgexu99 commented May 16, 2024

Description of Change

This PR enables systemPreferences to be used within a utility process fork and not only the main process.

Motive

systemPreferences.getMediaAccessStatus only returns accurate permissions-states at the start of a user's session, but may not reflect permission changes made to the app until the app full quits/restarts. This can lead to silent failures when we attempt to access a device when the permissions have been revoked mid-session.

Being able to call this function from within the Utility Module will allow for accurate permission state retrieval.

Testing

[x] added test
[x] manually verified change with gist

Release Notes

Notes: expose systemPreferences to utilityProcess

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label May 16, 2024
@georgexu99 georgexu99 marked this pull request as draft May 16, 2024 01:21
@georgexu99 georgexu99 changed the title chore: expose system preferences to utility process feat: expose system preferences to utility process May 17, 2024
@@ -1,4 +1,5 @@
// Utility side modules, please sort alphabetically.
export const utilityNodeModuleList: ElectronInternal.ModuleEntry[] = [
{ name: 'net', loader: () => require('./net') }
{ name: 'net', loader: () => require('./net') },
{ name: 'systemPreferences', loader: () => require('./system-preferences') }
Copy link
Member

Choose a reason for hiding this comment

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

Can we just map this to the @browser file? no need to copy paste the impl I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done- uses @electron/internal/browser/api/system-preferences now. However, the net module and some others all re-copy: any insights as to why we chose to do that

@georgexu99 georgexu99 marked this pull request as ready for review May 17, 2024 19:47
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants