Skip to content

Send a double click as a double click while driving - #478

Open
kevin9327 wants to merge 1 commit into
CopilotKit:mainfrom
kevin9327:fix/double-click-while-driving
Open

Send a double click as a double click while driving#478
kevin9327 wants to merge 1 commit into
CopilotKit:mainfrom
kevin9327:fix/double-click-while-driving

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

Take the wheel of a Bot's browser and double-click something — a row in a table, a node in a tree, a
word you want to replace. Nothing happens. The clicks land: the page highlights, the row takes focus.
It just never counts as a double click, and there is nothing on screen to say why. The way on is to
find the menu item that does the same thing, if there is one.

Why

Chrome fires dblclick on the page only when the second press tells it that it is the second.
Input.dispatchMouseEvent carries that as clickCount, and the live screen sent the same number
every time:

clickCount: kind === "moved" ? 0 : 1,

So two presses in the same place were two first clicks. No dblclick ever reached the page and
event.detail was always 1.

Measured, not reasoned

Driven through the shipped startScreencast against Chromium 151 (playwright-core 1.62.1's own
build), sending the two press-release pairs the surface builds for a double click, against a div
logging click and dblclick and a line of text to double-click on:

=== as the live screen sends today: clickCount 1, twice ===
  events the page saw : click detail=1 | click detail=1
  word selected       : ""

=== with the browser's own click count: 1 then 2 ===
  events the page saw : click detail=1 | click detail=2 | DBLCLICK detail=2
  word selected       : "Alpha "

The fix

MouseEvent.detail is how many times in a row that button has been pressed in the same place,
worked out by the person's own browser to its own timing and distance rules. That is exactly what
clickCount means, so it is sent rather than reconstructed on the far side — a second implementation
of double-click timing is the wrong thing for either end of this to be holding.

clickCount: kind === "moved" ? 0 : Math.max(1, event.detail),

Floored at one on a press, because screencast.ts refuses a press of zero for the reason its own
comment gives — Chrome would see a move that happens to have a button set, and no click at all — and
detail is zero on an event a script dispatched rather than a person.

screencast.ts already forwards message.clickCount ?? 1, so nothing on the computer changes.

Tests

New: app/tests/live-screen-mouse.test.tsx, 4 tests, on the harness
live-screen-keyboard.test.tsx already uses — a socket double, plus one frame delivered so the
canvas knows what size to map a click against.

  • Against origin/main (1c7bd92) with only the new file applied: 3 pass, 1 fail.
expect(socket.sent.map((message) => message.clickCount)).toEqual([1, 1, 2, 2]);
error: expect(received).toEqual(expected)
@@ -3,4 +3,4 @@
    1,
-   2,
-   2,
+   1,
+   1,
  ]
(fail) a double click is sent as a double click
  • With the change: 4 pass, 0 fail.

The other three are the guard against over-correcting and pass before and after: an ordinary click is
still exactly one click with the same payload it always had, a press whose detail is zero is still
sent as one click rather than none, and a move is still not a click.

bun test app/tests shared379 pass, 0 fail (375 on origin/main; the four new ones are the
whole difference).

Also run, clean: bunx biome check on both files and bun run --filter app typecheck.

Note on the changelog

A person driving a browser behaves differently afterwards, so there is an entry. It goes at the top
of ## Unreleased, the line every open PR touching the changelog inserts at, so it will conflict
with any other that lands first. Happy to rebase.

Every press the live screen sent claimed to be the first click in its place.
Chrome fires `dblclick` on the far page only when the second press says it is the
second, so a person holding the wheel could not double-click at all.

Measured against Chromium 151 through the shipped `startScreencast`, with the two
press-release pairs the surface builds. As sent today the page logged
`click detail=1 | click detail=1`, no `dblclick`, and double-clicking a word
selected nothing. With the browser's own count it logged
`click detail=1 | click detail=2 | DBLCLICK detail=2` and selected the word.

`MouseEvent.detail` is the count the person's browser already worked out, to its
own timing and distance rules, so it is sent rather than reconstructed. Floored at
one on a press, because the computer refuses a press of zero for the reason its
own comment gives, and `detail` is zero on an event a script dispatched.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant