[Webkit-unassigned] [Bug 214870] [WPE][Pointer Events] Add support for touch based pointer events

[bugzilla-daemon at webkit.org]

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

Carlos Alberto Lopez Perez <clopez at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #405352|review?                     |review-
              Flags|                            |

--- Comment #6 from Carlos Alberto Lopez Perez <clopez at igalia.com> ---
Comment on attachment 405352
  --> https://bugs.webkit.org/attachment.cgi?id=405352
Patch

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

Thanks! Patch looks good overall :)

But there are some issues that need to be fixed before landing:

1. If you can please upload a new version of it rebased and ensuring the the layout tests run on WPE (unskipping them, for that you have to put this line "pointerevents/ios [ Pass ]" in the file LayoutTests/platform/wpe/TestExpectations) and if they all pass, then a bonus point would be renaming that directory from ios to touch and also renaming the related lines on LayoutTests/platform/wpe/TestExpectations and LayoutTests/TestExpectations)

2. Please take a look at the previous comments from Chris and try to address them

I'm setting r- to make clear a new version of the patch is needed in order to continue with the review process.

> Source/WebCore/ChangeLog:9
> +        No new tests needed because we can reuse the tests from:
> +        LayoutTests/pointerevents/ios/

Those tests are skipped on the main TestExpectation file.
If they work for WPE as well, then this patch should unskip them on the WPE TestExpectation file.
It can be also a good idea to rename the directory to something like LayoutTests/pointerevents/touch as that seems more appropriate if they work in WPE

>> Source/WebCore/page/PointerCaptureController.cpp:336
>> +        dispatchEventForTouch(*touch->target(), PlatformTouchEvent() /* Don't care */, 0 /* Don't care */, isPrimary, view, type.string(), touch);
> 
> I don't think these /* Don't care */ comments are necessary.

Agree

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211125/259e2a0f/attachment-0001.htm>