Bug 168301 - Fallback to legacy type only when event is trusted
Summary: Fallback to legacy type only when event is trusted
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-14 03:55 PST by Philip Jägenstedt
Modified: 2017-02-14 13:13 PST (History)
10 users (show)

See Also:

Attachments
Patch (4.18 KB, patch)
2017-02-14 09:36 PST, Chris Dumez 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews115 for mac-elcapitan (1.77 MB, application/zip)
2017-02-14 10:29 PST, Build Bot 		no flags Details
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (831.42 KB, application/zip)
2017-02-14 10:41 PST, Build Bot 		no flags Details
Patch (5.07 KB, patch)
2017-02-14 10:46 PST, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (5.08 KB, patch)
2017-02-14 11:01 PST, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (5.05 KB, patch)
2017-02-14 13:12 PST, Chris Dumez 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Jägenstedt 2017-02-14 03:55:11 PST 
When dispatching e.g. an "animationstart" event, WebKit falls back to "webkitanimationstart" if there were no listeners for "animationstart". This happens in EventTarget::fireEventListeners.

This behavior has made its way into DOM, and was tweaked to only apply to trusted events:
https://github.com/whatwg/dom/issues/404
https://github.com/whatwg/dom/pull/406
https://github.com/w3c/web-platform-tests/pull/4687
Comment 1 Chris Dumez 2017-02-14 08:34:14 PST 
I like it. I'll prepare a patch.
Comment 2 Chris Dumez 2017-02-14 09:36:02 PST 
Created attachment 301514 [details]
Patch
Comment 3 Chris Dumez 2017-02-14 10:26:42 PST 
Comment on attachment 301514 [details]
Patch

Looks like it is failing a test. I'll investigate.
Comment 4 Build Bot 2017-02-14 10:29:03 PST 
Comment on attachment 301514 [details]
Patch

Attachment 301514 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3116930

New failing tests:
fast/events/wheelevent-in-scrolling-div.html
Comment 5 Build Bot 2017-02-14 10:29:08 PST 
Created attachment 301517 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 6 Build Bot 2017-02-14 10:41:48 PST 
Comment on attachment 301514 [details]
Patch

Attachment 301514 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3116984

New failing tests:
fast/events/wheelevent-in-scrolling-div.html
Comment 7 Build Bot 2017-02-14 10:41:53 PST 
Created attachment 301518 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 8 Chris Dumez 2017-02-14 10:46:59 PST 
Created attachment 301520 [details]
Patch
Comment 9 Darin Adler 2017-02-14 11:00:01 PST 
Comment on attachment 301520 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=301520&action=review

> Source/WebCore/dom/EventTarget.cpp:205
> +    // Do not fall back to legacy types when even is not trusted.

Typo: "even".
Comment 10 Chris Dumez 2017-02-14 11:01:23 PST 
Created attachment 301524 [details]
Patch
Comment 11 Ryosuke Niwa 2017-02-14 12:40:31 PST 
Comment on attachment 301524 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=301524&action=review

> Source/WebCore/dom/EventTarget.cpp:206
> +    // Do not fall back to legacy types when event is not trusted.
> +    if (!event.isTrusted())

I think it would have been clearer to say "Only fallback to legacy types on trusted events".
Comment 12 Chris Dumez 2017-02-14 13:00:33 PST 
Comment on attachment 301524 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=301524&action=review

>> Source/WebCore/dom/EventTarget.cpp:206
>> +    if (!event.isTrusted())
> 
> I think it would have been clearer to say "Only fallback to legacy types on trusted events".

Sure, I can update the comment before landing.
Comment 13 Ryosuke Niwa 2017-02-14 13:04:28 PST 
Weird, I thought the patch had already been landed.
Comment 14 Chris Dumez 2017-02-14 13:12:49 PST 
Created attachment 301536 [details]
Patch
Comment 15 Chris Dumez 2017-02-14 13:13:35 PST 
Committed r212321: <http://trac.webkit.org/changeset/212321>