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

Added a new section on User Activation v2. #3851

Merged
merged 15 commits into from
Dec 4, 2019
Merged

Conversation

mustaqahmed
Copy link
Contributor

@mustaqahmed mustaqahmed commented Jul 25, 2018

Replaced most of Section 6.3 with the new user activation model, preserved only
a part (events triggering user activation) into a new Section 6.4 which needs to
be addressed through a separate issue.

Fixes #1903.


/browsers.html ( diff )
/browsing-the-web.html ( diff )
/dnd.html ( diff )
/iframe-embed-object.html ( diff )
/index.html ( diff )
/input.html ( diff )
/interaction.html ( diff )
/media.html ( diff )
/origin.html ( diff )

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

Thanks for getting this started! It's looking pretty good, but is missing some of the processing model: a lot of things are currently just stated as "this is set/this is unset" or "this is asymmetric", but there is no normative algorithm backing that up.

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@dtapuska dtapuska mentioned this pull request Sep 6, 2018
source Outdated Show resolved Hide resolved
@domenic
Copy link
Member

domenic commented Apr 12, 2019

@mustaqahmed I did a pass through this to rigorize it, which should help. Please take a look. Note the explicit algorithms for activation and consumption, which were an important change.

The biggest outstanding issue I see is that nowhere in the spec calls the consumption algorithm. I would expect at least window.open() to do so. Probably more? So the next major step is actually implementing those three categories of APIs you mention, via updating this PR and/or sending other PRs.

@annevk and/or @bzbarsky, if you want to take a look, I think this is relatively ready now. Although, it looks like PR preview is not in good shape, so you'd have to read the raw diff; the links in the OP are outdated...

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I found some minor issues. I'm rather worried about all the browsing context traversal.

It's also not clear to me this approach has cross-browser buy-in. At least I don't think Firefox is actively pursuing this. Is Safari?

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated

<li><p>Let <var>windows</var> be the list of <code>Window</code> objects constructed by, for each
<var>browsingContext</var> of <var>browsingContexts</var>, taking the [[Window]] internal slot
value of <var>browsingContext</var>'s <code>WindowProxy</code> object.</p></li>
Copy link
Member

Choose a reason for hiding this comment

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

This is basically grabbing the active document but saying it in a different way. That doesn't seem great to me as it makes it harder to audit active document (a dangerous concept).

source Outdated Show resolved Hide resolved
@othermaciej
Copy link

I found some minor issues. I'm rather worried about all the browsing context traversal.

It's also not clear to me this approach has cross-browser buy-in. At least I don't think Firefox is actively pursuing this. Is Safari?

Safari has various things gated on a user gesture requirement. And it seems like a good idea to 'make "triggered by user activation" match browser behavior', as the title of the related issue states.

But I can't tell at a glance if this spec at all matches what we currently implement.

Is there a summary available of current browser behavior, how this matches or differs, and why any differences are ok? It's hard to review the spec changes without that or to evaluate whether it's something we want to implement. (We are trying to evaluate in any case.)

@mustaqahmed
Copy link
Contributor Author

Is there a summary available of current browser behavior, how this matches or differs, and why any differences are ok?

Here is a one-page summary of differences, from our BlinkOn presentation last week. The link within that page has more details.

@othermaciej
Copy link

Thanks, that's a helpful resource on browser differences in popup blocking. One thing I don't see there is a comparison of those browser behaviors to the spec text in the PR. Is that available, or is that something we should figure out from reading it?

Also, you mentioned there are many other APIs that depend on this concept. Do you have any comparisons for those? I'm particularly interested in video playback with sound. That's a case where Blink and WebKit have some gating on user actions, but it's not the same. And I'm not sure that we'd be willing to adopt the Blink behavior.

Finally, I'd like to make clear that I love the idea of spelling out precisely how user gesture / user activation requirements work. And also to make them consistent cross-browser. What I'm not sure of is what would change if WebKit implemented the behavior here, and whether we are ok with that change. It seems like we'd have to do a bunch of our own testing and analysis to figure that out. Which we can do, but it might take a while.

