Bug 143387 – Improvements to webkitmouseforce web API

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

[Beth Dakin]

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

[Beth Dakin]

Created attachment 250099 [details]
Patch

[Tim Horton]

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?

[Beth Dakin]

(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.

[Tim Horton]

(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.

:(

[Beth Dakin]

Created attachment 250101 [details]
Patch

[Beth Dakin]

Created attachment 250103 [details]
Patch

[Darin Adler]

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?

[Beth Dakin]

(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!

[Beth Dakin]

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

[Beth Dakin]

Follow-on build fixes https://trac.webkit.org/changeset/182341 and http://trac.webkit.org/changeset/182435