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

Analyze isTrusted usage #1912

Closed
annevk opened this issue Oct 17, 2016 · 15 comments
Closed

Analyze isTrusted usage #1912

annevk opened this issue Oct 17, 2016 · 15 comments

Comments

@annevk
Copy link
Member

annevk commented Oct 17, 2016

Other than click() and <form>.reset() most user-agent-dispatched events should have isTrusted initialized to true.

E.g., EventSource dispatching untrusted message events seems suspect and probably wrong.

@foolip
Copy link
Member

foolip commented Oct 17, 2016

Is there any case yet where we know isTrusted shouldn't be true?

@annevk
Copy link
Member Author

annevk commented Oct 17, 2016

I listed two cases in OP.

@foolip
Copy link
Member

foolip commented Oct 17, 2016

Oh, sorry. I thought this was simple in Blink, in that all events start out with isTrusted false and it's only set to true by the internal dispatch, but that's not the case.

Here's a form.reset() test:
https://software.hixie.ch/utilities/js/live-dom-viewer/saved/4576

It looks like isTrusted is true in Chrome, Edge, Firefox and Safari, isn't that harmless as long as no code exists that makes false assumptions?

@foolip
Copy link
Member

foolip commented Oct 17, 2016

In https://software.hixie.ch/utilities/js/live-dom-viewer/saved/4577, all except Edge have isTrusted as false on the click event. I wonder how that's made to work in Edge.

@annevk
Copy link
Member Author

annevk commented Oct 17, 2016

It's still a little bit unclear to me what the use of isTrusted is, but if it's about user-agent-dispatched events it seems somewhat harmful if they can be initiated through JavaScript.

@annevk
Copy link
Member Author

annevk commented Oct 17, 2016

@smaug----

@smaug----
Copy link

