Bug 142836 - Add events related to force click gesture
Summary: Add events related to force click gesture
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Event Handling (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-18 13:19 PDT by Beth Dakin
Modified: 2015-03-24 14:43 PDT (History)
10 users (show)

See Also:


Attachments
Patch (44.16 KB, patch)
2015-03-18 14:16 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
Patch (44.66 KB, patch)
2015-03-18 14:26 PDT, Beth Dakin
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-mavericks (623.08 KB, application/zip)
2015-03-18 15:42 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-mavericks-wk2 (584.46 KB, application/zip)
2015-03-18 15:49 PDT, Build Bot
no flags Details
Patch (55.25 KB, patch)
2015-03-19 12:24 PDT, Beth Dakin
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (587.59 KB, application/zip)
2015-03-19 13:17 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews102 for mac-mavericks (560.40 KB, application/zip)
2015-03-19 13:44 PDT, Build Bot
no flags Details
Patch (55.96 KB, patch)
2015-03-19 13:50 PDT, Beth Dakin
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (589.70 KB, application/zip)
2015-03-19 14:44 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews102 for mac-mavericks (629.18 KB, application/zip)
2015-03-19 15:09 PDT, Build Bot
no flags Details
Patch (40.78 KB, patch)
2015-03-20 11:31 PDT, Beth Dakin
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-mavericks (626.43 KB, application/zip)
2015-03-20 12:40 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (673.84 KB, application/zip)
2015-03-20 12:44 PDT, Build Bot
no flags Details
Patch (68.48 KB, patch)
2015-03-23 12:55 PDT, Beth Dakin
dino: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-mavericks (561.11 KB, application/zip)
2015-03-23 14:15 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-mavericks-wk2 (580.90 KB, application/zip)
2015-03-23 14:18 PDT, Build Bot
no flags Details
Patch for bots (69.59 KB, patch)
2015-03-24 13:32 PDT, Beth Dakin
no flags 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-03-18 13:19:33 PDT
This bug tracks exposing the force click gesture on the new ForceTouch trackpad to web content.

rdar://problem/20210239
Comment 1 Beth Dakin 2015-03-18 14:16:08 PDT
Created attachment 248964 [details]
Patch

This patch leaves three follow-up bugs:

1. The patch adds the mouseforceup event but does not yet fire it.
2. The patch adds the mouseforceclick event but does not yet fire it.
3. The patch only sends mouseforcechanged between mousedown and mouseforcedown, but we do intend to send it when force changes until mouseup.
Comment 2 Beth Dakin 2015-03-18 14:26:47 PDT
Created attachment 248965 [details]
Patch

This patch should apply to trunk.
Comment 3 WebKit Commit Bot 2015-03-18 14:28:38 PDT
Attachment 248965 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/Element.h:472:  The parameter name "platformMouseEvent" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:51:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/dom/WebKitMouseForceChangedEvent.h:62:  The parameter name "event" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/dom/WebKitMouseForceChangedEvent.h:62:  The parameter name "view" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 5 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Build Bot 2015-03-18 15:42:52 PDT
Comment on attachment 248965 [details]
Patch

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

New failing tests:
js/dom/global-constructors-attributes.html
Comment 5 Build Bot 2015-03-18 15:42:57 PDT
Created attachment 248976 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 6 Build Bot 2015-03-18 15:49:30 PDT
Comment on attachment 248965 [details]
Patch

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

New failing tests:
js/dom/global-constructors-attributes.html
Comment 7 Build Bot 2015-03-18 15:49:35 PDT
Created attachment 248977 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 8 Beth Dakin 2015-03-19 12:24:01 PDT
Created attachment 249046 [details]
Patch
Comment 9 Beth Dakin 2015-03-19 12:24:19 PDT
Hopefully this patch will make for greener bots.
Comment 10 WebKit Commit Bot 2015-03-19 12:27:50 PDT
Attachment 249046 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:51:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 36 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Build Bot 2015-03-19 13:17:37 PDT
Comment on attachment 249046 [details]
Patch

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

New failing tests:
js/dom/global-constructors-attributes.html
Comment 12 Build Bot 2015-03-19 13:17:40 PDT
Created attachment 249055 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 13 Build Bot 2015-03-19 13:44:35 PDT
Comment on attachment 249046 [details]
Patch

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

New failing tests:
js/dom/global-constructors-attributes.html
Comment 14 Build Bot 2015-03-19 13:44:40 PDT
Created attachment 249058 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 15 Beth Dakin 2015-03-19 13:50:20 PDT
Created attachment 249059 [details]
Patch

Another go at the bots.
Comment 16 WebKit Commit Bot 2015-03-19 13:53:27 PDT
Attachment 249059 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:51:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 Build Bot 2015-03-19 14:44:04 PDT
Comment on attachment 249059 [details]
Patch

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

New failing tests:
js/dom/global-constructors-attributes.html
Comment 18 Build Bot 2015-03-19 14:44:08 PDT
Created attachment 249061 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 19 Build Bot 2015-03-19 15:09:33 PDT
Comment on attachment 249059 [details]
Patch

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

New failing tests:
js/dom/global-constructors-attributes.html
Comment 20 Build Bot 2015-03-19 15:09:37 PDT
Created attachment 249064 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 21 Beth Dakin 2015-03-20 11:31:09 PDT
Created attachment 249124 [details]
Patch
Comment 22 WebKit Commit Bot 2015-03-20 11:34:10 PDT
Attachment 249124 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:51:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 23 Build Bot 2015-03-20 12:40:41 PDT
Comment on attachment 249124 [details]
Patch

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

New failing tests:
js/dom/global-constructors-attributes.html
Comment 24 Build Bot 2015-03-20 12:40:45 PDT
Created attachment 249130 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 25 Build Bot 2015-03-20 12:44:24 PDT
Comment on attachment 249124 [details]
Patch

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

New failing tests:
js/dom/global-constructors-attributes.html
Comment 26 Build Bot 2015-03-20 12:44:29 PDT
Created attachment 249131 [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 27 Tim Horton 2015-03-20 13:11:04 PDT
Comment on attachment 249124 [details]
Patch

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

> Source/WebCore/dom/Element.cpp:2137
> +#if ENABLE(MOUSE_FORCE_EVENTS)

Is there a reason we don't just disappear these functions if this isn't enabled?

> Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:32
> +

Where's the #if ENABLE(MOUSE_FORCE_EVENTS)?

> Source/WebCore/page/DOMWindow.idl:272
> +    [NotEnumerable] attribute EventHandler onwebkitmouseforcecancelled;

should these have Conditional=MOUSE_FORCE_EVENT?
Comment 28 Beth Dakin 2015-03-20 14:40:01 PDT
(In reply to comment #27)
> Comment on attachment 249124 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249124&action=review
> 
> > Source/WebCore/dom/Element.cpp:2137
> > +#if ENABLE(MOUSE_FORCE_EVENTS)
> 
> Is there a reason we don't just disappear these functions if this isn't
> enabled?
> 

Not necessarily a good one! I was just trying to minimize the number of spots I put the #ifs since they make code harder to read, but I am open to anything really.

> > Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:32
> > +
> 
> Where's the #if ENABLE(MOUSE_FORCE_EVENTS)?
> 

That does seem like an oversight.

> > Source/WebCore/page/DOMWindow.idl:272
> > +    [NotEnumerable] attribute EventHandler onwebkitmouseforcecancelled;
> 
> should these have Conditional=MOUSE_FORCE_EVENT?

SO!!! I am glad you asked, because I am really confused about this. Earlier versions of my patch included this. However, in http://trac.webkit.org/changeset/181507 Darin removed all conditionals from DOMWindow.idl, and I am very confused about why. I asked Anders, who reviewed the patch, and he wasn't totally sure about it either. I should really ask Darin. Oh, I will cc him!
Comment 29 Beth Dakin 2015-03-20 14:41:48 PDT
> > > Source/WebCore/page/DOMWindow.idl:272
> > > +    [NotEnumerable] attribute EventHandler onwebkitmouseforcecancelled;
> > 
> > should these have Conditional=MOUSE_FORCE_EVENT?
> 
> SO!!! I am glad you asked, because I am really confused about this. Earlier
> versions of my patch included this. However, in
> http://trac.webkit.org/changeset/181507 Darin removed all conditionals from
> DOMWindow.idl, and I am very confused about why. I asked Anders, who
> reviewed the patch, and he wasn't totally sure about it either. I should
> really ask Darin. Oh, I will cc him!

So to continue this thought…I didn't add the Conditional= part because Darin's patch seemed to imply that the new normal is to exclude it? But I confess that I am not clear on how the new normal prevents us from exposing all of these features to the web…
Comment 30 Darin Adler 2015-03-20 14:47:06 PDT
Comment on attachment 249124 [details]
Patch

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

>>> Source/WebCore/dom/Element.cpp:2137
>>> +#if ENABLE(MOUSE_FORCE_EVENTS)
>> 
>> Is there a reason we don't just disappear these functions if this isn't enabled?
> 
> Not necessarily a good one! I was just trying to minimize the number of spots I put the #ifs since they make code harder to read, but I am open to anything really.

My favorite pattern for this sort of thing is to have empty functions, inlined if necessary, to keep the #if statements out of all the call sites. I prefer separate functions because I don’t like all the UNUSED_PARAM you need if the conditionals are inside the functions, and also because it makes it easier to put them inlined and in headers.

> Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:36
> +    : force(0)

better to use C++11 member initialization syntax:

    double force { 0 };

Then we don’t need to declare or define a constructor at all.

> Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:41
> +    : m_force(0)

Same story here:

    double m_force { 0 };

Although in this case we should probably leave the default constructor, but just remove the explicit m_force initialization.

> Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:63
> +WebKitMouseForceChangedEvent::~WebKitMouseForceChangedEvent()
> +{
> +}

Seems better to just not declare or define this and let the compiler take care of it.

> Source/WebCore/dom/WebKitMouseForceChangedEvent.h:56
> +    virtual EventInterface eventInterface() const override;

Can this be private?

> Source/WebCore/dom/WebKitMouseForceChangedEvent.idl:29
> +    [InitializedByEventConstructor] readonly attribute float force;

It seems strange that this is float in the IDL and double in the code. Can it be double here too?

> Source/WebCore/html/HTMLAttributeNames.in:278
> +onwebkitmouseforcewillbegin
> +onwebkitmouseforceup

Better to keep these sorted in alphabetical order.

> Source/WebCore/html/HTMLBodyElement.cpp:125
> +        &onwebkitmouseforcewillbeginAttr,
> +        &onwebkitmouseforceupAttr,

Better to keep these sorted in alphabetical order.

>>> Source/WebCore/page/DOMWindow.idl:272
>>> +    [NotEnumerable] attribute EventHandler onwebkitmouseforcecancelled;
>> 
>> should these have Conditional=MOUSE_FORCE_EVENT?
> 
> SO!!! I am glad you asked, because I am really confused about this. Earlier versions of my patch included this. However, in http://trac.webkit.org/changeset/181507 Darin removed all conditionals from DOMWindow.idl, and I am very confused about why. I asked Anders, who reviewed the patch, and he wasn't totally sure about it either. I should really ask Darin. Oh, I will cc him!

It’s correct to include these unconditionally. We might decide some day we want to compile them out, but I suggest a pattern where the events are only there if needed, but the event handler machinery is always present.

However, you should add coverage for these new event handlers to the Layout test named fast/dom/event-handler-attributes.html
Comment 31 Darin Adler 2015-03-20 14:49:41 PDT
(In reply to comment #29)
> So to continue this thought…I didn't add the Conditional= part because
> Darin's patch seemed to imply that the new normal is to exclude it? But I
> confess that I am not clear on how the new normal prevents us from exposing
> all of these features to the web…

The feature this exposes is that if you set an attribute named onfoo and then send an event foo, the code in that attribute will run. I think it’s harmless to expose that to the web and better for testing to not have it be conditional.

As far as I know this doesn’t affect any technique for testing for the existence of a particular event, nor does it cause the engine to send events. A value for such an attribute would not be entirely ignored, but it would effectively be ignored. The handler would never be called because the event would never be dispatched.
Comment 32 Beth Dakin 2015-03-20 15:07:06 PDT
(In reply to comment #31)
> (In reply to comment #29)
> > So to continue this thought…I didn't add the Conditional= part because
> > Darin's patch seemed to imply that the new normal is to exclude it? But I
> > confess that I am not clear on how the new normal prevents us from exposing
> > all of these features to the web…
> 
> The feature this exposes is that if you set an attribute named onfoo and
> then send an event foo, the code in that attribute will run. I think it’s
> harmless to expose that to the web and better for testing to not have it be
> conditional.
> 
> As far as I know this doesn’t affect any technique for testing for the
> existence of a particular event, nor does it cause the engine to send
> events. A value for such an attribute would not be entirely ignored, but it
> would effectively be ignored. The handler would never be called because the
> event would never be dispatched.

Cool, thank you!
Comment 33 Beth Dakin 2015-03-23 12:34:49 PDT
(In reply to comment #30)
> Comment on attachment 249124 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249124&action=review
> 
> >>> Source/WebCore/dom/Element.cpp:2137
> >>> +#if ENABLE(MOUSE_FORCE_EVENTS)
> >> 
> >> Is there a reason we don't just disappear these functions if this isn't enabled?
> > 
> > Not necessarily a good one! I was just trying to minimize the number of spots I put the #ifs since they make code harder to read, but I am open to anything really.
> 
> My favorite pattern for this sort of thing is to have empty functions,
> inlined if necessary, to keep the #if statements out of all the call sites.
> I prefer separate functions because I don’t like all the UNUSED_PARAM you
> need if the conditionals are inside the functions, and also because it makes
> it easier to put them inlined and in headers.
> 

I added separate functions, but for the time being, I added them to the implementation file rather than the header file. I did that because these functions need to be marked WEBCORE_EXPORT, and I think that WEBCORE_EXPORT functions cannot be inlined without breaking some of the bots. If this is an incorrect understanding, I will fix. (I'll chat with Alex to confirm.)

> > Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:36
> > +    : force(0)
> 
> better to use C++11 member initialization syntax:
> 
>     double force { 0 };
> 
> Then we don’t need to declare or define a constructor at all.
> 
> > Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:41
> > +    : m_force(0)
> 
> Same story here:
> 
>     double m_force { 0 };
> 
> Although in this case we should probably leave the default constructor, but
> just remove the explicit m_force initialization.
> 

Fixed all of this. 

> > Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:63
> > +WebKitMouseForceChangedEvent::~WebKitMouseForceChangedEvent()
> > +{
> > +}
> 
> Seems better to just not declare or define this and let the compiler take
> care of it.
> 

Removed this.

> > Source/WebCore/dom/WebKitMouseForceChangedEvent.h:56
> > +    virtual EventInterface eventInterface() const override;
> 
> Can this be private?
> 

Looks like it can! Done.

> > Source/WebCore/dom/WebKitMouseForceChangedEvent.idl:29
> > +    [InitializedByEventConstructor] readonly attribute float force;
> 
> It seems strange that this is float in the IDL and double in the code. Can
> it be double here too?
> 

Yes it can. Changed to a double in the idl.

> > Source/WebCore/html/HTMLAttributeNames.in:278
> > +onwebkitmouseforcewillbegin
> > +onwebkitmouseforceup
> 
> Better to keep these sorted in alphabetical order.
> 

Fixed.

> > Source/WebCore/html/HTMLBodyElement.cpp:125
> > +        &onwebkitmouseforcewillbeginAttr,
> > +        &onwebkitmouseforceupAttr,
> 
> Better to keep these sorted in alphabetical order.
> 

Fixed.

> >>> Source/WebCore/page/DOMWindow.idl:272
> >>> +    [NotEnumerable] attribute EventHandler onwebkitmouseforcecancelled;
> >> 
> >> should these have Conditional=MOUSE_FORCE_EVENT?
> > 
> > SO!!! I am glad you asked, because I am really confused about this. Earlier versions of my patch included this. However, in http://trac.webkit.org/changeset/181507 Darin removed all conditionals from DOMWindow.idl, and I am very confused about why. I asked Anders, who reviewed the patch, and he wasn't totally sure about it either. I should really ask Darin. Oh, I will cc him!
> 
> It’s correct to include these unconditionally. We might decide some day we
> want to compile them out, but I suggest a pattern where the events are only
> there if needed, but the event handler machinery is always present.
> 
> However, you should add coverage for these new event handlers to the Layout
> test named fast/dom/event-handler-attributes.html

Will do!

New patch coming.
Comment 34 Beth Dakin 2015-03-23 12:55:43 PDT
Created attachment 249252 [details]
Patch
Comment 35 WebKit Commit Bot 2015-03-23 12:59:02 PDT
Attachment 249252 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/WebKitMouseForceChangedEvent.cpp:45:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 39 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 36 Tim Horton 2015-03-23 13:03:56 PDT
Comment on attachment 249252 [details]
Patch

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

> Source/WebCore/ChangeLog:23
> +

random extra line here.

> Source/WebKit2/Shared/mac/ActionMenuHitTestResult.h:68
> +    bool hitTestResultPreventsDefault; 

This name is not fantastic. Maybe contentPreventedDefault? or scriptPreventedDefault? or... I don't know. Those don't seem great either. Maybe just "preventDefaultActions"?
Comment 37 Beth Dakin 2015-03-23 13:15:30 PDT
(In reply to comment #36)
> Comment on attachment 249252 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249252&action=review
> 
> > Source/WebCore/ChangeLog:23
> > +
> 
> random extra line here.
> 

Fixed!

> > Source/WebKit2/Shared/mac/ActionMenuHitTestResult.h:68
> > +    bool hitTestResultPreventsDefault; 
> 
> This name is not fantastic. Maybe contentPreventedDefault? or
> scriptPreventedDefault? or... I don't know. Those don't seem great either.
> Maybe just "preventDefaultActions"?

I do think that contentPreventedDefault, so I will switch to that.
Comment 38 Build Bot 2015-03-23 14:15:36 PDT
Comment on attachment 249252 [details]
Patch

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

New failing tests:
js/dom/global-constructors-attributes.html
Comment 39 Build Bot 2015-03-23 14:15:41 PDT
Created attachment 249270 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 40 Build Bot 2015-03-23 14:17:57 PDT
Comment on attachment 249252 [details]
Patch

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

New failing tests:
js/dom/global-constructors-attributes.html
Comment 41 Build Bot 2015-03-23 14:18:02 PDT
Created attachment 249271 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 42 Dean Jackson 2015-03-23 14:27:23 PDT
Comment on attachment 249252 [details]
Patch

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

Are we going to update testRunner to be able to generate these mouse events?

> Source/WebCore/ChangeLog:15
> +        webkitmouseforcewillbegin -> Event is sent just before mousedown to indicate that 
> +        force can be perceived if the user presses any harder. The author should prevent 
> +        default on this event to both prevent the user agentâs default force click 
> +        features and to receive the other 5 events.

It's interesting that you have to prevent default here in order to get the other events. I'm not sure we have anything else like that.

> Source/WebCore/ChangeLog:45
> +        Add new files for WebKitMouseForceChangedEvent to build systems.
> +        * DerivedSources.cpp:
> +        * DerivedSources.make:
> +        * WebCore.vcxproj/WebCore.vcxproj:
> +        * WebCore.vcxproj/WebCore.vcxproj.filters:
> +        * WebCore.xcodeproj/project.pbxproj:
> +        * WebCore.xcodeproj/project.pbxproj:

No changes to the cmake files?

> Source/WebCore/dom/Element.h:475
> +    WEBCORE_EXPORT void dispatchMouseForceDown();
> +    WEBCORE_EXPORT void dispatchMouseForceUp();
> +    WEBCORE_EXPORT void dispatchMouseForceClick();

Why don't you need a PlatformMouseEvent here (at least for WillBegin, Down and Cancelled)? Don't we need a target?

> Source/WebCore/dom/EventNames.in:32
>  WebKitAnimationEvent
> +WebKitMouseForceChangedEvent
>  WebKitTransitionEvent

Why is there only one addition here? I expected a WebKitMouseForceDownEvent, WillBegin and Cancelled.
Comment 43 Beth Dakin 2015-03-23 15:25:10 PDT
(In reply to comment #42)
> Comment on attachment 249252 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249252&action=review
> 
> Are we going to update testRunner to be able to generate these mouse events?
> 

Yes! I am planning to hook this up very soon. Will file a follow-up bug.

> > Source/WebCore/ChangeLog:15
> > +        webkitmouseforcewillbegin -> Event is sent just before mousedown to indicate that 
> > +        force can be perceived if the user presses any harder. The author should prevent 
> > +        default on this event to both prevent the user agentâs default force click 
> > +        features and to receive the other 5 events.
> 
> It's interesting that you have to prevent default here in order to get the
> other events. I'm not sure we have anything else like that.
> 

It is a bit weird, but I fear spamming way too many events on web content that simply doesn't care otherwise. I think it's the right trade-off. I'm sure this will get further discussion.

> > Source/WebCore/ChangeLog:45
> > +        Add new files for WebKitMouseForceChangedEvent to build systems.
> > +        * DerivedSources.cpp:
> > +        * DerivedSources.make:
> > +        * WebCore.vcxproj/WebCore.vcxproj:
> > +        * WebCore.vcxproj/WebCore.vcxproj.filters:
> > +        * WebCore.xcodeproj/project.pbxproj:
> > +        * WebCore.xcodeproj/project.pbxproj:
> 
> No changes to the cmake files?
> 

I did edit CMakeLists.txt, but it looks like somehow that missed the Changelog. My build is still failing on Win though, so I'm missing something…

> > Source/WebCore/dom/Element.h:475
> > +    WEBCORE_EXPORT void dispatchMouseForceDown();
> > +    WEBCORE_EXPORT void dispatchMouseForceUp();
> > +    WEBCORE_EXPORT void dispatchMouseForceClick();
> 
> Why don't you need a PlatformMouseEvent here (at least for WillBegin, Down
> and Cancelled)? Don't we need a target?
> 
> > Source/WebCore/dom/EventNames.in:32
> >  WebKitAnimationEvent
> > +WebKitMouseForceChangedEvent
> >  WebKitTransitionEvent
> 
> Why is there only one addition here? I expected a WebKitMouseForceDownEvent,
> WillBegin and Cancelled.

Hmmm, still thinking this over…
Comment 44 Beth Dakin 2015-03-24 13:32:35 PDT
Created attachment 249345 [details]
Patch for bots

This patch should address all of Dean's comments. Uploading a new copy for the bots to chew on before I commit.
Comment 45 WebKit Commit Bot 2015-03-24 13:35:13 PDT
Attachment 249345 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/WebKitMouseForceEvent.cpp:45:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 46 Beth Dakin 2015-03-24 14:32:48 PDT
Committed change: http://trac.webkit.org/changeset/181907