8000 fix: expose all serial devices to setDevicePermissionHandler by jkleinsc · Pull Request #32651 · electron/electron · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: expose all serial devices to setDevicePermissionHandler #32651

New issue 8000

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 31, 2022
Merged

fix: expose all serial devices to setDevicePermissionHandler #32651

merged 1 commit into from
Jan 31, 2022

Conversation

jkleinsc
Copy link
Member

Description of Change

Due to logic copied from upstream chromium, not all serial devices are passed along to the handler specified via session.setDevicePermissionHandler. Given that we are giving control to the developer as to which ports are being exposed, we can probably skip this check.

Checklist

Release Notes

Notes: fixed issue where not all serial devices were exposed to the handler specified by session.setDevicePermissionHandler

@jkleinsc jkleinsc added semver/patch backwards-compatible bug fixes target/15-x-y labels Jan 27, 2022
@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label Jan 27, 2022
nornagon
nornagon approved these changes Jan 28, 2022
View reviewed changes
Copy link
Contributor
@nornagon nornagon left a comment

Choose a reason for hiding this comment

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

seems reasonable. Can we remove CanStorePersistentEntry entirely then?

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Jan 28, 2022
@jkleinsc jkleinsc force-pushed the serial-skip-persistent-entry-check branch from aa93c7c to 4c46f5c Compare January 31, 2022 16:58
@jkleinsc
Copy link
Member Author

seems reasonable. Can we remove CanStorePersistentEntry entirely then?

It still is used in one place to handle devices that might be missing some information:

electron/shell/browser/serial/serial_chooser_context.cc

Line 64 in aa93c7c

if (!SerialChooserContext::CanStorePersistentEntry(port)) {

@jkleinsc jkleinsc merged commit 365933f into main Jan 31, 2022
@jkleinsc jkleinsc deleted the serial-skip-persistent-entry-check branch January 31, 2022 19:04
@release-clerk
Copy link
release-clerk bot commented Jan 31, 2022

Release Notes Persisted

fixed issue where not all serial devices were exposed to the handler specified by session.setDevicePermissionHandler

This was referenced Jan 31, 2022
Merged
Merged
Merged
@trop
Copy link
Contributor
trop bot commented Jan 31, 2022

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

@trop
Copy link
Contributor
trop bot commented Jan 31, 2022

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

@trop
Copy link
Contributor
trop bot commented Jan 31, 2022

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

@trop trop bot removed the in-flight/15-x-y label Feb 2, 2022
@commit-lint commit-lint bot mentioned this pull request Apr 1, 2022
Open
@jkleinsc jkleinsc mentioned this pull request May 19, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nornagon nornagon nornagon approved these changes

Assignees
No one assigned
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: serial.getPorts return empty even after setDevicePermissionHandler grants the permissions
2 participants
@jkleinsc @nornagon
0