Do you perhaps have any test cases available that would help us figure out where we differ from this PR, for popups, media playback, and any other APIs affected by this?

source Outdated
</li>
</ol>

<p>To <dfn>consume user activation</dfn> for a <code>Window</code> <var>W</var>, perform the
Copy link
Member

Choose a reason for hiding this comment

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

If one is starting from an element or document, having this algorithm take a browsing context would be more convenient. Otherwise one has to say "browsing context's WindowProxy object's [[Window]] internal slot" just to have this algorithm get the browsing context again.

Copy link
Contributor

Choose a reason for hiding this comment

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

If one is starting from an element or document, wouldn't one want to go to the Window directly? Going through the browsing context could end up at a different Window than the Window associated with the element or document, no?

foolip added a commit to whatwg/fullscreen that referenced this pull request May 28, 2019
foolip added a commit to whatwg/fullscreen that referenced this pull request May 28, 2019
foolip added a commit to whatwg/fullscreen that referenced this pull request May 28, 2019
@mustaqahmed
Copy link
Contributor Author

@othermaciej: below are my explanations to your queries, sorry for taking so long:

Thanks, that's a helpful resource on browser differences in popup blocking. One thing I don't see there is a comparison of those browser behaviors to the spec text in the PR. Is that available, or is that something we should figure out from reading it?

It's more like the latter; let me explain why.

User activation is a much lower level concept that the capabilities (i.e. the activation-gated APIs) that define browser behavior, like popup, vibrate, autoplay etc. Each of those capabilities is a function of many different input parameters, and user activation is just one such parameter. For example, Chrome’s autoplay behavior depends on user activation as well as feature policy, iframe sandbox attributes, MEI etc. This PR is changing (more precisely, providing a clear definition of) the “user activation” parameter only; fully defining any capability like autoplay is beyond the scope of this PR.

In other words, for each capability, individual browsers and capability spec owners still rightfully own their spec discussion and/or implementation details. We are only proposing to formalize those APIs’ dependence on user-activation itself.

Also, you mentioned there are many other APIs that depend on this concept. Do you have any comparisons for those? I'm particularly interested in video playback with sound. That's a case where Blink and WebKit have some gating on user actions, but it's not the same. And I'm not sure that we'd be willing to adopt the Blink behavior.

It’s hard to compare all APIs here because of other browser-defined and/or API-specific parameters involved. In the last few weeks I added more APIs (including media playback) to my UAv2 consistency tester demos, and you can easily verify that for a simple capability like popup or fullscreen, the availability of user activation defines the API availability. But complex APIs like media playback are, well, complex, and they could remain so.

This shows that any browser engine can choose to implement UAv2 for its benefits (well-defined and consistent availability across frames through timers, postMessages etc without a complicated implementation), and still have browser-defined parameters to control any capability. If in future a capability deserves a browser-independent behavior, that could be a separate spec discussion. Through this PR we are doing the groundwork to make that future discussion possible, without proposing any capability-specific behavior.

Finally, I'd like to make clear that I love the idea of spelling out precisely how user gesture / user activation requirements work. And also to make them consistent cross-browser. What I'm not sure of is what would change if WebKit implemented the behavior here, and whether we are ok with that change. It seems like we'd have to do a bunch of our own testing and analysis to figure that out. Which we can do, but it might take a while.

Do you perhaps have any test cases available that would help us figure out where we differ from this PR, for popups, media playback, and any other APIs affected by this?

Please use my demos (linked above). I have generalized the consistency testers there to support many (any?) APIs. If you would like to see any API I haven’t added yet, let me know.

@foolip
Copy link
Member

foolip commented Aug 21, 2019

@mustaqahmed what is the status of this? whatwg/fullscreen#153 depends on it, and it would be nice to get that merged.

@domenic
Copy link
Member

