Bug 132043 - TouchEvent is not handled after releasing any point among touched points.
Summary: TouchEvent is not handled after releasing any point among touched points.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eunmi Lee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-23 00:53 PDT by Eunmi Lee
Modified: 2014-04-28 09:22 PDT (History)
3 users (show)

See Also:


Attachments
Patch (1.51 KB, patch)
2014-04-23 01:12 PDT, Eunmi Lee
no flags Details | Formatted Diff | Diff
Patch (3.52 KB, patch)
2014-04-24 00:06 PDT, Eunmi Lee
no flags Details | Formatted Diff | Diff
Patch (2.93 KB, patch)
2014-04-27 17:45 PDT, Eunmi Lee
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eunmi Lee 2014-04-23 00:53:35 PDT
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.
Comment 1 Eunmi Lee 2014-04-23 01:12:35 PDT
Created attachment 229963 [details]
Patch
Comment 2 Benjamin Poulain 2014-04-23 20:36:33 PDT
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.
Comment 3 Eunmi Lee 2014-04-23 21:15:37 PDT
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.
Comment 4 Eunmi Lee 2014-04-24 00:06:27 PDT
Created attachment 230055 [details]
Patch
Comment 5 Benjamin Poulain 2014-04-25 14:43:16 PDT
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.
Comment 6 Eunmi Lee 2014-04-27 17:33:49 PDT
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.
Comment 7 Eunmi Lee 2014-04-27 17:45:21 PDT
Created attachment 230272 [details]
Patch
Comment 8 WebKit Commit Bot 2014-04-27 19:09:33 PDT
Comment on attachment 230272 [details]
Patch

Clearing flags on attachment: 230272

Committed r167867: <http://trac.webkit.org/changeset/167867>
Comment 9 WebKit Commit Bot 2014-04-27 19:09:42 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Simon Fraser (smfr) 2014-04-28 09:22:43 PDT
This broke the iOS build.