Closed Bug 1864614 Opened 6 months ago Closed 5 months ago

[remote] DoubleClickTracker doesn't track multiple click sequences

Categories

(Remote Protocol :: WebDriver BiDi, defect, P2)

Product:
Remote Protocol
Component:
WebDriver BiDi
Type:
defect
Points:
3

Tracking

(firefox122 fixed)

Status:
RESOLVED FIXED
Milestone:
122 Branch
Tracking Flags:
Tracking Status
firefox122 --- fixed

People

(Reporter: whimboo, Assigned: Sasha)

References

(Blocks 2 open bugs)

Details

(Keywords: perf-alert, Whiteboard: [webdriver:m9], [wptsync upstream][webdriver:relnote])

Attachments

(2 files, 1 obsolete file)

Bug 1864614 - [remote] Refactor DoubleClickTracker to work with multiple clicks sequences.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1864614 - [wdspec] Update tests for multiple clicks in different command calls.
48 bytes, text/x-phabricator-request
Details | Review

Currently the code for WebDriver BiDi doesn't correctly use the DoubleClickTracker and as such no dblclick events are getting emitted. Specifically here is the call to DoubleClickTracker.setClick() which increments the internal counter.

Marionette is doing that correctly in MarionetteEventsChild.sys.mjs.

Fixing that is removing a blocker for bug 1841008 to get triple clicks working as well.

I wonder if we have to share the dblclick state between Marionette and WebDriver BiDi, but I think that this is really a very rare situation so that we won't have to do that.

James, do you think otherwise?

Flags: needinfo?(james)

It seems fine. My opinion is that for actions specifically the click-tracking state can be localised to a specific actions sequence (so you can guarantee a set of single clicks by breaking up the code into multiple action sequences). That also naturally means that you can't get a double click with a combination of HTTP and BiDi calls, so we don't need to share state between those.

Flags: needinfo?(james)

Thanks James. So lets consolidate the Marionette code with the Action implementation, which sounds fine and simplifies the code as well.

Given that this is a P1 blocker for Puppeteer I'm already going ahead and adding it for M9.

Priority: -- → P2
Whiteboard: [webdriver:m9]
Blocks: 1792089
Points: --- → 3
Severity: -- → S3
Assignee: nobody → aborovova
Status: NEW → ASSIGNED

Remove empty lines in the test data to avoid differences in getSelection behaviour.

Depends on D194614

Sasha, should we dupe bug 1841008 against this one given that you handle multi-click on this bug as well? If yes then we should probably rename this bug's summary as well.

Flags: needinfo?(aborovova)
Flags: needinfo?(aborovova)
Summary: "input.performActions" doesn't simulate "dblclick" events → [remote] Refactor DoubleClickTracker to work with multiple clicks sequences.
Duplicate of this bug: 1841008
Summary: [remote] Refactor DoubleClickTracker to work with multiple clicks sequences. → [remote] DoubleClickTracker doesn't track multiple click sequences

Comment on attachment 9365333 [details]
Bug 1864614 - [wdspec] Fix tests for triple clicks.

Revision D194615 was moved to bug 1795327. Setting attachment 9365333 [details] to obsolete.

Attachment #9365333 - Attachment is obsolete: true
Pushed by aborovova@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a5657402cf23
[remote] Refactor DoubleClickTracker to work with multiple clicks sequences. r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/212399303055
[wdspec] Update tests for multiple clicks in different command calls. r=webdriver-reviewers,whimboo
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/43413 for changes under testing/web-platform/tests
Whiteboard: [webdriver:m9] → [webdriver:m9], [wptsync upstream]

https://hg.mozilla.org/mozilla-central/rev/a5657402cf23
https://hg.mozilla.org/mozilla-central/rev/212399303055

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
status-firefox122: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 122 Branch
Upstream PR merged by moz-wptsync-bot
Blocks: 1681076

(In reply to Pulsebot from comment #9)

Pushed by aborovova@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a5657402cf23
[remote] Refactor DoubleClickTracker to work with multiple clicks sequences.
r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/212399303055
[wdspec] Update tests for multiple clicks in different command calls.
r=webdriver-reviewers,whimboo

== Change summary for alert #40542 (as of Mon, 04 Dec 2023 19:50:28 GMT) ==

Improvements:

Ratio Test Platform Options Absolute values (old vs new)
1% Base Content JS windows11-64-2009-shippable-qr fission 1,549,499.69 -> 1,528,896.00
1% Base Content JS linux1804-64-shippable-qr fission 1,546,895.33 -> 1,526,984.00
1% Base Content JS linux1804-64-shippable-qr fission 1,546,820.67 -> 1,526,984.00
1% Base Content JS macosx1015-64-shippable-qr fission 1,547,565.33 -> 1,527,784.00

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=40542

Keywords: perf-alert
Whiteboard: [webdriver:m9], [wptsync upstream] → [webdriver:m9], [wptsync upstream][webdriver:relnote]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: