[Webkit-unassigned] [Bug 135682] Runtime switch for long mouse press gesture. Part of 135257 - Add long mouse press gesture

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=135682





--- Comment #4 from Peyton Randolph <prandolph at apple.com>  2014-08-12 14:24:43 PST ---
(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
> > +2014-08-12  Peyton Randolph  <prandolph at apple.com>
> 
> 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
> > +2014-08-12  Peyton Randolph  <prandolph at apple.com>
> 
> 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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.