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

Clarifying the behavior for consuming the user activation and delegated capability #37

Open
EdgarChen opened this issue Apr 8, 2024 · 4 comments

Comments

@EdgarChen
Copy link

For the APIs that would consume the user activation and delegated capability, Fullscreen and Payment, they have different behavior. If the global has both valid transient activation and delegated capability, the Payment API only consumes the transient activation, which means the Payment API is allowed to be called again because of the delegated capability is still valid; however Fullscreen API seems to consume both. Is this intentional?

@vinhill
Copy link

vinhill commented Jun 25, 2024

Currently for fullscreen, this spec consumes the delegated capability in step 10 when a promise was already returned in current step 7, so there could be race conditions. Maybe the monkey patch to fullscreen spec should be made more similar to the one for payment request spec?

@michaelwasserman
Copy link

Fixing any race by consuming signals before returning the promise is probably a good idea.

I think fullscreen was aiming for conservative usable security properties by consuming both signals. It might be okay to only consume one signal, in order to permit some other activation-consuming API call before (or after?) requesting fullscreen. I suggest thinking through use cases and security considerations before making the actual change.

@vinhill
Copy link

vinhill commented Jul 2, 2024

My impression is that when the monkey patch for Fullscreen was written, the intended behavior was for the capability only to be consumed if there is no transient activation.

Maybe I am missing something. With the current monkey patch for Fullscreen, both are consumed. The reason for this is that in step 6 of requestFullscreen, we consume the transient activation and then the following check is always false, right?

  1. If this’s relevant global object does not have transient activation, then clear the map entry DELEGATED_CAPABILITY_TIMESTAMPS["fullscreen"] in this’s relevant global object.

The monkey patch was added in commit bc6a169. Here is the snapshot of the Fullscreen spec at that point, there the API did require but not consume transient activation. Back then, the delegated capability is only consumed if there is no transient activation.

So at that point, Fullscreen and Payment Requests had the same behavior. If there is transient activation, the delegated capability is not consumed. Maybe the monkey patch should have been updated once whatwg/fullscreen@b3bab96 ensured requestFullscreen consumes the transient activation. #38 would restore this original behavior.


The question remains whether this is the right choice, though I think this is a separate issue.

@mustaqahmed
Copy link
Collaborator

Thanks @EdgarChen for the catch, and @vinhill for looking into a fix. Sorry I missed this discussion for a while.

Maybe I am missing something. With the current monkey patch for Fullscreen, both are consumed.

Yes both are consumed, and I agree with @michaelwasserman that "consuming both" was determined to be the best/safest behavior. However, the monkey-patch still needs attention...

... at that point ... the (Fullscreen) API did require but not consume transient activation.

That's correct: consumption of user activation at requestFullscreen() had compat implications so the spec changed relatively recently, and our monkey-patch here needs to be cleaned up accordingly. I think resetting both states in a single step would be the best but I will let @vinhill decide in the PR.

the Payment API is allowed to be called again because of the delegated capability is still valid; however Fullscreen API seems to consume both. Is this intentional?

"Yes and no"...let me explain:

  • This is intentional because the details of a consumption behavior in this case (i.e. the global has both transient activation and the delegated capability) should be determined by the API owners, and we don't expect all activation consuming APIs to have a common behavior. I guess Section 5 should be more explicit about this? FYI @vinhill.

  • This is unintentional because both APIs here changed since the monkey-patches here were published (PaymentRequest now has "MAY consume", and we already noted Fullscreen changes). We will need to update both monkey-patches in consultation with the API owners, and I would defer this discussion until we push the monkey-patches to the main specs in future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants