WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
76104
WheelEvent should inherit from MouseEvent
https://bugs.webkit.org/show_bug.cgi?id=76104
Summary
WheelEvent should inherit from MouseEvent
Eric Seidel (no email)
Reported
2012-01-11 14:36:22 PST
WheelEvent should inherit from MouseEvent At least according to the most recent DOM3 Events spec I found:
http://dev.w3.org/2006/webapi/DOM-Level-3-Events/html/DOM3-Events.html#events-wheelevents
This is causing this IETC sample to fail:
http://samples.msdn.microsoft.com/ietestcenter/dominheritance/showdominheritancetest.htm?Prototype_WheelEvent
Looks like this file has been with us a long time. :)
http://trac.webkit.org/log/trunk/Source/WebCore/dom/WheelEvent.h
Attachments
WIP patch, ifdefs to avoid ObjC API changes
(8.25 KB, patch)
2012-01-12 02:24 PST
,
Dominic Cooney
no flags
Details
Formatted Diff
Diff
Patch
(16.13 KB, patch)
2012-06-01 09:32 PDT
,
Dominic Cooney
no flags
Details
Formatted Diff
Diff
Patch
(10.44 KB, patch)
2012-06-03 03:21 PDT
,
Dominic Cooney
no flags
Details
Formatted Diff
Diff
Patch
(11.57 KB, patch)
2012-06-03 09:26 PDT
,
Dominic Cooney
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ec2-cr-linux-02
(712.69 KB, application/zip)
2012-06-03 11:03 PDT
,
WebKit Review Bot
no flags
Details
Patch
(11.36 KB, patch)
2012-06-25 22:21 PDT
,
Dominic Cooney
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Dominic Cooney
Comment 1
2012-01-12 01:51:38 PST
This would be a public API change for Objective-C. What is the right way to approach that aspect of this?
Eric Seidel (no email)
Comment 2
2012-01-12 02:02:15 PST
Tim is the man to talk to. But I doubt it will be a problem. It is unlikely that any WebKit users would depend on this exact inheritance chain in their Obj-C.
Dominic Cooney
Comment 3
2012-01-12 02:24:19 PST
Created
attachment 122196
[details]
WIP patch, ifdefs to avoid ObjC API changes Waiting for advice on ObjC API changes.
WebKit Review Bot
Comment 4
2012-01-12 02:26:46 PST
Attachment 122196
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/dom/WheelEvent.h:34: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Review Bot
Comment 5
2012-01-12 02:55:57 PST
Comment on
attachment 122196
[details]
WIP patch, ifdefs to avoid ObjC API changes
Attachment 122196
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/11170752
Timothy Hatcher
Comment 6
2012-01-12 09:40:18 PST
Since DOMMouseEvent inherits from DOMUIEvent, which was the old superclass for DOMWheelEvent there should be no problem moving DOMWheelEvent to inherit from DOMMouseEvent.
Dominic Cooney
Comment 7
2012-06-01 09:32:25 PDT
Created
attachment 145328
[details]
Patch
WebKit Review Bot
Comment 8
2012-06-01 09:34:14 PDT
Please wait for approval from
timothy@apple.com
(or another member of the Apple Safari Team) before submitting because this patch contains changes to the Apple Mac WebKit.framework public API.
Timothy Hatcher
Comment 9
2012-06-02 11:07:12 PDT
The ObjC changes are fine. I'll leave the rest for another reviewer that knows this area better.
Kentaro Hara
Comment 10
2012-06-02 16:58:48 PDT
Comment on
attachment 145328
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=145328&action=review
> Source/WebCore/ChangeLog:7 > +
Please describe the link of the spec and the IETC test that support this change.
> Source/WebCore/dom/WheelEvent.h:-34 > - class WheelEvent : public MouseRelatedEvent {
Isn't there any concern to lose the inheritance from MouseRelatedEvent? I guess there might have been some reason behind the fact that MouseRelatedEvent was inherited instead of MouseEvent.
> Source/WebCore/dom/WheelEvent.h:-85 > - };
We might not want to fix the indentation, to keep the git/svn history sane.
Dominic Cooney
Comment 11
2012-06-03 02:52:09 PDT
Comment on
attachment 145328
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=145328&action=review
>> Source/WebCore/dom/WheelEvent.h:-34 >> - class WheelEvent : public MouseRelatedEvent { > > Isn't there any concern to lose the inheritance from MouseRelatedEvent? I guess there might have been some reason behind the fact that MouseRelatedEvent was inherited instead of MouseEvent.
MouseRelatedEvent was created to give a common interface between MouseEvent and WheelEvent. So WheelEvent still gets MouseRelatedEvent via MouseEvent. We can’t remove MouseRelatedEvent because it is also used by TouchEvent.
Dominic Cooney
Comment 12
2012-06-03 03:21:09 PDT
Created
attachment 145478
[details]
Patch
Kentaro Hara
Comment 13
2012-06-03 03:37:13 PDT
Comment on
attachment 145478
[details]
Patch Thanks for the clarification. The patch looks OK.
WebKit Review Bot
Comment 14
2012-06-03 04:25:09 PDT
Attachment 145478
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/dom/WheelEvent.h:34: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Dominic Cooney
Comment 15
2012-06-03 06:34:03 PDT
Comment on
attachment 145478
[details]
Patch Thanks for reviewing.
Kentaro Hara
Comment 16
2012-06-03 06:35:35 PDT
(In reply to
comment #15
)
> (From update of
attachment 145478
[details]
) > Thanks for reviewing.
Maybe you need to land it manually to avoid the style-bot error.
WebKit Review Bot
Comment 17
2012-06-03 07:33:45 PDT
Comment on
attachment 145478
[details]
Patch Clearing flags on attachment: 145478 Committed
r119359
: <
http://trac.webkit.org/changeset/119359
>
WebKit Review Bot
Comment 18
2012-06-03 07:33:51 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 19
2012-06-03 08:42:29 PDT
Re-opened since this is blocked by 88189
Dominic Cooney
Comment 20
2012-06-03 09:26:32 PDT
Created
attachment 145484
[details]
Patch
WebKit Review Bot
Comment 21
2012-06-03 10:30:15 PDT
Attachment 145484
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/dom/WheelEvent.h:34: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Review Bot
Comment 22
2012-06-03 11:03:14 PDT
Comment on
attachment 145484
[details]
Patch
Attachment 145484
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/12890214
New failing tests: http/tests/media/media-source/video-media-source-event-attributes.html
WebKit Review Bot
Comment 23
2012-06-03 11:03:19 PDT
Created
attachment 145489
[details]
Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Ryosuke Niwa
Comment 24
2012-06-03 15:18:30 PDT
It appears that this patch also broke a couple of browser tests:
http://build.chromium.org/p/chromium.webkit/builders/Win%20%28dbg%29/builds/6453
Dominic Cooney
Comment 25
2012-06-03 18:08:52 PDT
Comment on
attachment 145484
[details]
Patch Clearing r? while I look at browser test failures.
Dominic Cooney
Comment 26
2012-06-25 22:21:50 PDT
Created
attachment 149453
[details]
Patch
Kentaro Hara
Comment 27
2012-06-25 22:23:59 PDT
What was the cause of the browser test failure? (How did the latest patch fix it?)
WebKit Review Bot
Comment 28
2012-06-25 22:25:17 PDT
Attachment 149453
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/dom/WheelEvent.h:34: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Kentaro Hara
Comment 29
2012-06-25 23:14:01 PDT
Comment on
attachment 149453
[details]
Patch
> What was the cause of the browser test failure? (How did the latest patch fix it?)
Discussed with dominicc offline. WheelEvent::isMouseEvent() returns false in the latest patch. Looks OK.
WebKit Review Bot
Comment 30
2012-06-26 00:41:50 PDT
Comment on
attachment 149453
[details]
Patch Clearing flags on attachment: 149453 Committed
r121234
: <
http://trac.webkit.org/changeset/121234
>
WebKit Review Bot
Comment 31
2012-06-26 00:41:57 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug