[webkit-reviews] review requested: [Bug 90662] [EFL][WK2] Add NativeWebTouchEvent and handle the Touch event. : [Attachment 164365] Patch

[bugzilla-daemon at webkit.org]

Gyuyoung Kim <gyuyoung.kim at samsung.com> has asked  for review:
Bug 90662: [EFL][WK2] Add NativeWebTouchEvent and handle the Touch event.
https://bugs.webkit.org/show_bug.cgi?id=90662

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

------- Additional Comments from Gyuyoung Kim <gyuyoung.kim at samsung.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=164365&action=review


Set r? again. It looks take a look further a little bit.

> Source/WebKit2/Shared/efl/WebEventFactory.cpp:199
> +    switch (type) {

It looks it is better to add new function to convert from EFL event type to
WebKit event type as QT port. I think this function can be used by other
functions.

http://trac.webkit.org/browser/trunk/Source/WebKit2/Shared/qt/WebEventFactoryQt
.cpp#L66

>> Source/WebKit2/Shared/efl/WebEventFactory.cpp:218
>> +	WebPlatformTouchPoint::TouchPointState state =
static_cast<WebPlatformTouchPoint::TouchPointState>(0);
> 
> I think that 0 may be TouchReleased.
> If you want to set default value for error case(default), I think that
TouchCancelled looks better.
> 
> And I think that it should be moved into EINA_LIST_FOREACH.
> Now this is only for first item.
> The state of second item will be the state of previous item when point->state
is error case.

Should we move *state* initialization into EINA_LIST_FOREACH loop? Evas only
supports 5 states as below,

  504 /**
  505  * State of Evas_Coord_Touch_Point
  506  */
  507 typedef enum _Evas_Touch_Point_State
  508 {
  509	 EVAS_TOUCH_POINT_DOWN, /**< Touch point is pressed down */
  510	 EVAS_TOUCH_POINT_UP, /**< Touch point is released */
  511	 EVAS_TOUCH_POINT_MOVE, /**< Touch point is moved */
  512	 EVAS_TOUCH_POINT_STILL, /**< Touch point is not moved after pressed */

  513	 EVAS_TOUCH_POINT_CANCEL /**< Touch point is cancelled */
  514 } Evas_Touch_Point_State;

It looks QT port just sets this out of loop. But, I don't mind to move it into
loop inside.