[webkit-reviews] review denied: [Bug 101545] LongPress and LongTap gestures should start drag/drop and open context menu respectively. : [Attachment 173036] Patch

[bugzilla-daemon at webkit.org]

Antonio Gomes <tonikitoo at webkit.org> has denied Varun Jain
<varunjain at chromium.org>'s request for review:
Bug 101545: LongPress and LongTap gestures should start drag/drop and open
context menu respectively.
https://bugs.webkit.org/show_bug.cgi?id=101545

Attachment 173036: Patch
https://bugs.webkit.org/attachment.cgi?id=173036&action=review

------- Additional Comments from Antonio Gomes <tonikitoo at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=173036&action=review


It looks reasonable. I have a request that will require another round, so
r-'ing for now.

> Source/WebCore/page/EventHandler.cpp:338
> +    , m_didStartDrag(false)

I would like this variable to be named more specific. What started a drag?
m_longPressGestureDidStartDrag or something like this.

> Source/WebCore/page/EventHandler.cpp:2620
> +#if !OS(ANDROID)

that made me curious about what port you are testing this on... :)

> Source/WebCore/page/EventHandler.cpp:3312
> +bool EventHandler::handleDrag(const MouseEventWithHitTestResults& event,
bool checkForDragHysteresis)

The added bool can very much be an enum (even reuse the existing boolean
naming).