Bug 143387 - Improvements to webkitmouseforce web API
Summary: Improvements to webkitmouseforce web API
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-03 14:36 PDT by Beth Dakin
Modified: 2015-04-06 13:28 PDT (History)
7 users (show)

See Also:


Attachments
Patch (10.05 KB, patch)
2015-04-03 14:48 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
Patch (9.94 KB, patch)
2015-04-03 15:11 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
Patch (10.12 KB, patch)
2015-04-03 15:44 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-03 14:36:37 PDT
This bug tracks the following improvements to the webkitmouseforce web API:

1. Need to dispatch webkitmouseforceup and webkitmouseforceclick
2. Need to dispatch webkitmouseforcechanged through mouseup
3. Should use pressure values from pressureChangedWithEvent instead of the immediate action gesture recognizer.

rdar://problem/20281853
rdar://problem/20281808
Comment 1 Beth Dakin 2015-04-03 14:48:11 PDT
Created attachment 250099 [details]
Patch
Comment 2 Tim Horton 2015-04-03 14:51:51 PDT
Comment on attachment 250099 [details]
Patch

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

> Source/WebKit2/UIProcess/WebPageProxy.h:992
> +    void pressureDidChange(float force, int stage);

Some indication of what kind of pressure or what you're talking about at all might be good :D mousePressureDidChange? No idea.

Also why is it pressure and force?

> Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:1171
> +    if (m_lastPressureEventForceStage == 1 && stage == 2)

Isn't the negativity of stageTransition how you're supposed to figure out which direction you're going? Can we totally avoid caching the old one in that case?
Comment 3 Beth Dakin 2015-04-03 14:54:09 PDT
(In reply to comment #2)
> Comment on attachment 250099 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=250099&action=review
> 
> > Source/WebKit2/UIProcess/WebPageProxy.h:992
> > +    void pressureDidChange(float force, int stage);
> 
> Some indication of what kind of pressure or what you're talking about at all
> might be good :D mousePressureDidChange? No idea.
> 
> Also why is it pressure and force?
> 

The NSResponder method uses the term "pressure," which is why I used it here. Maybe we shouldn't let that spread into our code? But for the time being I used it to keep it more directly tied to the NSResponder method of that name.

> > Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:1171
> > +    if (m_lastPressureEventForceStage == 1 && stage == 2)
> 
> Isn't the negativity of stageTransition how you're supposed to figure out
> which direction you're going? Can we totally avoid caching the old one in
> that case?

stageTransition is not consistent for this purpose as far as I can tell.
Comment 4 Tim Horton 2015-04-03 14:56:32 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > Comment on attachment 250099 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=250099&action=review
> > 
> > > Source/WebKit2/UIProcess/WebPageProxy.h:992
> > > +    void pressureDidChange(float force, int stage);
> > 
> > Some indication of what kind of pressure or what you're talking about at all
> > might be good :D mousePressureDidChange? No idea.
> > 
> > Also why is it pressure and force?
> > 
> 
> The NSResponder method uses the term "pressure," which is why I used it
> here. Maybe we shouldn't let that spread into our code? But for the time
> being I used it to keep it more directly tied to the NSResponder method of
> that name.

Got it! Should the argument be 'pressure', then?

> > > Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:1171
> > > +    if (m_lastPressureEventForceStage == 1 && stage == 2)
> > 
> > Isn't the negativity of stageTransition how you're supposed to figure out
> > which direction you're going? Can we totally avoid caching the old one in
> > that case?
> 
> stageTransition is not consistent for this purpose as far as I can tell.

:(
Comment 5 Beth Dakin 2015-04-03 15:11:01 PDT
Created attachment 250101 [details]
Patch
Comment 6 Beth Dakin 2015-04-03 15:44:22 PDT
Created attachment 250103 [details]
Patch
Comment 7 Darin Adler 2015-04-03 16:30:29 PDT
Comment on attachment 250103 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:345
> +    , m_lastForceStage(0)

No need to initialize here if you take my suggestion below.

> Source/WebKit2/WebProcess/WebPage/WebPage.h:1348
> +    int m_lastForceStage;

Better to initialize this here:

    int m_lastForceStage { 0 };

or even:

    int m_lastForceStage { };

> Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:1168
> +    float overallForce = stage < 1 ? force : force + stage - 1;

I guess the force number is in the range [0,1]? Should we assert that?
Comment 8 Beth Dakin 2015-04-03 16:44:17 PDT
(In reply to comment #7)
> Comment on attachment 250103 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=250103&action=review
> 
> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:345
> > +    , m_lastForceStage(0)
> 
> No need to initialize here if you take my suggestion below.
> 
> > Source/WebKit2/WebProcess/WebPage/WebPage.h:1348
> > +    int m_lastForceStage;
> 
> Better to initialize this here:
> 
>     int m_lastForceStage { 0 };
> 
> or even:
> 
>     int m_lastForceStage { };
> 

Will do!

> > Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:1168
> > +    float overallForce = stage < 1 ? force : force + stage - 1;
> 
> I guess the force number is in the range [0,1]? Should we assert that?

Right now the force ranges from 0-2. Stage 0 represents the stage between touching the trackpad and mousedown, but AppKit does not send us any force information in that time except for 0 and 1 to represent exiting or entering that stage. 

Then stage 1 is between mousedown and mouseforcedown. We get force in a range from 0-1 there.

And stage 2 is mouseforcedown and beyond. We get force in a range from 0-1 there as well.

So right now we expose force from 0-2 where 0 represents anything before mousedown or after mouseup, and 1 corresponds to forcemousedown/up. 2 is the theoretical hardest you can press.

I don't think we want to assert anything at this time, because all of this could change. We're figuring out the best way to massage the platform events into something that makes sense for a web API, and it's an ongoing discussion.

Thanks Darin!
Comment 9 Beth Dakin 2015-04-03 16:53:46 PDT
http://trac.webkit.org/changeset/182338
Comment 10 Beth Dakin 2015-04-06 13:28:44 PDT
Follow-on build fixes https://trac.webkit.org/changeset/182341 and http://trac.webkit.org/changeset/182435