[Webkit-unassigned] [Bug 54417] [chromium] Add a basic gesture recognizer to the Chromium platform

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Robert Kroeger <rjkroege at chromium.org>  2011-05-10 08:55:09 PST ---
(From update of attachment 92841)
View in context: https://bugs.webkit.org/attachment.cgi?id=92841&action=review

>> Source/WebCore/platform/PlatformGestureRecognizer.cpp:39
>> +#if !PLATFORM(CHROMIUM)
> 
> We probably should exclude this file from the Chromium build instead of adding platform-specific ifdefs to a platform agnostic file.

Done in patch3

>> Source/WebCore/platform/chromium/GestureRecognizerChromium.cpp:40
>> +// Edge functions.
> 
> I'd remove this comment.

excessive comments removed in patch3

>> Source/WebCore/platform/chromium/GestureRecognizerChromium.cpp:49
>> +static const double maxClickDownTime = 0.8;
> 
> I'd rename this constant so that the comment is not needed.

all fixed in patch3

>> Source/WebCore/platform/chromium/GestureRecognizerChromium.cpp:66
>> +// TODO(rjkroege): write this function
> 
> WebKit uses FIXME comments rather than TODO comments.

done. the comment was out of date.

>> Source/WebCore/platform/chromium/GestureRecognizerChromium.cpp:176
>> +    gm->setState(GestureRecognizerChromium::PendingSyntheticClick);
> 
> gm => gestureRecongizer
> 
> WebKit prefers variable names that are made of complete english words.

done in patch3

>> Source/WebCore/platform/chromium/GestureRecognizerChromium.cpp:180
>> +bool reset(GestureRecognizerChromium* gm, const PlatformTouchPoint& p)
> 
> Unused parameters should have their names omitted (otherwise they won't build on PLATFORM(MAC))

done in patch3

>> Source/WebCore/platform/chromium/GestureRecognizerChromium.h:48
>> +// invoking WebViewImpl.
> 
> WebKit doesn't usually have class-level comments.

removed in patch3

>> Source/WebCore/platform/chromium/GestureRecognizerChromium.h:52
>> +    // The states of the gesture manager state machine.
> 
> This comment is useless.

removed in patch3

>> Source/WebCore/platform/chromium/GestureRecognizerChromium.h:64
>> +    // Adds a new edge function to the GestureRecognizerChromium.
> 
> Almost all of these comment don't add any information and should be removed.

removed in patch3

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