Bug 135682 – Runtime switch for long mouse press gesture. Part of 135257 - Add long mouse press gesture

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

[Peyton Randolph]

Since the long mouse press changes certain interactions with the page, it would be nice to have a runtime switch so clients can opt into the long mouse press.

[Peyton Randolph]

Created attachment 236159 [details]
Patch

[Peyton Randolph]

Created attachment 236466 [details]
Patch

[Tim Horton]

Comment on attachment 236466 [details]
Patch

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

> Source/WebCore/ChangeLog:1
> [phone]  Peyton Randolph  <[e-mail]>

What's up with the duplicate changelog?

> Source/WebCore/page/EventHandler.cpp:1572
> +        clearLongMousePressState();

Seems like you're doing the clear in either case; can this be simplified?

> Source/WebCore/page/EventHandler.cpp:1589
> +        clearLongMousePressState();

Is this all here to make the runtime switch dynamically switchable in the middle of a press? I think it's sufficient if you can't get IN to a long press, you don't need to check in each step once it's started.

> Source/WebCore/page/EventHandler.cpp:1609
> +        clearLongMousePressState();

Another duplicate clear :(

[Peyton Randolph]

(In reply to comment #3)
> (From update of attachment 236466 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236466&action=review
> 
> > Source/WebCore/ChangeLog:1
> > [phone]  Peyton Randolph  <[e-mail]>
> 
> What's up with the duplicate changelog?
> 
> > Source/WebCore/page/EventHandler.cpp:1572
> > +        clearLongMousePressState();
> 
> Seems like you're doing the clear in either case; can this be simplified?
> 
> > Source/WebCore/page/EventHandler.cpp:1589
> > +        clearLongMousePressState();
> 
> Is this all here to make the runtime switch dynamically switchable in the middle of a press? I think it's sufficient if you can't get IN to a long press, you don't need to check in each step once it's started.
> 
> > Source/WebCore/page/EventHandler.cpp:1609
> > +        clearLongMousePressState();
> 
> Another duplicate clear :(

(In reply to comment #3)
> (From update of attachment 236466 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236466&action=review
> 
> > Source/WebCore/ChangeLog:1
> > [phone]  Peyton Randolph  <[e-mail]>
> 
> What's up with the duplicate changelog?

Removed. Had two editors open at the same time.
> 
> > Source/WebCore/page/EventHandler.cpp:1572
> > +        clearLongMousePressState();
> 
> Seems like you're doing the clear in either case; can this be simplified?

Refactored by moving the clear to before the if statement.

> 
> > Source/WebCore/page/EventHandler.cpp:1589
> > +        clearLongMousePressState();
> 
> Is this all here to make the runtime switch dynamically switchable in the middle of a press? I think it's sufficient if you can't get IN to a long press, you don't need to check in each step once it's started.

The check in recognizeLongMousePress isn't strictly necessary, but the check in cancelTrackingPotentialLongMousePress is because it gets called whenever a drag is recognized.

I removed the check in recognizeLongMousePress.

> 
> > Source/WebCore/page/EventHandler.cpp:1609
> > +        clearLongMousePressState();
> 
> Another duplicate clear :(

Refactored.

[Peyton Randolph]

Created attachment 236468 [details]
Patch

[Tim Horton]

Comment on attachment 236468 [details]
Patch

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

> Source/WebCore/page/EventHandler.cpp:1606
> +    if (!(page && page->settings().longMousePressEnabled()))

You still don't really need this because it's crazy to worry about the case where the timer started and then the preference was turned off and then the cancel comes through, but I don't really care either way.

[Peyton Randolph]

Created attachment 236472 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 236472 [details]
Patch

Clearing flags on attachment: 236472

Committed r172503: <http://trac.webkit.org/changeset/172503>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.