Bug 219255 - WTR: handle iframe elements in testdriver pointer move action
Summary: WTR: handle iframe elements in testdriver pointer move action
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-11-23 05:16 PST by Carlos Garcia Campos
Modified: 2021-02-03 05:19 PST (History)
6 users (show)

See Also:


Attachments
WIP patch (149.01 KB, patch)
2020-11-23 05:19 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
WIP patch (49.01 KB, patch)
2020-12-10 01:34 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
WIP patch (106.49 KB, patch)
2020-12-10 02:50 PST, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
WIP patch (108.50 KB, patch)
2020-12-10 04:38 PST, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (111.45 KB, patch)
2020-12-10 06:35 PST, Carlos Garcia Campos
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2020-11-23 05:16:42 PST
We are failing to process pointer move events because testdriver doesn't correctly handle iframe elements.
Comment 1 Carlos Garcia Campos 2020-11-23 05:19:29 PST
Created attachment 414800 [details]
WIP patch

This patch applies on top of patch attached to bug #219024. It fixes the timeout in imported/w3c/web-platform-tests/pointerevents/pointerevent_attributes_hoverable_pointers.html and imported/w3c/web-platform-tests/pointerevents/pointerlock/pointerevent_pointermove_in_pointerlock.html
Comment 2 Radar WebKit Bug Importer 2020-11-30 05:17:32 PST
<rdar://problem/71802566>
Comment 3 Carlos Garcia Campos 2020-12-10 01:34:02 PST
Created attachment 415846 [details]
WIP patch
Comment 4 EWS Watchlist 2020-12-10 01:34:39 PST
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 5 Carlos Garcia Campos 2020-12-10 02:50:57 PST
Created attachment 415849 [details]
WIP patch
Comment 6 Carlos Garcia Campos 2020-12-10 04:38:26 PST
Created attachment 415855 [details]
WIP patch
Comment 7 Carlos Garcia Campos 2020-12-10 06:35:57 PST
Created attachment 415871 [details]
Patch
Comment 8 Carlos Garcia Campos 2020-12-11 01:27:03 PST
The API test failure is unrelated.
Comment 9 Carlos Garcia Campos 2020-12-14 01:22:51 PST
Ping reviewers
Comment 10 Darin Adler 2021-02-01 15:07:18 PST
Comment on attachment 415871 [details]
Patch

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

> LayoutTests/imported/w3c/web-platform-tests/uievents/click/click_event_target_child_parent-expected.txt:11
> -PASS Click targets the nearest common ancestor
> +FAIL Click targets the nearest common ancestor assert_equals: Click should be sent to the nearest common ancestor expected "mousedown@link1,mouseup@link_container1,click@link_container1,mousedown@link_container2,mouseup@link2,click@link_container2" but got "mousedown@link1,mouseup@link_container1,mousedown@link_container2,mouseup@link2,click@link_container2"

Do you understand the cause of this one?
Comment 11 Carlos Garcia Campos 2021-02-02 01:13:54 PST
Comment on attachment 415871 [details]
Patch

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

>> LayoutTests/imported/w3c/web-platform-tests/uievents/click/click_event_target_child_parent-expected.txt:11
>> +FAIL Click targets the nearest common ancestor assert_equals: Click should be sent to the nearest common ancestor expected "mousedown@link1,mouseup@link_container1,click@link_container1,mousedown@link_container2,mouseup@link2,click@link_container2" but got "mousedown@link1,mouseup@link_container1,mousedown@link_container2,mouseup@link2,click@link_container2"
> 
> Do you understand the cause of this one?

hmm, I don't remember, I'll investigate. I have follow up patches in my local branch, I'll check them too.
Comment 12 Sam Sneddon [:gsnedders] 2021-02-03 05:19:25 PST
Comment on attachment 415871 [details]
Patch

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

Mildly surprised this doesn't progress anything in infrastructure/, given that has a number of tests for testdriver like this. But this generally looks good to me, modulo the comments below (and Darin's one above).

> LayoutTests/imported/w3c/web-platform-tests/resources/testdriver-vendor.js:24
> +      let frames = document.getElementsByTagName("iframe");
> +      for (let i = 0; i < frames.length; i++) {

Would it not be better to use window.frames here? As it is, this only handles iframes and not framesets. (This is likely a marginal issue, but an easy fix.)

> LayoutTests/imported/w3c/web-platform-tests/resources/testdriver-vendor.js:68
> +                        let frame = findElementInFrame(action.origin, window);
> +                        if (!frame)
> +                            return Promise.reject(new Error("Pointer origin element in different document or iframe."));

Given `findElementInFrame` above recurses, this seems slightly unclear. Maybe "Pointer origin element not in this document or descendent document"? I'm not really happy with that either, though.