Remove modification of document.requestStorageAccess algorithm
#34
Conversation
index.bs
Outdated
| @@ -127,7 +127,7 @@ When invoked on {{Document}} |doc| with {{USVString}} |requestedOrigin|, the <df | |||
| 1. Return. | |||
| 1. Let |permissionState| be the result of [=requesting permission to use=] "<a permission><code>top-level-storage-access</code></a>" with |descriptor|. | |||
|
|
|||
| NOTE: Note that when requesting permissions and deciding whether to show a prompt, user agents apply implementation-defined behavior to shape the end user experience. Particularly for `top-level-storage-access`, user agents are known to apply custom rules that will grant or deny a permission without showing a prompt. | |||
| NOTE: Note that when requesting permissions and deciding whether to show a prompt, user agents apply implementation-defined behavior to shape the end user experience. Particularly for `top-level-storage-access`, user agents are known to apply custom rules that will grant or deny a permission without showing a prompt. User agents may also treat the `top-level-storage-access` permission as a "superset" of other permissions, and may grant those other permissions when `top-level-storage-access` permission is granted. | |||
There was a problem hiding this comment.
A few thoughts:
- This doesn't seem placed right. In this step we check for permission for
top-level-storage-access, we're not trying to determine other permissions. - It's generally discouraged (sometimes disallowed) to use RFC 2119 terms ("may", in this case) in informative notes.
- I can't really wrap my head around the "superset" concept for the relationship between these two permissions, but maybe that's just me :)
Would it make sense to just add another normative step here with a MAY, something like:
1. The user agent may (or should?) grant the "storage-access" permission for origin and doc's origin.
There was a problem hiding this comment.
Thinking about this more maybe this should be just a regular step, it's not clear why it has to be may/should :)
There was a problem hiding this comment.
These steps are written in the context of a "client" that is interacting with permissions via request permission to use, so I don't think we can just say "if permissionState is "granted", grant permission for storage-access", because we don't have access to those internals here (IIUC). I think we would have to patch the request permission to use algorithm instead, such that it grants both storage-access and top-level-storage-access whenever top-level-storage-access is requested and granted.
That was the reason I put this here as a note, rather than as a regular step. And this note already talks about user-agent-defined behavior in request permission to use, which is why I expected that "may" would be fine. WDYT?
There was a problem hiding this comment.
I defer to @johannhof on this, but for what my opinion is worth:
dual writeorcheck both permissionsaccomplishes the same thing in the rSA steps. I'm aware there are other side effects with permissions.query, etc., but we already anticipated wanting those side effects and could specify them separately vs mandating dual-write.- if I'm not mistaken, switching to
maywould mean we'd need to delete WPTs that verify the autogrant of rSA, and that's a pretty important part of what this API does. It seems like it should be an explicit requirement, not implementation defined behavior (implementation defined behavior seems fine when deciding to grant the permission, since developers would already need to contend with humans being shown a prompt, but less fine in specifying what the permission does once it's granted).
Basically, it makes sense to me to dual-write, but I worry that having it be implementation-defined behavior is not what we want (but again, I defer to Johann).
There was a problem hiding this comment.
PTAL - I've updated the PR to still mandate the top-level-storage-access/storage-access dual write, but do so by properly integrating into the Permissions spec.
This updates the spec to match Chrome's actual implementation, as of https://crrev.com/c/4301415. Closes #18.