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: emit an event when accessing restricted path in File System Access API #42561

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

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Jun 18, 2024

Description of Change

Closes #42459

More closely match upstream spec behavior when File System Access API attempts to open a file or directory in a blocked path. Now, a file-system-access-restricted event is emitted on session when a user attempts to access a restricted path:

{
  origin: 'https://www.buzzfeed.com/',
  isDirectory: true,
  path: '/Users/codebytere/Downloads'
}

Developers can then decide how to proceed.

Checklist

Release Notes

Notes: Aligned failure pathway in File System Access API with upstream when attempting to open a file or directory in a blocked path.

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/30-x-y PR should also be added to the "30-x-y" branch. target/31-x-y PR should also be added to the "31-x-y" branch. target/32-x-y PR should also be added to the "32-x-y" branch. labels Jun 18, 2024
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jun 18, 2024
@codebytere codebytere force-pushed the dialog-web-api branch 3 times, most recently from 1f08bd4 to 297c490 Compare June 18, 2024 10:00
Copy link
Member

@MarshallOfSound MarshallOfSound left a comment

Choose a reason for hiding this comment

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

Electron has historically never contained user-facing UI. It introduces problems around messaging, UI style, translations, etc.

Is this not something that apps can understand and implement themselves?

@codebytere
Copy link
Member Author

codebytere commented Jun 18, 2024

@MarshallOfSound we have for other web platform APIs in specific circumstances, but - at the moment we log-only, which i don't think is a good experience either, for reasons in part outlined by the issue author. We could potentially emit an event?

@jkleinsc
Copy link
Contributor

@codebytere we emit events for device choosers from Session (eg https://www.electronjs.org/docs/latest/api/session#event-select-usb-device) when upstream would should a dialog.

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.

We should emit an event and let developers handle this much like we do for the device choosers.

@codebytere
Copy link
Member Author

@jkleinsc ah good shout - i'll do that!

@codebytere codebytere changed the title fix: show a dialog when accessing restricted path in File System Access API fix: emit an event when accessing restricted path in File System Access API Jun 19, 2024
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jun 19, 2024
@marcello3d
Copy link

would there be a way to allow the restricted directory to be used? because currently this doesn't let you choose ~/Downloads on macOS if we want our web app to save multiple files in there

@jkleinsc jkleinsc added semver/minor backwards-compatible functionality and removed semver/patch backwards-compatible bug fixes labels Jun 20, 2024
@electron-cation electron-cation bot added api-review/requested 🗳 new-pr 🌱 PR opened in the last 24 hours labels Jun 20, 2024
@codebytere
Copy link
Member Author

@marcello3d reworked it a bit - a69e079 should enable that!

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jun 25, 2024
@codebytere codebytere changed the title fix: emit an event when accessing restricted path in File System Access API feat: emit an event when accessing restricted path in File System Access API Jun 26, 2024
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.

Docs need some clarification.

* `isDirectory` boolean - Whether or not the path is a directory.
* `path` string - The blocked path attempting to be accessed.
* `callback` Function
* `action` string - Can be one of `block`, `tryAgain`, or `allow`.
Copy link
Contributor

Choose a reason for hiding this comment

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

The docs should explain what the different actions mean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review/requested 🗳 semver/minor backwards-compatible functionality target/30-x-y PR should also be added to the "30-x-y" branch. target/31-x-y PR should also be added to the "31-x-y" branch. target/32-x-y PR should also be added to the "32-x-y" branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: showDirectoryPicker returns AbortError with no alternative when a blocklisted folder is selected
4 participants