Bug 143569 - Add force property to MouseEvents
Summary: Add force property to MouseEvents
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-09 11:43 PDT by Beth Dakin
Modified: 2015-04-13 12:19 PDT (History)
10 users (show)

See Also:


Attachments
Patch (59.07 KB, patch)
2015-04-09 11:58 PDT, Beth Dakin
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-mavericks (563.10 KB, application/zip)
2015-04-09 13:05 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (618.46 KB, application/zip)
2015-04-09 13:13 PDT, Build Bot
no flags Details
Patch (65.64 KB, patch)
2015-04-09 13:52 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
Patch (66.79 KB, patch)
2015-04-09 15:26 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
Patch that also adds force to PlatformMouseEvent (83.61 KB, patch)
2015-04-11 12:20 PDT, Beth Dakin
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2015-04-09 11:43:32 PDT
Right now, WebKitMouseForceEvent inherits from MouseEvent and includes a force property. Instead, force should be a property on all mouse events.

rdar://problem/20472954
Comment 1 Beth Dakin 2015-04-09 11:58:05 PDT
Created attachment 250451 [details]
Patch
Comment 2 WebKit Commit Bot 2015-04-09 11:59:29 PDT
Attachment 250451 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/Element.cpp:2279:  Missing space after ,  [whitespace/comma] [3]
ERROR: Source/WebCore/dom/MouseEvent.cpp:117:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MouseEvent.cpp:128:  Wrong number of spaces before statement. (expected: 27)  [whitespace/indent] [4]
ERROR: Source/WebCore/dom/MouseEvent.cpp:276:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 4 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Build Bot 2015-04-09 13:05:28 PDT
Comment on attachment 250451 [details]
Patch

Attachment 250451 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6034319655043072

New failing tests:
js/dom/global-constructors-attributes.html
Comment 4 Build Bot 2015-04-09 13:05:32 PDT
Created attachment 250453 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 5 Build Bot 2015-04-09 13:13:31 PDT
Comment on attachment 250451 [details]
Patch

Attachment 250451 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4670016649691136

New failing tests:
js/dom/global-constructors-attributes.html
Comment 6 Build Bot 2015-04-09 13:13:35 PDT
Created attachment 250455 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 7 Beth Dakin 2015-04-09 13:52:43 PDT
Created attachment 250465 [details]
Patch
Comment 8 WebKit Commit Bot 2015-04-09 13:54:49 PDT
Attachment 250465 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/MouseEvent.cpp:117:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MouseEvent.cpp:128:  Wrong number of spaces before statement. (expected: 27)  [whitespace/indent] [4]
ERROR: Source/WebCore/dom/MouseEvent.cpp:276:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 3 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Beth Dakin 2015-04-09 15:26:28 PDT
Created attachment 250472 [details]
Patch
Comment 10 WebKit Commit Bot 2015-04-09 15:28:36 PDT
Attachment 250472 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/MouseEvent.cpp:117:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MouseEvent.cpp:128:  Wrong number of spaces before statement. (expected: 27)  [whitespace/indent] [4]
ERROR: Source/WebCore/dom/MouseEvent.cpp:276:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 3 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Tim Horton 2015-04-10 12:14:56 PDT
Comment on attachment 250472 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1092
> +    RefPtr<MouseEvent> mouseEvent = MouseEvent::create(eventNames().clickEvent, true, true, currentTime(), nullptr, singleClick, screenPoint.x(), screenPoint.y(), documentPoint.x(), documentPoint.y(), false, false, false, false, 0, nullptr, 0, nullptr);

Can we improve this? We discussed a zero-force constant to make these things better.
Comment 12 Beth Dakin 2015-04-11 12:20:04 PDT
Created attachment 250577 [details]
Patch that also adds force to PlatformMouseEvent

Simon thought it would make more sense to add force to PlatformMouseEvent in addition to adding it to MouseEvent. It is an AppKit implementation detail that force is not a part of the Mac platform mouse events, but Simon feels it should still be a part of PlatformMouseEvent.
Comment 13 WebKit Commit Bot 2015-04-11 12:23:04 PDT
Attachment 250577 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/PlatformMouseEvent.h:69:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/PlatformMouseEvent.h:76:  Wrong number of spaces before statement. (expected: 31)  [whitespace/indent] [4]
ERROR: Source/WebCore/page/DragController.cpp:96:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MouseEvent.cpp:117:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MouseEvent.cpp:128:  Wrong number of spaces before statement. (expected: 27)  [whitespace/indent] [4]
ERROR: Source/WebCore/dom/MouseEvent.cpp:276:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 6 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 Darin Adler 2015-04-12 13:03:06 PDT
Comment on attachment 250577 [details]
Patch that also adds force to PlatformMouseEvent

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

Looks fine. Broke the iOS and Windows builds, though.

> Source/WebCore/platform/PlatformMouseEvent.h:47
> +static const double ForceAtClick = 0;
> +static const double ForceAtForceClick = 1;

These should not be marked static, since they are in a header file.

> Source/WebKit2/Shared/mac/WebEventFactory.mm:378
> +#if __MAC_OS_X_VERSION_MAX_ALLOWED >= 101003

I don’t think MAX_ALLOWED is the right way to check this. Is it?
Comment 15 Beth Dakin 2015-04-13 12:10:10 PDT
Thanks Darin!

(In reply to comment #14)
> Comment on attachment 250577 [details]
> Patch that also adds force to PlatformMouseEvent
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=250577&action=review
> 
> Looks fine. Broke the iOS and Windows builds, though.
> 

I think I have those fixed now.

> > Source/WebCore/platform/PlatformMouseEvent.h:47
> > +static const double ForceAtClick = 0;
> > +static const double ForceAtForceClick = 1;
> 
> These should not be marked static, since they are in a header file.
> 

Fixed!

> > Source/WebKit2/Shared/mac/WebEventFactory.mm:378
> > +#if __MAC_OS_X_VERSION_MAX_ALLOWED >= 101003
> 
> I don’t think MAX_ALLOWED is the right way to check this. Is it?

Actually, I think it is. I did work through this with Dan and Tim. It seems like the dot releases, i.e. 101003, are only defined in the context of MAX_ALLOWED, not the usual MIN_REQUIRED.
Comment 16 Beth Dakin 2015-04-13 12:19:21 PDT
http://trac.webkit.org/changeset/182748