TouchEvent is not handled after releasing any point among touched points.

• Component: WebKit2
• Status: RESOLVED FIXED

[Eunmi Lee]

m_isTrackingTouchEvents is set to false when we release any touched point,
so we can not process rest of touched points.

In the EFL port's unit test, we test as follows and it fails because #5 and #6 is not processed.
1. first touch start -> 2. second touch start -> 3. second touch move -> 4. second touch end
-> 5. first touch move -> 6. first touch end

So, I will modify code to set m_isTrackingTouchEvents to false when all touched points are released.

[Eunmi Lee]

Created attachment 229963 [details]
Patch

[Benjamin Poulain]

Comment on attachment 229963 [details]
Patch

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

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1532
> -    if (event.type() == WebEvent::TouchEnd || event.type() == WebEvent::TouchCancel)
> +    if ((event.touchPoints().size() <= 1 && event.type() == WebEvent::TouchEnd) || event.type() == WebEvent::TouchCancel)

You are right, I completely mishandled multitouch here. :(

Unfortunately checking for the size of touchPoints() is not enough. You could have two touch points simultaneously in touchEnd or touchCancel. You will need to loop over the touch points and find if they are all released or canceled.

Can you please do that in a little utility function? I separated the iOS touch event dispatch recently and the same bug exist in there.

[Eunmi Lee]

Comment on attachment 229963 [details]
Patch

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

>> Source/WebKit2/UIProcess/WebPageProxy.cpp:1532
>> +    if ((event.touchPoints().size() <= 1 && event.type() == WebEvent::TouchEnd) || event.type() == WebEvent::TouchCancel)
> 
> You are right, I completely mishandled multitouch here. :(
> 
> Unfortunately checking for the size of touchPoints() is not enough. You could have two touch points simultaneously in touchEnd or touchCancel. You will need to loop over the touch points and find if they are all released or canceled.
> 
> Can you please do that in a little utility function? I separated the iOS touch event dispatch recently and the same bug exist in there.

Thanks for review :)
I will update new patch.

[Eunmi Lee]

Created attachment 230055 [details]
Patch

[Benjamin Poulain]

Comment on attachment 230055 [details]
Patch

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

Thanks for fixing this!

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1496
> +bool WebPageProxy::areAllTouchPointsReleased(const WebTouchEvent& event) const

This should not be a member of WebPageProxy, the function does not need to access any of the states of WebPageProxy, it should be an utility static function in the implementation file.

[Eunmi Lee]

Comment on attachment 230055 [details]
Patch

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

>> Source/WebKit2/UIProcess/WebPageProxy.cpp:1496
>> +bool WebPageProxy::areAllTouchPointsReleased(const WebTouchEvent& event) const
> 
> This should not be a member of WebPageProxy, the function does not need to access any of the states of WebPageProxy, it should be an utility static function in the implementation file.

yes, right. I will update new patch for that.

[Eunmi Lee]

Created attachment 230272 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 230272 [details]
Patch

Clearing flags on attachment: 230272

Committed r167867: <http://trac.webkit.org/changeset/167867>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

This broke the iOS build.