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: implement systemPreferences.getMediaAccessStatus() on Windows #24275
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks solid
return ConvertDeviceAccessStatus( | ||
DeviceAccessStatus::DeviceAccessStatus_Allowed); | ||
} else { | ||
args->ThrowError("Invalid media type"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not originating from this PR but when we have a unknown
state in the docs, why do we want to throw an error ? seem to do it for macOS as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deepak1556 The return type can have unknown
, the media_type
argument can't be unknown
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For an unknown media type why can't the return type by unknown, the current system calls both on windows and macOS never return an unknown state, the value is being unused. I wouldn't mind if this state wasn't documented but it is :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to bring this up with api-WG
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can address it in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
d058966
to
8c471b4
Compare
@@ -426,6 +426,9 @@ This user consent was not required on macOS 10.13 High Sierra or lower so this m | |||
macOS 10.14 Mojave or higher requires consent for `microphone` and `camera` access. | |||
macOS 10.15 Catalina or higher requires consent for `screen` access. | |||
|
|||
Windows 10 has a global setting controlling `microphone` and `camera` access for all win32 applications. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is specific for win32 applications should we safety check if for APPX / Window Store builds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The system API returns the global value, it should work fine. Electron Window Store app is still Win32.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from an impl perspective, & i agree we might want to discuss the unknown
type but don't think it should necessarily block this PR :)
Release Notes Persisted
|
/trop run backport-to 8-x-y |
The backport process for this PR has been manually initiated - |
I was unable to backport this PR to "8-x-y" cleanly; |
/trop run backport-to 9-x-y |
The backport process for this PR has been manually initiated - |
I was unable to backport this PR to "9-x-y" cleanly; |
/trop run backport-to 10-x-y |
The backport process for this PR has been manually initiated - |
I have automatically backported this PR to "10-x-y", please check out #24311 |
Description of Change
Implement systemPreferences.getMediaAccessStatus() on Windows
Checklist
npm test
passesRelease Notes
Notes: Implemented
systemPreferences.getMediaAccessStatus()
on Windows.