[webkit-reviews] review granted: [Bug 54417] [chromium] Add a basic gesture recognizer to the Chromium platform : [Attachment 95759] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jun 7 10:12:37 PDT 2011
Benjamin Poulain <benjamin at webkit.org> has granted Robert Kroeger
<rjkroege at chromium.org>'s request for review:
Bug 54417: [chromium] Add a basic gesture recognizer to the Chromium platform
https://bugs.webkit.org/show_bug.cgi?id=54417
Attachment 95759: Patch
https://bugs.webkit.org/attachment.cgi?id=95759&action=review
------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=95759&action=review
Yep, I like it.
I think the test will evolve a bit (kinetic scrolling, frames, block +
overflow) but that is a good start.
Some comments:
> Source/WebCore/platform/chromium/GestureRecognizerChromium.cpp:92
> +void InnerGestureRecognizer::addEdgeFunction(State state, unsigned fingerId,
PlatformTouchPoint::State touchType, bool touchHandledByJavaScript,
GestureTransitionFunction f)
> +{
> + m_edgeFunctions.add(signature(state, fingerId, touchType,
touchHandledByJavaScript), f);
A full variable name would be nice instead of "f".
> Source/WebCore/platform/chromium/GestureRecognizerChromium.cpp:158
> +// 1 LSB bit for signature >= 1
This is a bit unclear.
> Source/WebCore/platform/chromium/GestureRecognizerChromium.h:48
> + enum State {
> + NoGesture,
> + PendingSyntheticClick,
> + Scroll
> + };
Is the indent right here?
More information about the webkit-reviews
mailing list