1476612 - key events should be considered user interactions when received by editable HTML elements

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Andrea Marchesini [:baku]]

https://searchfox.org/mozilla-central/rev/c296d5b2391c8b37374b118180b64cca66c0aa16/dom/events/EventStateManager.cpp#887-894

I don't see why this is a valid statement. Here a scenario:

1. site.com does a window.open(page.html)
2. page.html shows a a form and it appears in a popup window.
3. the user clicks on a <input type=text> element.

The document should be marked as user-interacted.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: editable.patch

Comment 2

[Andrea Marchesini [:baku]]

It's important to share why I found this issue:

for the anti-tracking project, we grant first party storage access to tracking resources after user interactions. In my testing, I saw that typing something into a <input type=text> was not enough. Basically I'm using the user-interaction monitoring for something that is different than media autoplaying. It behaves perfectly, except for editable HTML elements.

Comment 3

[Andrew Overholt [:overholt]]

baku, does this have anything to do with bug 1359867? For that change, we've been debating what counts as a user interaction.

Comment 4

[Andrea Marchesini [:baku]]

Not at the moment. The concept of user-interaction here is taken from media autoplay. See bug 1470346.
We can use part of this approach for allow-top-navigation-by-user-activation iframe sandbox. I'll follow that bug.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8992960 [details] [diff] [review]
editable.patch

Review of attachment 8992960 [details] [diff] [review]:
-----------------------------------------------------------------

When block autoplay is enabled, we don't want to activate for events sent to editable elements. It would be confusing if video started playing when you were typing something. Ideally we only want to gesture activate on a "play" button click/submit. So we don't want this change for our use case.

Maybe what you want for your use case is to use nsIDocument::UserHasInteracted() [1], instead of nsIDocument::HasBeenUserGestureActivated()?

UserHasInteracted() doesn't filter out events targeted at editable elements [2]. Also note that when one document in a doctree is gesture activated (for HasBeenUserGestureActivated()), every document the same tab is (effectively) gesture activated, including descendant document, which may not be what you want here. UserHasInteracted() only activates the current document, and ancestor documents. Also "gesture activation" only activates for presses of printable keys, so key events which are likely to be keyboard shortcuts don't activate. Whereas the "user has interacted" feature doesn't filter those out.

So I suggest you either:
1. Use nsIDocument::UserHasInteracted() instead of nsIDocument::HasBeenUserGestureActivated(), or,
2. Refactor the HasBeenUserGestureActivated() feature to instead store some kind of bitmask so we can both get the behaviour we want out of the one feature. Maybe we could merge both UserHasInteracted() and HasBeenUserGestureActivated() into one, as it's confusing having two.


[1] https://searchfox.org/mozilla-central/rev/c296d5b2391c8b37374b118180b64cca66c0aa16/dom/base/nsIDocument.h#3373
[2] https://searchfox.org/mozilla-central/rev/c296d5b2391c8b37374b118180b64cca66c0aa16/dom/events/EventStateManager.cpp#508

Comment 6

[Andrea Marchesini [:baku]]

Attachment: editable.patch

Thanks for your comment!

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8993136 [details] [diff] [review]
editable.patch

Review of attachment 8993136 [details] [diff] [review]:
-----------------------------------------------------------------

Easy as! :)

Commit message needs updating.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/694089774911
AntiTracking should use nsIDocument::Get/SetUserHasInteracted instead of UserGestureActivation, r=cpearce

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/694089774911