Use of isTrusted is that default handling can then check whether the event is from UA.
'click' being the special case, since dispatchEvent(new MouseEvent('click')) needs to still trigger default handling.
(and talking about the default handling browsers have, not the different thing which has been in the spec for ages, but I think I've lost track whether that has changed recently.)

@foolip
Copy link
Member

foolip commented Oct 17, 2016

The form.reset() case seems similar in principle to video.volume=0.5 queueing a task to fire a trusted volumechange event. form.reset() does dispatch the trusted reset event synchronously, but I don't think that matters.

If all UA-created events except the ones created inside click() have isTrusted set to true, that's at least a kind of simple.

@annevk
Copy link
Member Author

annevk commented Oct 17, 2016

I guess, a little unclear what utility it offers to developers, but I suppose it doesn't matter.

@foolip
Copy link
Member

foolip commented Oct 17, 2016

Yeah, the isTrusted attribute is kind of pointless AFAICT, at most a convenience to distinguish real and synthetic events with the UA doing the bookkeeping for you.

domenic pushed a commit that referenced this issue Oct 18, 2016
This aligns firing and dispatching of events with updated terminology in
the DOM Standard. Among the changes:

* A lot less usage of "default action" which isn't really a thing.
  Instead we make use of the return value of the fire and dispatch
  algorithms.
* Instead of saying things bubble or are cancelable we initialize the
  attributes as such.
* We no longer re-state defaults for isTrusted, bubbles, and cancelable,
  as that only leads to confusion when they are *not* re-stated.
* We now use the legacy target override flag rather than supplying a
  named argument.

This fixes #1713, but plenty of follow up issues remain:

* #805 for the remainder of "default action" usage
* #1394 for updating synthetic click events
* #1887 for removing "fire a simple event" usage
* #1893 for updating when checkboxes get checked
* #1900 for figuring out if event dispatch requires more hooks
* #1912 for revisiting isTrusted usage
* #1913 for updating synthetic mouse events
* #1922 for making more events composed
@domenic
Copy link
Member

domenic commented Oct 19, 2016

at most a convenience to distinguish real and synthetic events with the UA doing the bookkeeping for you.

I actually really like this characterization and would support updating DOM to make it the primary definition of isTrusted. Maybe with some nice warning about how the name is historical.

domenic added a commit that referenced this issue Oct 19, 2016
Closes #1602. Browsers already behave this way, and it is more consistent with how the rest of the spec almost always fires trusted events. See also #1912.
@dtapuska
Copy link
Contributor

dtapuska commented Oct 19, 2016

@foolip isTrusted was useful for chrome extensions to determine if the user actually did the action or it was injected by something on the page.

@dtapuska
Copy link
Contributor

Perhaps I don't follow the spec change. But it seems isTrusted is true most of the time which is entirely incorrect...

Here is an example where it is false:
http://jsbin.com/lewegog/edit?html,output

@domenic
Copy link
Member

domenic commented Oct 19, 2016

So in general the rule seems to be that whenever the UA dispatches the event, isTrusted = true, and when the web developer does, isTrusted = false.

Of course these terms are kind of vague. Presumably dispatchEvent() calls is the web developer dispatching, even though it's UA code inside the dispatchEvent() algorithm that actually dispatches the event.

click() is then a sticky case. It seems like in practice it's treated like "basically dispatchEvent()", so is not trusted, even though technically it's again UA code doing the dispatch.

But in all other cases where the UA dispatches the event, it's trusted.

@dtapuska
Copy link
Contributor

That is precisely the way I implemented it. Synthetic clicks are slightly
special.

On Oct 19, 2016 16:25, "Domenic Denicola" [email protected] wrote:

So in general the rule seems to be that whenever the UA dispatches the
event, isTrusted = true, and when the web developer does, isTrusted = false.

Of course these terms are kind of vague. Presumably dispatchEvent() calls
is the web developer dispatching, even though it's UA code inside the
dispatchEvent algorithm that actually dispatches the event.

click() is then a sticky case. It seems like in practice it's treated like
"basically dispatchEvent()", so is not trusted, even though technically
it's UA code doing the dispatch.

But in all other cases where the UA dispatches the event, it's trusted.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1912 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AJ7KRAuSs_qAjzlBKD8k26WrJ1rPrwx9ks5q1ny9gaJpZM4KYSl5
.

domenic added a commit that referenced this issue Oct 24, 2016
Closes #1602. Browsers already behave this way, and it is more
consistent with how the rest of the spec almost always fires trusted
events. See also #1912.

In terms of spec mechanics, this is done by using the new "create an
event" hook from DOM, which sets isTrusted to true for us.
domenic added a commit that referenced this issue Oct 25, 2016
Closes #1602. Browsers already behave this way, and it is more
consistent with how the rest of the spec almost always fires trusted
events. See also #1912.

In terms of spec mechanics, this is done by using the new "create an
event" hook from DOM, which sets isTrusted to true for us.
domenic added a commit that referenced this issue Oct 25, 2016
Closes #1602. Browsers already behave this way, and it is more
consistent with how the rest of the spec almost always fires trusted
events. See also #1912.

In terms of spec mechanics, this is done by using the new "create an
event" hook from DOM, which sets isTrusted to true for us.
domenic added a commit that referenced this issue Oct 25, 2016
Closes #1602. Browsers already behave this way, and it is more
consistent with how the rest of the spec almost always fires trusted
events. See also #1912.

In terms of spec mechanics, this is done by using the new "create an
event" hook from DOM, which sets isTrusted to true for us.

This helps with #1789.
annevk added a commit that referenced this issue Oct 27, 2016
<form>.reset() is trusted by browsers.

One cannot trigger “send select update notifications” from script as
far as I can tell.

Fixes #1912.
domenic pushed a commit that referenced this issue Nov 16, 2016
<form>.reset() always sets isTrusted in current implementations, so we
remove the spec language that sometimes set it to false.

One cannot trigger “send select update notifications” from script, so
the spec language that sometimes sets isTrusted to false for those
events can never be triggered, and is thus be removed.

Fixes #1912.
annevk added a commit to whatwg/dom that referenced this issue Nov 17, 2016
annevk added a commit to whatwg/dom that referenced this issue Nov 17, 2016
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
This aligns firing and dispatching of events with updated terminology in
the DOM Standard. Among the changes:

* A lot less usage of "default action" which isn't really a thing.
  Instead we make use of the return value of the fire and dispatch
  algorithms.
* Instead of saying things bubble or are cancelable we initialize the
  attributes as such.
* We no longer re-state defaults for isTrusted, bubbles, and cancelable,
  as that only leads to confusion when they are *not* re-stated.
* We now use the legacy target override flag rather than supplying a
  named argument.

This fixes whatwg#1713, but plenty of follow up issues remain:

* whatwg#805 for the remainder of "default action" usage
* whatwg#1394 for updating synthetic click events
* whatwg#1887 for removing "fire a simple event" usage
* whatwg#1893 for updating when checkboxes get checked
* whatwg#1900 for figuring out if event dispatch requires more hooks
* whatwg#1912 for revisiting isTrusted usage
* whatwg#1913 for updating synthetic mouse events
* whatwg#1922 for making more events composed
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
Closes whatwg#1602. Browsers already behave this way, and it is more
consistent with how the rest of the spec almost always fires trusted
events. See also whatwg#1912.

In terms of spec mechanics, this is done by using the new "create an
event" hook from DOM, which sets isTrusted to true for us.

This helps with whatwg#1789.
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
<form>.reset() always sets isTrusted in current implementations, so we
remove the spec language that sometimes set it to false.

One cannot trigger “send select update notifications” from script, so
the spec language that sometimes sets isTrusted to false for those
events can never be triggered, and is thus be removed.

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

No branches or pull requests

5 participants