[Webkit-unassigned] [Bug 66800] Modify WebTouchEvent structure to match WebCore::TouchEvent

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from borenet at gmail.com  2011-08-24 06:24:07 PST ---
(From update of attachment 104914)
View in context: https://bugs.webkit.org/attachment.cgi?id=104914&action=review

>> Source/WebKit/chromium/public/WebTouchPoint.h:-45
>> -    };
> 
> Can you explain the reasons for and the consequences of removing this enum?

The enum limits the number of possible id's for WebTouchPoints.  While it could be expanded, it will become difficult to keep this in sync with WebTouchEvent::touchPointsLengthCap - since we now have an upper bound of eight touch points, we could see as many as eight different id's.  Since the enum is never actually used, it makes more sense to me to remove it.

>> Source/WebKit/chromium/src/WebInputEventConversion.cpp:270
>> +        m_touchPoints.append(PlatformTouchPointBuilder(widget, event.touches[i]));
> 
> Should you rename m_touchPoints to m_touches for consistency?

Probably, but that's a part of PlatformTouchEvent, and changing it would make this a much higher-impact change.

>> Source/WebKit/chromium/src/WebInputEventConversion.cpp:426
>> +        type = Undefined;
> 
> Does WebCore use touch event types other than start/move/end/cancel? If so, mention in the header comment which ones we honour and the fallback case. If not, perhaps there should be an assert here?

Those are the only touch event types.  What is the preferred way to do assertions?

>> Source/WebKit/chromium/src/WebInputEventConversion.cpp:431
>> +    timeStampSeconds = event.timeStamp() * 1.0e-3;
> 
> Eeek! Surely there's an existing kMillisecondsToSeconds you could use? Or at least use a static constant in this file.

The best I'm finding is in WebCore/icu/unicode/utypes.h.  That seems pretty far off - would it be better to define a constant here?

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