[webkit-reviews] review granted: [Bug 83135] [Qt] Improve the tap gesture recognizer : [Attachment 135547] Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Kenneth Rohde Christiansen
<kenneth at webkit.org>'s request for review:
Bug 83135: [Qt] Improve the tap gesture recognizer
https://bugs.webkit.org/show_bug.cgi?id=83135

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

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=135547&action=review


r=me

We really need unit tests for this state machine though :(

> Source/WebKit2/UIProcess/qt/QtTapGestureRecognizer.cpp:71
> +	       if (!withinDistance(touchPoint, maxDoubleTapDistance)) {
> +		   reset(); // Received a new tap, that is unrelated to the
previous one. Ignore previous.
>		   m_tapState = SingleTapStarted;
> -	       }
> -	   } else
> -	       m_tapState = SingleTapStarted;
> -	   m_touchBeginEventForTap = adoptPtr(new QTouchEvent(*event));
> +	       } else
> +		   m_tapState = DoubleTapCandidate;

I kind of like logic the "positive" way, i.e.

if (withinDistance(touchPoint, ...)
    m_tapState = DoubleTapCandidate;
} else {
    reset();
    m_tapState = SingleTapStarted;
}

But that's also a kind of nitpick which you can freely ignore if you want :)