[Webkit-unassigned] [Bug 35691] [Chromium] Adding new platform types for touch events and touch points.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 3 13:58:32 PST 2010


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





--- Comment #4 from Garret Kelly (Google) <gdk at chromium.org>  2010-03-03 13:58:32 PST ---
(In reply to comment #3)
> (From update of attachment 49923 [details])
> > +++ b/WebKit/chromium/public/WebInputEvent.h
> >  
> > +// WebTouchEvent --------------------------------------------------------------
> > +
> > +class WebTouchEvent : public WebInputEvent {
> > +public:
> > +    static const int touchPointCountCap = 4;
> 
> nit: CountCap -> LengthCap for consistency with WebKeyboardEvent

Done.

> 
> > +
> > +    int modifiers;
> > +    int touchPointCount;
> > +    WebTouchPoint touchPoints[touchPointCountCap];
> 
> Is touchPointCount necessary?  Perhaps you could use
> touchPoints[i].state == WebTouchPoint::Undefined as
> an array terminator instead?

That'd require using up a WebTouchPoint entry in the array just to indicate the
total length. For the sake of space it seems better to stick with the
touchPointCount.

> WebInputEvent already defines a modifiers member var,
> so I'm surprised to see it repeated here.

Removed.

> 
> > +++ b/WebKit/chromium/public/WebTouchPoint.h
> ...
> > +#include <string.h>
> 
> ^^^ this include seems unnecessary

Entirely. I'm baffled as to why I put that there. Removed. :)

> 
> > +namespace WebKit {
> > +
> > +class WebTouchPoint {
> > +public:
> > +    WebTouchPoint()
> > +        : id(0)
> > +        , state(Undefined)
> > +        , screenPosition(0, 0)
> > +        , position(0, 0) { }
> > +
> > +    enum State {
> > +        Undefined,
> > +        TouchReleased,
> > +        TouchPressed,
> > +        TouchMoved,
> > +        TouchStationary,
> > +        TouchCancelled,
> 
> How about StateReleased, StatePressed, etc. to better
> match the name of the enum?
> 
> Since these enum values will primarily be written w/
> the WebTouchPoint prefix, it seems unnecessary to have
> the word "Touch" in the names of the enum values.

Very sensible. Done. Also renamed Undefined -> StateUndefined.

Thank you very much for the comments. :)

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



More information about the webkit-unassigned mailing list