domenic commented Aug 21, 2019

My read is that the multi-implementer interest criteria is still not clearly met. We have had very helpful engagement from both @bzbarsky and @othermaciej, but the conversations haven't really settled. This may be the kind of situation where a TPAC session or other synchronous conversation would be ideal.

One could argue that this proposal (which matches one browser) is better than the current spec text (which matches zero), and thus we should merge it. But I don't want to be disrespectful to the ongoing discussions, so I am hesitant to do so.

@annevk annevk requested a review from EdgarChen August 26, 2019 14:14
source Outdated Show resolved Hide resolved
@mustaqahmed
Copy link
Contributor Author

I have added a session on TPAC Plenary Day to have a face-to-face discussion about this PR. Please help me spread the word, specially to engineers from other browsers who are attending TPAC. @bratell-at-opera @othermaciej @EdgarChen @annevk @domenic @foolip

source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
@domenic
Copy link
Member

domenic commented Nov 11, 2019

I pushed a commit which rearranges and tweaks things a bit, but it was mostly editorial. I think this is relatively good to go.

Things to double check:

  • Are @annevk's concerns about browsing context traversal captured correctly with the XXX box?
  • Is @annevk OK with having a definition that applies to browsing contexts, i.e. allowing "does B have transient activation" as a convenience instead of having to do "does B's WindowProxy's [[Window]] have transient activation"?
  • Nothing in the spec currently references sticky activation or consuming activation. I understand we are just working on the base model for now, but I am surprised that popups in particular don't consume activation. (And maybe input type=file too.) Is this as intended?

@mustaqahmed
Copy link
Contributor Author

Thanks @domenic for cleaning things up.

Re your third point above (consumption through popups): in early 2017 only two out of the four major browsers used to consume, which changed to three out of four by the end of 2017. I don't know if Mozilla has changed since then.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I think I would prefer leaving out browsing contexts as having this to prevent others from taking a dependency. I realize this introduces some redundant wording, but I suspect that over time that'll become better as more things are properly anchored in documents/windows.

source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
@domenic
Copy link
Member

domenic commented Nov 20, 2019

@mustaqahmed can you work on @annevk's final comments, and removing the browsing context indirection in favor of adding extra wording in each call site?

Regarding consumption, given the lack of interop there it makes sense to worry about that in a separate issue.

@mustaqahmed
Copy link
Contributor Author

I will resume my work on this PR tomorrow.

@mustaqahmed
Copy link
Contributor Author

Addressed all review comments, PTAL.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

LGTM. @annevk want to take a final look?

@domenic domenic merged commit 8f8c1f5 into whatwg:master Dec 4, 2019
@marcoscaceres
Copy link
Member

marcoscaceres commented Dec 5, 2019

Soooo... we now have quite a few specs that need to be updated as "triggered by user activation" is no longer a thing. I see "consume user activation" and a few others things are now exported, but I was unable to find a nice way to go from, for instance:

If the method was not triggered by user activation, return a promise rejected with with a "SecurityError" DOMException.

How would I rewrite the above to use the new "consume user activation" or whichever of the new exports is appropriate?

@mustaqahmed
Copy link
Contributor Author

Thanks @marcoscaceres, that's a very timely question. Yes, our next step is to gradually fix other specs that rely on user activation in some way. We need to file an issue for individual API's spec.

@domenic: Any idea what's the best way to track issues in multiple specs at a central place? Perhaps we can file a "meta" issue in this (HTML) spec; then every time we file an issue (or find an existing issue) in a different spec, we could refer to this meta issue. Or may be we can maintain a public spreadsheet that lists the issue-link and current status for each of those specs?

@domenic
Copy link
Member

domenic commented Dec 5, 2019

A meta-issue on HTML sounds like a great plan to me!

@mustaqahmed
Copy link
Contributor Author

#5129 is the meta issue we will use.

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

Successfully merging this pull request may close these issues.

Make "triggered by user activation" match browser behavior
9 participants