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

If both are available, consume delegated fullscreen capability and transient activation in one step #38

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

Conversation

vinhill
Copy link

@vinhill vinhill commented Jun 25, 2024

In reference to #37, I suggest to make the monkey patch to the Fullscreen API more similar to the one for the Payment Request API in one way:

  • Do not consume both transient activation and delegated capability, rather consume only the former if available. This is analogous to the behavior for Payment Request API.
  • Consume the delegated capability at the same time where the transient activation would be consumed and not in parallel with returning a promise. This avoids race conditions.

@vinhill
Copy link
Author

vinhill commented Jun 25, 2024

I'm not aware of there being a test for this. If this PR is welcome, I can create a test to ensure fullscreen can be requested twice if both transient activation and delegated capability are available.

@michaelwasserman
Copy link

michaelwasserman commented Jul 1, 2024

Thanks for the PR!

  • Do not consume both transient activation and delegated capability, rather consume only the later if available

The updated text actually consumes user activation, and leaves the delegated capability signal active, when both are available. I'm not entirely sure which is preferable, but the Payment Request change does also seem to prefer consuming user activation. Consider updating the PR title and first comments description to match the proposed algorithm changes.

I also left a comment on #37, it might help to think through use cases and security considerations before changing the consumption of both signals to one or the other.

@vinhill vinhill changed the title Do not consume transient activation if fullscreen capability is delegated Do not consume delegated fullscreen capability if having transient activation Jul 2, 2024
@mustaqahmed
Copy link
Collaborator

I finally replied in #37, sorry for taking so long. May I ask for an update here based on that?

@vinhill vinhill changed the title Do not consume delegated fullscreen capability if having transient activation If both are available, consume delegated fullscreen capability and transient activation in one step Jul 9, 2024
@vinhill
Copy link
Author

vinhill commented Jul 9, 2024

@mustaqahmed thanks for your reply, resetting both transient activation and delegated capability in one step seems good to me. I updated the PR.

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

Successfully merging this pull request may close these issues.

3 participants