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.

Sign up for GitHub

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

Jump to bottom

Should the Esc key count as user activation? #7

Closed
domenic opened this issue Sep 21, 2021 · 3 comments
Closed

Should the Esc key count as user activation? #7

domenic opened this issue Sep 21, 2021 · 3 comments

Comments

@domenic
Copy link
Collaborator

domenic commented Sep 21, 2021

When coding up the current spec/explainer draft, I realized that the Esc key counts as user activation. This means that on desktop, all the fancy abuse prevention mechanisms for the cancel event do nothing: since pressing Esc means there is always recent user activation, the cancel event will always fire (and can be preventDefault()ed).

On the one hand: that seems fine! On desktop we don't have to worry about back trapping, and we can already preventDefault() the Esc keydown event. No issues; this makes the API easier to use on desktop as you don't have to worry so much about user activation!

On the other hand: this is a weird divergence between desktop and Android (or desktop and other platforms where the close signal is not a user activation), and makes code harder to test. So maybe we should spec it so that an Esc keypress (or other user action) that sends a close signal, never counts as user activation, regardless of the platform.
domenic added a commit that referenced this issue Sep 21, 2021
@domenic
Align spec with implementation experience
b96e092 
* The constructor needs to _consume_ transient user activation, not just check it.
* We need a reentrancy flag for the cancel event.
* Note the discussion about the Esc key being user activation (#7).
@domenic domenic mentioned this issue Sep 21, 2021
Merged
domenic added a commit that referenced this issue Sep 21, 2021
@domenic
Align spec with implementation experience
c5f4382 
* The constructor needs to _consume_ transient user activation, not just check it.
* We need a reentrancy flag for the cancel event.
* We need to not fire close if destroy() was called during the cancel event handler.
* Note the discussion about the Esc key being user activation (#7).
@domenic
Copy link
Collaborator Author

domenic commented Sep 21, 2021

@chrishtr suggests that maybe the Esc key in general should not count as user activation, which would indeed solve this problem. For example, visiting a page, and doing nothing but pressing Esc, should maybe not give that page license to show a popup, or play unmuted video.

This is pretty interesting and probably deserves wider discussion with the HTML community. I will add a comment at whatwg/html#3849.
@domenic domenic mentioned this issue Sep 21, 2021
Closed
@mustaqahmed
Copy link

I agree with @domenic's proposal here, and this should be discussed in whatwg/html#3849.

(Reposting from that HTML issue: Chrome stopped counting Esc key as a user activation a while ago.)
pull bot pushed a commit to luojiguicai/chromium that referenced this issue Oct 19, 2021
@domenic
Implement user activation gating for CloseWatcher cancel events
f81bb8a 
This matches the spec after
WICG/close-watcher#8. Note the outstanding
question at WICG/close-watcher#7 about the
Esc key; since Chromium currently does not count the Esc key as user
activation, we have one particular behavior.

The tracking of last user activation time ensures that any user
activation allows the CloseWatcher to, in the future, fire a cancel
event---not just during the transient activation duration time window.
This gives the desired semantics outlined in
https://github.com/WICG/close-watcher#asking-for-confirmation and
https://github.com/WICG/close-watcher#abuse-analysis.

Bug: 1171318

Change-Id: I34212f29539d5796b4a9bdc4c7351b9d8ea5880d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3168969
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#932757}
@domenic
Copy link
Collaborator Author

domenic commented Oct 27, 2021

This seems to be being taken care of on the HTML side, so we are good to go: whatwg/html#7248. Thanks @mustaqahmed for doing that work!
@domenic domenic closed this as completed Oct 27, 2021
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
@domenic
Implement user activation gating for CloseWatcher cancel events
c20c6ff 
This matches the spec after
WICG/close-watcher#8. Note the outstanding
question at WICG/close-watcher#7 about the
Esc key; since Chromium currently does not count the Esc key as user
activation, we have one particular behavior.

The tracking of last user activation time ensures that any user
activation allows the CloseWatcher to, in the future, fire a cancel
event---not just during the transient activation duration time window.
This gives the desired semantics outlined in
https://github.com/WICG/close-watcher#asking-for-confirmation and
https://github.com/WICG/close-watcher#abuse-analysis.

Bug: 1171318

Change-Id: I34212f29539d5796b4a9bdc4c7351b9d8ea5880d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3168969
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#932757}
NOKEYCHECK=True
GitOrigin-RevId: f81bb8a5905014bebe9cb4520cff7a8ac036702d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants
@domenic @mustaqahmed