Bug 143569 – Add force property to MouseEvents

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Beth Dakin]

Right now, WebKitMouseForceEvent inherits from MouseEvent and includes a force property. Instead, force should be a property on all mouse events.

rdar://problem/20472954

[Beth Dakin]

Created attachment 250451 [details]
Patch

[WebKit Commit Bot]

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.

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Beth Dakin]

Created attachment 250465 [details]
Patch

[WebKit Commit Bot]

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.

[Beth Dakin]

Created attachment 250472 [details]
Patch

[WebKit Commit Bot]

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.

[Tim Horton]

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.

[Beth Dakin]

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.

[WebKit Commit Bot]

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.

[Darin Adler]

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?

[Beth Dakin]

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.

[Beth Dakin]

http://trac.webkit.org/changeset/182748