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

RESOLVED FIXED in Firefox 43

Status

()

RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

Trunk
mozilla43
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

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.
Created attachment 8650479 [details] [diff] [review]
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.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Attachment #8650479 - Flags: review?(bugs)
Created attachment 8650483 [details] [diff] [review]
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.
Attachment #8650483 - Flags: review?(bugs)

Comment 4

3 years ago
I wonder if we could get rid of DOMActivate.
(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?
Flags: needinfo?(bugs)

Comment 6

3 years ago
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.
Flags: needinfo?(bugs)

Comment 7

3 years ago
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.
Attachment #8650483 - Flags: review?(bugs) → review+

Comment 8

3 years ago
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?
Attachment #8650479 - Flags: review?(bugs) → review-
(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.
Created attachment 8651046 [details] [diff] [review]
part.1 Add test for checking isTrust attribute value of DOMActivate event
Attachment #8650479 - Attachment is obsolete: true
Attachment #8651046 - Flags: review?(bugs)

Updated

3 years ago
Attachment #8651046 - Flags: review?(bugs) → review+
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/2518b11aca2618c587702399b5fc9749f27e02ea
changeset:  2518b11aca2618c587702399b5fc9749f27e02ea
user:       Masayuki Nakano <masayuki@d-toybox.com>
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 <masayuki@d-toybox.com>
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
https://hg.mozilla.org/mozilla-central/rev/2518b11aca26
https://hg.mozilla.org/mozilla-central/rev/6eaeb32125f4
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.