-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add userContext field to WebDriver BiDi's setPermission #438
Conversation
dc5b1e8
to
d00fab7
Compare
User contexts in WebDriver BiDi represents the same concepts as the user agents in the infra spec: https://w3c.github.io/webdriver-bidi/#user-contexts According to the infra definition a user agent can be a user profile or a private browsing window. This PR adds an ability for the automation client to configure which "user agent" should the permissions apply too.
d00fab7
to
4934712
Compare
@miketaylr could you please take a look? Does the definition like this make sense from the perspective of the permissions spec? It looks like until now the spec didn't deal with multiple user agents at once. |
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.
(apologies for the delay here!)
@@ -1459,6 +1461,9 @@ <h6 id="webdriver-bidi-command-permissions-setPermission"> | |||
</li> | |||
<li>Let |state| be the value of the `state` field of |command parameters|. | |||
</li> | |||
<li>Let |user context id| be the value of the `userContext` field of |command |
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.
Trying to understand what a "user context" looks like... in https://w3c.github.io/webdriver-bidi/#user-context, it seems like a data structure that has an ID, has a storage partition, and has a collection of traversables? Is that right? Or is it strictly just a unique identifier and the rest happens by magic?
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 user context roughly corresponds to the user agent of the infra spec definition (except that the WebDriver can identify them). User context IDs are unique, except for the default user context (identified as "default"). In implementations, a user context is an incognito window in Chrome or a container in Firefox.
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.
@miketaylr is there anything else I could try to clarify?
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.
@OrKoN apologies, was OOO last week. Thanks for the clarification - makes sense. Did you intend to close this 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.
Sorry, closing was not intended.
This PR adds `goog:` prefixed user context to allow use cases required by Puppeteer. Once permissions spec issue is resolved w3c/permissions#438 we can remove the prefix.
Ah, sorry, it got auto-closed since I moved on with the vendor-prefixed implementation for now. |
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.
This LGTM, mind taking a look @marcoscaceres?
@OrKoN - do we have interest from another vendor to implement this?
I have not received any feedback from other vendors on this particular change (except for the general implementation interest for WebDriver BiDi/permissions). |
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.
This seems fine.
|
@OrKoN mind rebasing? Then we can merge. |
@miketaylr @marcoscaceres thanks for reviewing, I have re-based. |
SHA: 645867d Reason: push, by miketaylr Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
User contexts in WebDriver BiDi represents the same concepts as the user agents in the infra spec: https://w3c.github.io/webdriver-bidi/#user-contexts According to the infra definition a user agent can be a user profile or a private browsing window.
This PR adds an ability for the automation client to configure which "user agent" should the permissions apply too.
closes #439
The following tasks have been completed:
Implementation commitment:
Preview | Diff