930843 - NS_UI_ACTIVATE should be dispatched as trusted event even if it's caused by untrusted event

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/HTMLButtonElement.cpp?rev=a03c584f9874#230
http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/HTMLInputElement.cpp?rev=3975fe261080#3401

Even if it's activated by untrusted event, the activation actually occurs. It means that its notification is truth. So, the events should be always dispatched as trusted event.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=89092227e1cf

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.1 Add test for checking isTrust attribute value of DOMActivate event

I think that DOMActivate event should be trusted event even if untrused click event causes it.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.2 NS_UI_ACTIVATE event should be trusted event even if it's caused by an untrusted event

However, Element::PostHandleEventForLinks() needs to distinguish if an NS_UI_ACTIVATE is caused by trusted event. For solving this issue, InternalUIEvent (NS_UI_ACTIVATE is the only event which uses InternalUIEvent as concrete class) should store if it's caused by a trusted event.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

I wonder if we could get rid of DOMActivate.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Olli Pettay [:smaug] from comment #4)
> I wonder if we could get rid of DOMActivate.

Hmm, do we have some data if whether DOMActivate is not used actually or not so?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

We don't have data, afaik. We should add some telemetry for that.
But now that I look at the blink/webkit source code... unfortunately they do support DOMActivate after all.
Let's deal possible DOMActivate removal in some other bug.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8650483 [details] [diff] [review]
part.2 NS_UI_ACTIVATE event should be trusted event even if it's caused by an untrusted event

IsTrustable is kind of wacky, and hopefully won't be used elsewhere.
Could we add MOZ_ASSERT(message == NS_UI_ACTIVATE); 
to IsTrustable(). And change the name of the method to IsTrustableDOMActive()
to make it clear that it shouldn't be used in other cases.
With that, r+



All this click + DOMActivate handling is rather unusual, given that usually only trusted events trigger default handling, but the web relies on untrusted clicks to trigger links too.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8650479 [details] [diff] [review]
part.1 Add test for checking isTrust attribute value of DOMActivate event

>+<botton id="button">button</button>
botton? Did this test actually pass?
And if so, why?

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Olli Pettay [:smaug] from comment #8)
> Comment on attachment 8650479 [details] [diff] [review]
> part.1 Add test for checking isTrust attribute value of DOMActivate event
> 
> >+<botton id="button">button</button>
> botton? Did this test actually pass?
> And if so, why?

Wow! But it's actually passed :-( I guess that "botton" element include the <input> elements. So, it's clicked on one of them?

https://treeherder.mozilla.org/#/jobs?repo=try&revision=89092227e1cf
> 03:29:27     INFO -  193 INFO TEST-START | dom/events/test/test_dom_activate_event.html
> 03:29:27     INFO -  ++DOMWINDOW == 55 (0x8fde8000) [pid = 2075] [serial = 340] [outer = 0x945f1800]
> 03:29:27     INFO -  MEMORY STAT | vsize 631MB | residentFast 234MB | heapAllocated 88MB
> 03:29:28     INFO -  194 INFO TEST-OK | dom/events/test/test_dom_activate_event.html | took 626ms

Anyway, I'll correct it though.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.1 Add test for checking isTrust attribute value of DOMActivate event

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cbc8b4f2b9a2

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/2518b11aca2618c587702399b5fc9749f27e02ea
changeset:  2518b11aca2618c587702399b5fc9749f27e02ea
user:       Masayuki Nakano <[e-mail]>
date:       Sat Aug 22 13:02:39 2015 +0900
description:
Bug 930843 part.1 Add test for checking isTrust attribute value of DOMActivate event r=smaug

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/6eaeb32125f4ce179182e8742e0cb43c6251ebc8
changeset:  6eaeb32125f4ce179182e8742e0cb43c6251ebc8
user:       Masayuki Nakano <[e-mail]>
date:       Sat Aug 22 13:02:39 2015 +0900
description:
Bug 930843 part.2 NS_UI_ACTIVATE event should be trusted event even if it's caused by an untrusted event r=smaug

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2518b11aca26
https://hg.mozilla.org/mozilla-central/rev/6eaeb32125f4