Intent to Ship: Do not perform default action on un-trusted events

134 views
Skip to first unread message

Dave Tapuska

unread,
Jun 7, 2016, 10:36:44 PM6/7/16
to blink-dev
dtap...@chromium.org https://www.w3.org/TR/uievents/#trusted-events According to the UI Events specification un-trusted events should not invoke the default action. 'click' is the only event that is a legacy permitted case. The isTrusted support was added in https://www.chromestatus.com/features/6461137440735232 which identifies trusted events from un-trusted events. We wish to prevent synthetic events from executing the default action.

This feature has been implemented behind a runtime flag for some time the goal is to move it from a disabled runtime flag to a stable runtime flag.

UMA data shows that 0.0000786182249882 % of page visits execute a default action with untrusted events (7 day aggregation; dev channel).
Other vendors are compliant with the spec and we wish to be more inter-operable with this request. There are a number of things that javascript can do to steal focus from the user by allowing synthetic events to execute the default action so there are security benefits with making this change. Issues such as 160471 and 423975 as solved by implementing this feature.
Firefox: Shipped Edge: Public Support Safari: No public signals Web developers: No signals
We added UMA tracking of executing the default action and the usage appears very low so there appears to be little compatibility risk.
None
Yes https://crbug.com/520519 https://www.chromestatus.com/features/5718803933560832

Rick Byers

unread,
Jun 9, 2016, 11:32:17 AM6/9/16
to Dave Tapuska, blink-dev
LGTM1

Jochen Eisinger

unread,
Jun 9, 2016, 11:40:28 AM6/9/16
to Rick Byers, Dave Tapuska, blink-dev
lgtm2

Dimitri Glazkov

unread,
Jun 9, 2016, 11:40:51 AM6/9/16
to Jochen Eisinger, Rick Byers, Dave Tapuska, blink-dev
LGTM3.

Bo Liu

unread,
Aug 31, 2016, 7:08:24 PM8/31/16
to Dimitri Glazkov, Jochen Eisinger, Rick Byers, Dave Tapuska, blink-dev
This change breaks FastClick library on <select> elements on android only:

I know there's usage counter stats already. I'm just making sure again with blink owners if we should forge ahead and not block m53 on this breakage? Eg I don't know how popular FastClick is, or if dev population on android is big enough to be representative, things like that.

Rick Byers

unread,
Aug 31, 2016, 8:13:27 PM8/31/16
to bo...@chromium.org, Dimitri Glazkov, Jochen Eisinger, Dave Tapuska, blink-dev
Let's discuss on the bug - there are a bunch of details here.

PhistucK

unread,
Sep 2, 2016, 9:06:25 AM9/2/16
to Rick Byers, bo...@chromium.org, Dimitri Glazkov, Jochen Eisinger, Dave Tapuska, blink-dev
This broke my private extension (no biggie, obviously :)), but perhaps my stupid code is not so original and others have come up with this crap as well. I was dispatching click in a lazy (as in a lazy man that wanted to press six (Shift included!) keyboard keys less :)) way -
element.dispatchEvent(new Event("click"));
Changing this to -
element.dispatchEvent(new MouseEvent("click"));
Fixed it, of course, which makes sense, obviously.

So -
1. Does the specification mandate that the event object is an object of certain instance?
1a. If not, should it?
2. Is this an intended side effect of this change?
2a. If not, should it?


PhistucK

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

Dave Tapuska

unread,
Sep 2, 2016, 9:47:09 AM9/2/16
to PhistucK, Rick Byers, bo...@chromium.org, Dimitri Glazkov, Jochen Eisinger, blink-dev
The spec references https://www.w3.org/TR/uievents/#event-type-click which clearly indicates it is the MouseEvent interface. It is likely reasonable for us not to do activation behaviour on non MouseEvents. From what I recall FireFox behaved this way as well. 

It is a mixed bag on the implementation. Some code sites check for MouseEvent and others don't. It is intended to be a MouseEvent so testing whether it is a click or not is implemented that way.
Reply all
Reply to author
Forward
0 new messages