Bug 102643 – [EFL][WK2] Implement gesture recognizer.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Eunmi Lee]

I will make patch to add gesture_start, gesture_end and gesture_move APIs to the ewk_view's smart class.

We can tap, pan, flick and zoom using gesture APIs when TILED_BACKING_STORE is enabled.

[Eunmi Lee]

Created attachment 175595 [details]
Patch

[Eunmi Lee]

Created attachment 175740 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 175740 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175740&action=review

> Source/WebKit2/UIProcess/API/efl/GestureHandler.cpp:44
> +    UNUSED_PARAM(event);

WebKit prefers to remove parameter name if there is no macro inside function.

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:237
> +#else

Need to add UNUSED_PARAM

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:248
> +#else

ditto.

[Gyuyoung Kim]

CC'ing Kenneth, Kenneth, how do you think about this patch ?

[Kenneth Rohde Christiansen]

Comment on attachment 175740 [details]
Patch

Huh? It is impossible to see how this is supposed to evolve. Gestures for interacting with the web view need to work together and cannot be looked upon as fully separate gestures. I am not convinced this is the right direction

[Eunmi Lee]

(In reply to comment #5)
> (From update of attachment 175740 [details])
> Huh? It is impossible to see how this is supposed to evolve. Gestures for interacting with the web view need to work together and cannot be looked upon as fully separate gestures. I am not convinced this is the right direction

1. I understand your question is "why gestures are fed using smart API from outside?".
The main reason of that is to support "C++ API layer of Tizen"(Bada).
If we merge the C++ layer, the ewk_view can not get the event directly and all events are fed by C++ layer.
So, we need route to feed gestures to the ewk_view.

2. Are you worried about to interact with W3C touch events?
We can enable and disable the gestures for condition of touch events' preventDefault.

3. I have a plan to make bugs for each gesture after merging this patch.
but do you think it is good to add all codes in this bug for understanding?
If so, I will add all codes about gestures here.

[Eunmi Lee]

Comment on attachment 175740 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175740&action=review

>> Source/WebKit2/UIProcess/API/efl/GestureHandler.cpp:44
>> +    UNUSED_PARAM(event);
> 
> WebKit prefers to remove parameter name if there is no macro inside function.

OK, I will remove it.

>> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:237
>> +#else
> 
> Need to add UNUSED_PARAM

Thanks I will add it.

>> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:248
>> +#else
> 
> ditto.

ditto

[Eunmi Lee]

Created attachment 177938 [details]
Patch

[Dominik Röttsches (drott)]

What I am concerned about, and I think that's also what Kenneth partially means: Can you combine multiple gestures? Can you for example send two "start" signals, one for PAN, one for PINCH? Or can you transition between them? 

On a touchscreen, PAN is one finger, PINCH would be two finger, so they're separate. But if you imagine a trackpad, one finger usually means pointer movement, two finger means PAN but can also mean PINCH - how can you transition between the two or change from one to the other? Or do you have to decide for example at the beginning of the gesture wheter it's going to be PAN or PINCH?

Kenneth: Which gestures do you think need to "work together"?

[Eunmi Lee]

(In reply to comment #9)
> What I am concerned about, and I think that's also what Kenneth partially means: Can you combine multiple gestures? Can you for example send two "start" signals, one for PAN, one for PINCH? Or can you transition between them? 
> 
> On a touchscreen, PAN is one finger, PINCH would be two finger, so they're separate. But if you imagine a trackpad, one finger usually means pointer movement, two finger means PAN but can also mean PINCH - how can you transition between the two or change from one to the other? Or do you have to decide for example at the beginning of the gesture wheter it's going to be PAN or PINCH?
> 
> Kenneth: Which gestures do you think need to "work together"?

The main idea of this patch is that "all things for recognizing the gesture are done outside of the web view".
That means, the web view does not have to know the number of touch points and decide current state is Pan or Pinch and the web view just processes the gesture APIs when they are called.

Answering for your question - "two finger means PAN but can also mean PINCH",
I think your question is that you want to scroll the page when we move the two finger together within same distance and zoom the page when we change the distance between fingers.
That can be processed in the PINCH state. we can process scroll or zoom for each two finger's movement.

By the way,
I've decide to change this patch.
I think it is not efficient to depend on applications or libraries outside of web view to recognize gestures,
so I am going to make "gesture recognizer" inside webkit2 efl port (like webkit2 qt port) to provide same behavior for touches to applications and libraries which use webkit2 efl.

[Eunmi Lee]

(In reply to comment #10)
> (In reply to comment #9)
> > What I am concerned about, and I think that's also what Kenneth partially means: Can you combine multiple gestures? Can you for example send two "start" signals, one for PAN, one for PINCH? Or can you transition between them? 
> > 
> > On a touchscreen, PAN is one finger, PINCH would be two finger, so they're separate. But if you imagine a trackpad, one finger usually means pointer movement, two finger means PAN but can also mean PINCH - how can you transition between the two or change from one to the other? Or do you have to decide for example at the beginning of the gesture wheter it's going to be PAN or PINCH?
> > 
> > Kenneth: Which gestures do you think need to "work together"?
> 
> The main idea of this patch is that "all things for recognizing the gesture are done outside of the web view".
> That means, the web view does not have to know the number of touch points and decide current state is Pan or Pinch and the web view just processes the gesture APIs when they are called.
> 
> Answering for your question - "two finger means PAN but can also mean PINCH",
> I think your question is that you want to scroll the page when we move the two finger together within same distance and zoom the page when we change the distance between fingers.
> That can be processed in the PINCH state. we can process scroll or zoom for each two finger's movement.
> 
> By the way,
> I've decide to change this patch.
> I think it is not efficient to depend on applications or libraries outside of web view to recognize gestures,
> so I am going to make "gesture recognizer" inside webkit2 efl port (like webkit2 qt port) to provide same behavior for touches to applications and libraries which use webkit2 efl.

Additionally, 
I thought that the web view can not recognize gesture with C++ API layer of Tizen because web view ca not get the raw event directly in that case,
but it is possible to recognize gesture if we use Touch Events of webkit instead of raw event from Evas.

[Eunmi Lee]

Created attachment 182520 [details]
Patch

[Eunmi Lee]

Comment on attachment 182520 [details]
Patch

This patch is in a work-in-process.

[Eunmi Lee]

Created attachment 182537 [details]
Patch

[Eunmi Lee]

Created attachment 182722 [details]
Patch

Change to use screen position instead of contents position because contents can be changed before getting touch events result.

[Kenneth Rohde Christiansen]

Comment on attachment 182722 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182722&action=review

> Source/WebKit2/PlatformEfl.cmake:89
>      UIProcess/efl/FormClientEfl.cpp
> +    UIProcess/efl/GestureHandler.cpp
> +    UIProcess/efl/GestureRecognizer.cpp
>      UIProcess/efl/InputMethodContextEfl.cpp

Are these WebKit2 dependent? Can we somehow in the future share code with Qt?

> Source/WebKit2/UIProcess/efl/GestureRecognizer.cpp:2
> + * Copyright (C) 2013 Samsung Electronics. All rights reserved.

This code looks to be inspired by the Qt code...

[Kenneth Rohde Christiansen]

(In reply to comment #15)
> Created an attachment (id=182722) [details]
> Patch
> 
> Change to use screen position instead of contents position because contents can be changed before getting touch events result.

Be aware that screen positions might be in actually device positions or UI positions.

[Kenneth Rohde Christiansen]

 
> Kenneth: Which gestures do you think need to "work together"?

Almost all :-)

1 finger -> pan, tap, double tap
2 fingers -> pinch include move, possible turn into pan when fingers are very close, otherwise do scale and move at the same time.
3 fingers -> possible tread as pan

You should be able to move from say pan to pinch, and the other way around

[Eunmi Lee]

(In reply to comment #16)
> (From update of attachment 182722 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182722&action=review
> 
> > Source/WebKit2/PlatformEfl.cmake:89
> >      UIProcess/efl/FormClientEfl.cpp
> > +    UIProcess/efl/GestureHandler.cpp
> > +    UIProcess/efl/GestureRecognizer.cpp
> >      UIProcess/efl/InputMethodContextEfl.cpp
> 
> Are these WebKit2 dependent? Can we somehow in the future share code with Qt?
> 

Some codes are depends on WebKit2 - such as TouchEvents class (WebPlatformTouchPoint, NativeWebTouchEvent)
They can be changed to use common structure and shared with WebKit1,
but I think I don't have to implement gesture for WebKit1 EFL now and gesture codes can be refactored when WebKit1 EFL wants to use that.

For sharing code with Qt,
I think some codes to recognize the gestures can be shared with Qt, but it is not easy to make it because:
- Qt port is using Qt's data structures, so they have to be changed to use common structures.
- Qt is using QQuickFlickable and QQuickItem for flicking and scaling, but EFL does not have such libraries, so caller codes have to be refactored to support both Qt and EFL.
So, codes can be shared after refactoring in the future.

> > Source/WebKit2/UIProcess/efl/GestureRecognizer.cpp:2
> > + * Copyright (C) 2013 Samsung Electronics. All rights reserved.
> 
> This code looks to be inspired by the Qt code...

Yes, right. I have to add Copyright for Nokia. :)

[Eunmi Lee]

(In reply to comment #17)
> (In reply to comment #15)
> > Created an attachment (id=182722) [details] [details]
> > Patch
> > 
> > Change to use screen position instead of contents position because contents can be changed before getting touch events result.
> 
> Be aware that screen positions might be in actually device positions or UI positions.

OK, thank you. :)

[Eunmi Lee]

Created attachment 183716 [details]
Patch

Add Nokia in the copyright of gesture files.

[Eunmi Lee]

Created attachment 184174 [details]
Patch

Rebase patch

[Antonio Gomes]

Comment on attachment 184174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184174&action=review

Some nits:

> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:150
> +    WebKit::GestureRecognizer* gestureRecognizer() { return m_gestureRecognizer.get(); }

should be const?

> Source/WebKit2/UIProcess/efl/GestureRecognizer.cpp:62
> +    for (int i = 0; i < numberOfTouchPoints; ++i)

you lack {} wrapping 'for'

[Eunmi Lee]

Comment on attachment 184174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184174&action=review

>> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:150
>> +    WebKit::GestureRecognizer* gestureRecognizer() { return m_gestureRecognizer.get(); }
> 
> should be const?

Thanks for your review :)
I will update this patch for changed Efl port and apply your comments.

[Chris Dumez]

Comment on attachment 184174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184174&action=review

>>> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:150
>>> +    WebKit::GestureRecognizer* gestureRecognizer() { return m_gestureRecognizer.get(); }
>> 
>> should be const?
> 
> Thanks for your review :)
> I will update this patch for changed Efl port and apply your comments.

Actually, it was discussed on the mailing list that we should not make the getter const if we return a non-const pointer. I think the current patch is correct.

[Chris Dumez]

I was referring to https://lists.webkit.org/pipermail/webkit-dev/2012-October/022576.html

[Mikhail Pozdnyakov]

Comment on attachment 184174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184174&action=review

> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:76
> +#if ENABLE(TOUCH_EVENTS) && USE(TILED_BACKING_STORE)

USE(TILED_BACKING_STORE) is not needed since coord graphics code path is always used in EFL WK2.

> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:149
> +#if USE(TILED_BACKING_STORE)

ditto

>>>> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:150
>>>> +    WebKit::GestureRecognizer* gestureRecognizer() { return m_gestureRecognizer.get(); }
>>> 
>>> should be const?
>> 
>> Thanks for your review :)
>> I will update this patch for changed Efl port and apply your comments.
> 
> Actually, it was discussed on the mailing list that we should not make the getter const if we return a non-const pointer. I think the current patch is correct.

Agree with Chris, that should not be const indeed.

> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:286
> +#if USE(TILED_BACKING_STORE)

should be removed as well, and everywhere within the patch

> Source/WebKit2/UIProcess/efl/GestureHandler.h:32
> +#include <wtf/Vector.h>

think it could be also forward declared

> Source/WebKit2/UIProcess/efl/GestureHandler.h:52
> +    GestureHandler(EwkViewImpl*);

explicit :)

> Source/WebKit2/UIProcess/efl/GestureRecognizer.cpp:72
> +    } else if (event.type() == WebEvent::TouchMove) {

"switch case" would look better imho

> Source/WebKit2/UIProcess/efl/PanGestureHandler.cpp:57
> +    if (points.size() < 1) {

==0?

> Source/WebKit2/UIProcess/efl/PanGestureHandler.h:40
> +class PanGestureHandler : private GestureHandler {

why do you need private inheritance here?

> Source/WebKit2/UIProcess/efl/PinchGestureHandler.h:40
> +class PinchGestureHandler : private GestureHandler {

and here

> Source/WebKit2/UIProcess/efl/TapGestureHandler.h:39
> +class TapGestureHandler : private GestureHandler {

and here

[Mikhail Pozdnyakov]

Comment on attachment 184174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184174&action=review

> Source/WebKit2/UIProcess/efl/GestureRecognizer.cpp:107
> +    if (m_currentGesture == TapGesture)

what about current handler instead?

> Source/WebKit2/UIProcess/efl/GestureRecognizer.h:76
> +    TapGestureHandler m_tapGestureHandler;

oh, so why than they need common base interface??? if they are all declared and used with their own types

[Mikhail Pozdnyakov]

Comment on attachment 184174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184174&action=review

> Source/WebKit2/UIProcess/efl/GestureRecognizer.h:55
> +    virtual ~GestureRecognizer() { }

is anyone inherited from GestureRecognizer? guess it should not need virtual

> Source/WebKit2/UIProcess/efl/GestureRecognizer.h:74
> +    Gesture m_currentGesture;

think we don't need it.

>> Source/WebKit2/UIProcess/efl/GestureRecognizer.h:76
>> +    TapGestureHandler m_tapGestureHandler;
> 
> oh, so why than they need common base interface??? if they are all declared and used with their own types

to be more clear: they all(including Gesture m_currentGesture) could be replaced with OwnPtr<GestureHandler> m_currentHandler;

[Chris Dumez]

Comment on attachment 184174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184174&action=review

> Source/WebKit2/UIProcess/efl/PanGestureHandler.cpp:47
> +    if (points.size() < 1) {

if (points.isEmpty())

>> Source/WebKit2/UIProcess/efl/PanGestureHandler.cpp:57
>> +    if (points.size() < 1) {
> 
> ==0?

== 0 would be against WebKit coding style :P
I think this should be simply: if (points.isEmpty())

[Mikhail Pozdnyakov]

(In reply to comment #30)
> (From update of attachment 184174 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=184174&action=review
> 
> > Source/WebKit2/UIProcess/efl/PanGestureHandler.cpp:47
> > +    if (points.size() < 1) {
> 
> if (points.isEmpty())
> 
> >> Source/WebKit2/UIProcess/efl/PanGestureHandler.cpp:57
> >> +    if (points.size() < 1) {
> > 
> > ==0?
> 
> == 0 would be against WebKit coding style :P
> I think this should be simply: if (points.isEmpty())

Here I meant just that it's better to be compared to '0' (not empty) in principal. But good that you pointed out how to do it according to style guidelines :)

[Eunmi Lee]

Comment on attachment 184174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184174&action=review

Mikhail and Christophe, thanks for comments :)
I will update patch for changed EFL port and comments.

>> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:76
>> +#if ENABLE(TOUCH_EVENTS) && USE(TILED_BACKING_STORE)
> 
> USE(TILED_BACKING_STORE) is not needed since coord graphics code path is always used in EFL WK2.

Yes, I will remove all USE(TILED_BACKING_STORE).

>>>>> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:150
>>>>> +    WebKit::GestureRecognizer* gestureRecognizer() { return m_gestureRecognizer.get(); }
>>>> 
>>>> should be const?
>>> 
>>> Thanks for your review :)
>>> I will update this patch for changed Efl port and apply your comments.
>> 
>> Actually, it was discussed on the mailing list that we should not make the getter const if we return a non-const pointer. I think the current patch is correct.
> 
> Agree with Chris, that should not be const indeed.

Ok, thanks.

>> Source/WebKit2/UIProcess/efl/GestureHandler.h:32
>> +#include <wtf/Vector.h>
> 
> think it could be also forward declared

right.

>> Source/WebKit2/UIProcess/efl/GestureHandler.h:52
>> +    GestureHandler(EwkViewImpl*);
> 
> explicit :)

oh, I missed that :)

>> Source/WebKit2/UIProcess/efl/GestureRecognizer.cpp:62
>> +    for (int i = 0; i < numberOfTouchPoints; ++i)
> 
> you lack {} wrapping 'for'

I will add.

>> Source/WebKit2/UIProcess/efl/GestureRecognizer.cpp:72
>> +    } else if (event.type() == WebEvent::TouchMove) {
> 
> "switch case" would look better imho

yes, I agree.

>> Source/WebKit2/UIProcess/efl/GestureRecognizer.cpp:107
>> +    if (m_currentGesture == TapGesture)
> 
> what about current handler instead?

ok, I will think about name again.

>> Source/WebKit2/UIProcess/efl/GestureRecognizer.h:55
>> +    virtual ~GestureRecognizer() { }
> 
> is anyone inherited from GestureRecognizer? guess it should not need virtual

Nobody inherits this class yet. I will remove virtual.

>>> Source/WebKit2/UIProcess/efl/GestureRecognizer.h:76
>>> +    TapGestureHandler m_tapGestureHandler;
>> 
>> oh, so why than they need common base interface??? if they are all declared and used with their own types
> 
> to be more clear: they all(including Gesture m_currentGesture) could be replaced with OwnPtr<GestureHandler> m_currentHandler;

Yes, I will do refactoring all about this.

>>> Source/WebKit2/UIProcess/efl/PanGestureHandler.cpp:47
>>> +    if (points.size() < 1) {
>> 
>> if (points.isEmpty())
> 
> Here I meant just that it's better to be compared to '0' (not empty) in principal. But good that you pointed out how to do it according to style guidelines :)

oh, I have to use isEmpty() :)

>> Source/WebKit2/UIProcess/efl/PanGestureHandler.h:40
>> +class PanGestureHandler : private GestureHandler {
> 
> why do you need private inheritance here?

I think I wanted to hide GestureHandler's variables, but I don't have to use private because variables are protected.

[Eunmi Lee]

Created attachment 189001 [details]
Patch

Rebase and update for comments.

[Eunmi Lee]

Created attachment 191401 [details]
Patch

Rebased.

[Eunmi Lee]

Created attachment 193276 [details]
Patch

Modified gesture recognizer's logic like NIX webkit.

[Kenneth Rohde Christiansen]

Comment on attachment 193276 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193276&action=review

LGTM

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:56
> +    (this->*m_state)(type, WKTouchEventGetTouchPoints(eventRef));

Maybe s/m_state/m_eventHandlerFn/

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:69
> +    static const double s_doubleTapTimeout;
> +    static const double s_tapAndHoldTimeout;
> +    static const int s_squaredDoubleTapThreshold = 10000;
> +    static const int s_squaredPanThreshold = 100;

why are some declared here and others not? and what are the units? MS = milli seconds

...TimeoutMillis (or what is common to use in WebCore)

[Mikhail Pozdnyakov]

Comment on attachment 193276 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193276&action=review

> Source/WebKit2/UIProcess/API/efl/EwkView.h:102
> +class EwkView : public WebKit::GestureRecognizerClient {

we usually have clients decoupled from view..

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:48
> +    virtual void handlePinchStarted(const Vector<WebCore::IntPoint>*) { }

why pointer and not reference? can it be null?

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:49
> +    virtual void handlePinch(const Vector<WebCore::IntPoint>*) { }

ditto.

[Eunmi Lee]

Comment on attachment 193276 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193276&action=review

>> Source/WebKit2/UIProcess/API/efl/EwkView.h:102
>> +class EwkView : public WebKit::GestureRecognizerClient {
> 
> we usually have clients decoupled from view..

Yes, I will separate GestureRecognizerClient :)

>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:56
>> +    (this->*m_state)(type, WKTouchEventGetTouchPoints(eventRef));
> 
> Maybe s/m_state/m_eventHandlerFn/

I agree that m_state is ambiguous. how about m_recognizerFunction? because functions are recognizing gestures :)

>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:48
>> +    virtual void handlePinchStarted(const Vector<WebCore::IntPoint>*) { }
> 
> why pointer and not reference? can it be null?

hmm, It should not be null. I will change it to reference.

>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:49
>> +    virtual void handlePinch(const Vector<WebCore::IntPoint>*) { }
> 
> ditto.

ditto.

>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:69
>> +    static const int s_squaredPanThreshold = 100;
> 
> why are some declared here and others not? and what are the units? MS = milli seconds
> 
> ...TimeoutMillis (or what is common to use in WebCore)

It can be confusing because I defined only integer variables here.
so, I will modify to declare all static const variables here and define all of them in the cpp file :)

and units is seconds, so I will rename to TimeoutInSeconds.

[Eunmi Lee]

Created attachment 193753 [details]
Patch

Update for comments.

[Mikhail Pozdnyakov]

Comment on attachment 193753 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193753&action=review

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:41
> +public:

virtual destructor missing.

[Eunmi Lee]

Created attachment 206052 [details]
Patch

Add virtual destructor for GestureRecognizerClient and rebased.

[Eunmi Lee]

Created attachment 207367 [details]
Patch

Rebase.

[Gyuyoung Kim]

Comment on attachment 207367 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207367&action=review

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:129
> +    case kWKEventTypeTouchMove: {

It would be good if you keep consistency to use { } for case state.

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:151
> +    default:

Isn't it use ASSERT_NOT_REACHED() ?

[Eunmi Lee]

Comment on attachment 207367 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207367&action=review

>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:129
>> +    case kWKEventTypeTouchMove: {
> 
> It would be good if you keep consistency to use { } for case state.

I think that { } for cast statement is usually used only if it is necessary. (for example, WebPageProxy::didReceiveEvent())
So, I will keep this codes.

>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:151
>> +    default:
> 
> Isn't it use ASSERT_NOT_REACHED() ?

We can reach here if type is kWKEventTypeTouchCancel.
So, I will add case for kWKEventTypeTouchCancel and add ASSERT_NOT_REACHED() for default case.

[Eunmi Lee]

Created attachment 207443 [details]
Patch

Update for comments and modify createVectorWithWKArray to fix the crash.

[Gyuyoung Kim]

Comment on attachment 207443 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207443&action=review

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:116
> +    default:

ASSERT_NOT_REACHED(); ?

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:131
> +        if (m_firstPressedPoint.distanceSquaredToPoint(currentPoint) > s_squaredPanThreshold) {

Isn't it better to add a function to check threshold as nix port's implementation ?

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:170
> +            if (m_firstPressedPoint.distanceSquaredToPoint(toIntPoint(getPointAtIndex(touchPoints, 0))) > s_squaredDoubleTapThreshold)

ditto.

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:180
> +        if (m_firstPressedPoint.distanceSquaredToPoint(currentPoint) > s_squaredPanThreshold) {

ditto.

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:40
> +class GestureRecognizerClient {

Should we have virtual class ? Do you have a plan to support other port in efl ?

[Eunmi Lee]

Comment on attachment 207443 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207443&action=review

>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:116
>> +    default:
> 
> ASSERT_NOT_REACHED(); ?

Yes, I will check again and add ASSERT_NOT_REACHED() to the whole file.

>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:131
>> +        if (m_firstPressedPoint.distanceSquaredToPoint(currentPoint) > s_squaredPanThreshold) {
> 
> Isn't it better to add a function to check threshold as nix port's implementation ?

ok, I will make separated function for that.

>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:40
>> +class GestureRecognizerClient {
> 
> Should we have virtual class ? Do you have a plan to support other port in efl ?

Actually no, but I think it can be shared with other port later.
anyway, it is not necessary now, so I will remove this virtual class.

[Eunmi Lee]

Created attachment 207509 [details]
Patch

Update for Gyuyoung's comments.

[Ryuan Choi]

Comment on attachment 207509 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207509&action=review

> Source/WebKit2/UIProcess/API/efl/GestureRecognizerClient.h:44
> +class GestureRecognizerClient {

I am confusing what this class can do.
Will this class have the logic by itself?

If this class would just forward the handles to EwkView, we can make this as interface for EwkView to inherit.

[Ryuan Choi]

Comment on attachment 207509 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207509&action=review

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:123
> +        ASSERT(m_tapAndHoldTimer);

ASSERT(!m_tapAndHoldTimer);

[Gyuyoung Kim]

(In reply to comment #49)
> (From update of attachment 207509 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207509&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/GestureRecognizerClient.h:44
> > +class GestureRecognizerClient {
> 
> I am confusing what this class can do.
> Will this class have the logic by itself?
> 
> If this class would just forward the handles to EwkView, we can make this as interface for EwkView to inherit.

It looks Eunmi added it to be aligned with nix port as Eunmi said before. So, virtual class was added as well. If there is no need to be aligned with nix port, it looks we don't need to use Client class. Eunmi, is my assumption right? what do you think about it ?

[Eunmi Lee]

Comment on attachment 207509 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207509&action=review

>>> Source/WebKit2/UIProcess/API/efl/GestureRecognizerClient.h:44
>>> +class GestureRecognizerClient {
>> 
>> I am confusing what this class can do.
>> Will this class have the logic by itself?
>> 
>> If this class would just forward the handles to EwkView, we can make this as interface for EwkView to inherit.
> 
> It looks Eunmi added it to be aligned with nix port as Eunmi said before. So, virtual class was added as well. If there is no need to be aligned with nix port, it looks we don't need to use Client class. Eunmi, is my assumption right? what do you think about it ?

Ryuan and Gyuyoung, thanks for comments.
As Ryuan's suggestion, GestureRecognizerClient codes can be moved to the EwkView.
However, I'm afraid to add many things to the EwkView. Do you think that is ok? If so, I will merge GestureRecognizerClient codes to the EwkView.

[Build Bot]

Comment on attachment 207509 [details]
Patch

Attachment 207509 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1253285

New failing tests:
editing/selection/leak-document-with-selection-inside.html

[Build Bot]

Created attachment 207547 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3

[Eunmi Lee]

(In reply to comment #52)
> (From update of attachment 207509 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207509&action=review
> 
> >>> Source/WebKit2/UIProcess/API/efl/GestureRecognizerClient.h:44
> >>> +class GestureRecognizerClient {
> >> 
> >> I am confusing what this class can do.
> >> Will this class have the logic by itself?
> >> 
> >> If this class would just forward the handles to EwkView, we can make this as interface for EwkView to inherit.
> > 
> > It looks Eunmi added it to be aligned with nix port as Eunmi said before. So, virtual class was added as well. If there is no need to be aligned with nix port, it looks we don't need to use Client class. Eunmi, is my assumption right? what do you think about it ?
> 
> Ryuan and Gyuyoung, thanks for comments.
> As Ryuan's suggestion, GestureRecognizerClient codes can be moved to the EwkView.
> However, I'm afraid to add many things to the EwkView. Do you think that is ok? If so, I will merge GestureRecognizerClient codes to the EwkView.

As I discuss with Ryuan, I will move codes to the EwkView.

[Eunmi Lee]

Created attachment 207619 [details]
Patch

Move GestureRecognizerClient to the EwkView.

[Ryuan Choi]

Comment on attachment 207619 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207619&action=review

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:122
> +        ASSERT(m_tapAndHoldTimer);

It looks wrong.

[Eunmi Lee]

(In reply to comment #57)
> (From update of attachment 207619 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207619&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:122
> > +        ASSERT(m_tapAndHoldTimer);
> 
> It looks wrong.

My mistake. I will fix that.

[Eunmi Lee]

Created attachment 207650 [details]
Patch

As discussing with Ryuan, change GestureRecognizerClient to GestureRecognizer::Client and change its functions to pure virtual function.

[Mikhail Pozdnyakov]

Comment on attachment 207650 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207650&action=review

> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1417
> +void EwkView::handlePan(const IntPoint& point)

no need for 'point' so far..

> Source/WebKit2/UIProcess/API/efl/EwkView.h:105
> +    : public WebKit::GestureRecognizer::Client

I don't like this inheritance, clients are usually just owned by view. Can we follow standard pattern?

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:42
> +    class Client {

nested type.. :/ Why do we need client? what if GestureRecognizer was aware of EwkView instead (like others clients do)?

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:70
> +    static const int s_squaredPanThreshold;

two variables above can be defined right here.

[Eunmi Lee]

Comment on attachment 207650 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207650&action=review

>> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1417
>> +void EwkView::handlePan(const IntPoint& point)
> 
> no need for 'point' so far..

ok, I will remove point.

>> Source/WebKit2/UIProcess/API/efl/EwkView.h:105
>> +    : public WebKit::GestureRecognizer::Client
> 
> I don't like this inheritance, clients are usually just owned by view. Can we follow standard pattern?

Clients owned by view and GestureRecognizer::Client are different.
1. "Clients owned by view" are WKClient type and they can not be inherited, so EwkView has to own them.
2. "GestureRecognizer::Client" is to get the gestures from GestureRecognizer and the EwkView is the main body to handle each gestures, so it is better to inherit Client directly.
   We already use this kind of Client in the CoreIPC::Connection::Client, WebPopupMenuProxy::Client, DownloadManager::Client and so on.
As a minor reason, GestureRecognizer::Client is not the type of WKClient, so If it is separated Client owned by EwkView, it can be confused with WKClients.
So, I want to distinguish GestureRecognizer::Client with WKClients.

>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:42
>> +    class Client {
> 
> nested type.. :/ Why do we need client? what if GestureRecognizer was aware of EwkView instead (like others clients do)?

Why do you hate neted type? we are already using nested type in the webkit, so I 'm curious.
Client makes codes structural and easy to understand the connection between EwkView and GestureRecognizer, so I think it is better than calling EwkView's functions directly.

>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:70
>> +    static const int s_squaredPanThreshold;
> 
> two variables above can be defined right here.

I moved them to the cpp file by comment #36 for consistency.

[Eunmi Lee]

Created attachment 207683 [details]
Patch

Remove parameter name from EwkView::handlePan().

[Mikhail Pozdnyakov]

(In reply to comment #61)
> (From update of attachment 207650 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207650&action=review
> 
> >> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1417
> >> +void EwkView::handlePan(const IntPoint& point)
> > 
> > no need for 'point' so far..
> 
> ok, I will remove point.
> 
> >> Source/WebKit2/UIProcess/API/efl/EwkView.h:105
> >> +    : public WebKit::GestureRecognizer::Client
> > 
> > I don't like this inheritance, clients are usually just owned by view. Can we follow standard pattern?
> 
> Clients owned by view and GestureRecognizer::Client are different.
> 1. "Clients owned by view" are WKClient type and they can not be inherited, so EwkView has to own them.
> 2. "GestureRecognizer::Client" is to get the gestures from GestureRecognizer and the EwkView is the main body to handle each gestures, so it is better to inherit Client directly.
>    We already use this kind of Client in the CoreIPC::Connection::Client, WebPopupMenuProxy::Client, DownloadManager::Client and so on.
> As a minor reason, GestureRecognizer::Client is not the type of WKClient, so If it is separated Client owned by EwkView, it can be confused with WKClients.
> So, I want to distinguish GestureRecognizer::Client with WKClients.
> 
> >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:42
> >> +    class Client {
> > 
> > nested type.. :/ Why do we need client? what if GestureRecognizer was aware of EwkView instead (like others clients do)?
> 
> Why do you hate neted type? we are already using nested type in the webkit, so I 'm curious.

No, I don't hate them :) I find however that in this case it just brings complexity and I don't see benefit (so far) in having them.
Introducing of a new interface would make sense if we had say several clients, now I see only EwkView.

[Eunmi Lee]

Comment on attachment 207650 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207650&action=review

>>>> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1417
>>>> +void EwkView::handlePan(const IntPoint& point)
>>> 
>>> no need for 'point' so far..
>> 
>> ok, I will remove point.
> 
> No, I don't hate them :) I find however that in this case it just brings complexity and I don't see benefit (so far) in having them.
> Introducing of a new interface would make sense if we had say several clients, now I see only EwkView.

Actually, I thought GestureRecognizer can be used by other ports later.
However, we use that only in the EFL port and it is even in the efl folder now.
So, I will modify codes to use EwkView directly now.

[Mikhail Pozdnyakov]

Comment on attachment 207683 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207683&action=review

> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1397
> +void EwkView::handleSingleTap(const IntPoint&)

it's hard to say now since all the methods are empty, but is it really necessary to have them in view? I'd prefer having logic encapsulated within a dedicated object than making view do everything.

[Ryuan Choi]

(In reply to comment #65)
> (From update of attachment 207683 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207683&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1397
> > +void EwkView::handleSingleTap(const IntPoint&)
> 
> it's hard to say now since all the methods are empty, but is it really necessary to have them in view? I'd prefer having logic encapsulated within a dedicated object than making view do everything.

I found GestureHandler until 10th patch, but 11th patch refactored it to match Nix port. (see #35)

I also think that dedicated object like GestureHandler (or any better name?) looks better.

[Eunmi Lee]

(In reply to comment #66)
> (In reply to comment #65)
> > (From update of attachment 207683 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=207683&action=review
> > 
> > > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1397
> > > +void EwkView::handleSingleTap(const IntPoint&)
> > 
> > it's hard to say now since all the methods are empty, but is it really necessary to have them in view? I'd prefer having logic encapsulated within a dedicated object than making view do everything.
> 
> I found GestureHandler until 10th patch, but 11th patch refactored it to match Nix port. (see #35)
> 
> I also think that dedicated object like GestureHandler (or any better name?) looks better.

I agree with you. EwkView is getting bigger, so it is better to have separated object.
I will make GestureHandler to delegate function for gestures.

[Eunmi Lee]

Created attachment 207714 [details]
Patch

Update for comments.

[Build Bot]

Comment on attachment 207714 [details]
Patch

Attachment 207714 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1288346

New failing tests:
editing/selection/leak-document-with-selection-inside.html

[Build Bot]

Created attachment 207717 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.4

[Eunmi Lee]

Created attachment 207722 [details]
Patch

Rebase.

[Eunmi Lee]

Created attachment 207732 [details]
Patch

Fix wrong usage of Vector in the createVectorWithWKArray().

[Gyuyoung Kim]

Comment on attachment 207732 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207732&action=review

Looks better otherwise.

> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:53
> +    static const double s_doubleTapTimeoutInSeconds;

Is there any reason these static variables should be declared as public ? It looks these variables are being used as private.

[Eunmi Lee]

(In reply to comment #73)
> (From update of attachment 207732 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207732&action=review
> 
> Looks better otherwise.
> 
> > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.h:53
> > +    static const double s_doubleTapTimeoutInSeconds;
> 
> Is there any reason these static variables should be declared as public ? It looks these variables are being used as private.

No, I will make them private.

[Eunmi Lee]

Created attachment 207909 [details]
Patch

Make static variables private and do not assert for TouchMove and TouchEnd in the noGesture because they can be called after long press.

[Gyuyoung Kim]

Comment on attachment 207909 [details]
Patch

LGTM. But, Ryuan, Mikhail or someone else might have a final look before landing.

[Mikhail Pozdnyakov]

(In reply to comment #76)
> (From update of attachment 207909 [details])
> LGTM. But, Ryuan, Mikhail or someone else might have a final look before landing.

OK, I don't want to prevent from landing but I still have a concern regarding the intended design (maybe it can be addressed later):

GestureRecognizer <-> GestureHandler relationships are still unclear to me. 

So far I see two perspectives: 
1) GestureRecognizer is(can be) efl-agnostic and shared between ports, than GestureHandler should be a common interface with port-specific implementation.
My question is: is it possible? would qt/nix people like to re-use GestureRecognizer at least in theory?
 

2) GestureRecognizer is efl specific, than what's the point in having  GestureHandler? if the point is only to make GestureRecognizer more readable I would make GestureHandler private and declare it in cpp file.

[Eunmi Lee]

(In reply to comment #77)
> (In reply to comment #76)
> > (From update of attachment 207909 [details] [details])
> > LGTM. But, Ryuan, Mikhail or someone else might have a final look before landing.
> 
> OK, I don't want to prevent from landing but I still have a concern regarding the intended design (maybe it can be addressed later):
> 
> GestureRecognizer <-> GestureHandler relationships are still unclear to me. 
> 
> So far I see two perspectives: 
> 1) GestureRecognizer is(can be) efl-agnostic and shared between ports, than GestureHandler should be a common interface with port-specific implementation.
> My question is: is it possible? would qt/nix people like to re-use GestureRecognizer at least in theory?
Yes, GestureRecognizer can be shared if other ports want - especially NIX port because it referenced NIX's logic.
And GestureRecognizer should be re-factored when other ports start to use.
I can re-refactor that after all gesture codes for EFL are landed.

> 2) GestureRecognizer is efl specific, than what's the point in having  GestureHandler? if the point is only to make GestureRecognizer more readable I would make GestureHandler private and declare it in cpp file.
Yes, at this point, GestureHandler is separated from GestureRecognizer for readability and it can be private.
So, I will modify codes for that.
Gyuyoung, would you review again after I upload new patch?

[Gyuyoung Kim]

(In reply to comment #78)
> (In reply to comment #77)
> > (In reply to comment #76)
> > > (From update of attachment 207909 [details] [details] [details])
> > > LGTM. But, Ryuan, Mikhail or someone else might have a final look before landing.
> > 
> > OK, I don't want to prevent from landing but I still have a concern regarding the intended design (maybe it can be addressed later):
> > 
> > GestureRecognizer <-> GestureHandler relationships are still unclear to me. 
> > 
> > So far I see two perspectives: 
> > 1) GestureRecognizer is(can be) efl-agnostic and shared between ports, than GestureHandler should be a common interface with port-specific implementation.
> > My question is: is it possible? would qt/nix people like to re-use GestureRecognizer at least in theory?
> Yes, GestureRecognizer can be shared if other ports want - especially NIX port because it referenced NIX's logic.
> And GestureRecognizer should be re-factored when other ports start to use.

I wonder when nix port contributor starts to upload their gesture patch. Any nix port folk can say about it ?


> > 2) GestureRecognizer is efl specific, than what's the point in having  GestureHandler? if the point is only to make GestureRecognizer more readable I would make GestureHandler private and declare it in cpp file.
> Yes, at this point, GestureHandler is separated from GestureRecognizer for readability and it can be private.
> So, I will modify codes for that.
> Gyuyoung, would you review again after I upload new patch? 

Sure, no problem. Mikhail may want to take a look it as well :)

[Eunmi Lee]

Created attachment 208000 [details]
Patch

Make GestureClient private.

[Mikhail Pozdnyakov]

Comment on attachment 208000 [details]
Patch

lgtm

[Gyuyoung Kim]

Comment on attachment 208000 [details]
Patch

r=me based on informal review.

[Lauro Moura Maranhao Neto]

(In reply to comment #79)
> (In reply to comment #78)
> > (In reply to comment #77)
> > > (In reply to comment #76)
> > > > (From update of attachment 207909 [details] [details] [details] [details])
> > > > LGTM. But, Ryuan, Mikhail or someone else might have a final look before landing.
> > > 
> > > OK, I don't want to prevent from landing but I still have a concern regarding the intended design (maybe it can be addressed later):
> > > 
> > > GestureRecognizer <-> GestureHandler relationships are still unclear to me. 
> > > 
> > > So far I see two perspectives: 
> > > 1) GestureRecognizer is(can be) efl-agnostic and shared between ports, than GestureHandler should be a common interface with port-specific implementation.
> > > My question is: is it possible? would qt/nix people like to re-use GestureRecognizer at least in theory?
> > Yes, GestureRecognizer can be shared if other ports want - especially NIX port because it referenced NIX's logic.
> > And GestureRecognizer should be re-factored when other ports start to use.
> 
> I wonder when nix port contributor starts to upload their gesture patch. Any nix port folk can say about it ?
> 

We've already uploaded the WebCore parts in a bunch of bugs but most of them are waiting for review/approval. After that we'd upload the WK2 and Tools with MiniBrowser/GestureRecognizer.

metabug Nix: https://bugs.webkit.org/show_bug.cgi?id=117657
metabug WebCore: https://bugs.webkit.org/show_bug.cgi?id=117658
WebCore NIX directives: https://bugs.webkit.org/show_bug.cgi?id=118326
Nix files in Source/Platform: https://bugs.webkit.org/show_bug.cgi?id=118331
Stubs and platform files: https://bugs.webkit.org/show_bug.cgi?id=118358
Build files and scripts: https://bugs.webkit.org/show_bug.cgi?id=118367

[Gyuyoung Kim]

(In reply to comment #83)
> We've already uploaded the WebCore parts in a bunch of bugs but most of them are waiting for review/approval. After that we'd upload the WK2 and Tools with MiniBrowser/GestureRecognizer.

Will you put GestureRecognizer on MiniBrowser, not port layer ? Why ?

[Hugo Parente Lima]

(In reply to comment #84)
> (In reply to comment #83)
> > We've already uploaded the WebCore parts in a bunch of bugs but most of them are waiting for review/approval. After that we'd upload the WK2 and Tools with MiniBrowser/GestureRecognizer.
> 
> Will you put GestureRecognizer on MiniBrowser, not port layer ? Why ?

Yes, in the Nix philosophy gesture recognizer is an application business and the engine has nothing to do with it. This implies more code on the application side but OTOH the application will have more freedom to have whatever gesture it needs without depend on things implemented on Nix layer.

[Eunmi Lee]

(In reply to comment #85)
> (In reply to comment #84)
> > (In reply to comment #83)
> > > We've already uploaded the WebCore parts in a bunch of bugs but most of them are waiting for review/approval. After that we'd upload the WK2 and Tools with MiniBrowser/GestureRecognizer.
> > 
> > Will you put GestureRecognizer on MiniBrowser, not port layer ? Why ?
> 
> Yes, in the Nix philosophy gesture recognizer is an application business and the engine has nothing to do with it. This implies more code on the application side but OTOH the application will have more freedom to have whatever gesture it needs without depend on things implemented on Nix layer.

I understand.
Unlike NIX port, WK2 Efl implements default behaviors in the port directory.
So, let's think about sharing gesture recognizer later and implement gestures in the Efl port like reviewed patch.

Gyuyoung, could you commit the patch?

[Gyuyoung Kim]

(In reply to comment #85)
> (In reply to comment #84)
> > (In reply to comment #83)
> > > We've already uploaded the WebCore parts in a bunch of bugs but most of them are waiting for review/approval. After that we'd upload the WK2 and Tools with MiniBrowser/GestureRecognizer.
> > 
> > Will you put GestureRecognizer on MiniBrowser, not port layer ? Why ?
> 
> Yes, in the Nix philosophy gesture recognizer is an application business and the engine has nothing to do with it. This implies more code on the application side but OTOH the application will have more freedom to have whatever gesture it needs without depend on things implemented on Nix layer.

Ok, we have different direction to support gesture recognizer. I land this patch for EFL port at the moment.

[WebKit Commit Bot]

Comment on attachment 208000 [details]
Patch

Clearing flags on attachment: 208000

Committed r153770: <http://trac.webkit.org/changeset/153770>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.