Input tools can leave keys and the mouse button held down; the root cause is an unguarded Keyboard.press() in Puppeteer

[Liohtml]

#2347 fixed one unpaired-keyDown in press_key, and I claimed at the time that it was the only instance. That was wrong. I went back and audited every input path, and verified each finding below against real Chrome and the vendored puppeteer-core@25.3.0 — no theorising this time.

There is a single root cause, and most of the damage is not in this repo.

Root cause: Keyboard.press() has no try/finally

puppeteer-core/lib/puppeteer/cdp/Input.js:

async press(key, options = {}) {
  await this.down(key, options);   // keyDown dispatched
  await this.up(key);              // if this rejects, the key stays down
}

A rejecting up() — a CDP hiccup, a detached session, a response timeout — leaves the key logically held down in the renderer. Every input path below is a consequence of this.

Where it surfaces

1. press_key — main key left held

Verified on main: with a transient failure on the main key's key up, the page sees
["dControl","dShift","dC","uShift","uControl"] — no uC. #2347 released only the modifiers. PR #2351 fixes this one (split down/up so the release can be retried).

2. fill / fill_form — key left held, and the tool reports success

This is the one I'd prioritise. Locator.fill() types short values (< typingThreshold, default 100 chars) through handle.type() → Keyboard.type(), which presses each character with the same unguarded press() (locators.js, the typeable-input branch → from(handle.type(textToType))).

Repro — locator('#i').fill('abc') with a transient failure on b's key up:

key events seen by page: [ 'da', 'ua', 'db', 'dc', 'uc' ]
                                      ^^^^ no 'ub' — "b" is still down
fill() did NOT throw.

The Locator's retry re-reads the input value, types the remainder, and resolves successfully. So the tool reports success while leaving a key held down in the renderer. Silent corruption — the agent has no way to know. (This rhymes with #2199.)

selectNativeSelectOption and selectOption go through Locator.fill() too.

3. type_text — same, via Keyboard.type()

4. drag — mouse button left held, and the obvious fix is a trap

ElementHandle.drag() calls page.mouse.down(); only drop() calls page.mouse.up(). Drag interception is never enabled here (setDragInterception has zero hits repo-wide), so this is the live path. A failing drop() leaves the left mouse button held down, breaking every later click.

I wrote the obvious fix — release the mouse on the failure path — and it makes things worse. drop() clears page._isDragging only on success:

await dataOrElement.drag(this);
page._isDragging = false;      // never reached if drag/drop throws
await page.mouse.up();

So releasing the button without clearing that flag leaves _isDragging === true, and the next drag takes the if (!page._isDragging) branch, skips mouse.down() entirely, and dies. Verified against real Chrome:

drop threw: drop failed
logs after failed drag  : [ 'down', 'up' ]     <- button released, fix "worked"
page._isDragging LEAKED?: true
second drag FAILED      : 'left' is not pressed.
second drag logs        : []                   <- no mousedown ever dispatched

A stuck button traded for a permanently broken drag tool. _isDragging isn't in types.d.ts, so clearing it means reaching into a Puppeteer internal — which is why I pulled this out of #2351 rather than guess. This one needs your call.

Not affected

Mouse.click() — it eagerly creates both the down() and up() promises before Promise.all, and click/click_at never pass a delay. (It is not a general property: on the delay path up() is created after the await, so it would leak. Worth knowing if a delay option is ever added.)

Suggested direction

The high-leverage fix is upstream. A try/finally in Puppeteer's Keyboard.press() closes press_key, type_text, fill, fill_form and select in one change, and fixes it for every Puppeteer user rather than just this server. Working around it here would mean reimplementing type() on top of the non-public charIsKey(), which I don't think is right.

drag needs handling separately, either upstream (drag()/drop() should release the mouse and clear _isDragging in a finally) or here, if you're comfortable touching the internal.

Happy to do the work — send the Puppeteer PR, and/or a drag fix here once you've said which way you want it. Just didn't want to write more code before checking, having already got ahead of myself once on #2283.

Repro scripts for the fill and drag findings available on request; both are ~30 lines of plain puppeteer-core.

[OrKoN]

Before we dig into this, what are the realistic reasons for down to fail but a recovery up to succeed? It sounds like this is hypothetical, let's try to find some practical scenarios where this leads to isses.

[Liohtml]

Fair challenge, and you're right on the keyboard half. Let me split the issue, because I conflated two things with very different evidence.

The keyboard cases are hypothetical — I concede

Every keyboard scenario I described (press_key, type_text, fill) needs a dispatchKeyEvent to reject while the session stays healthy enough for the recovery up() to land. I injected that failure with a stub. I do not have a realistic trigger for it, and your instinct is the right one: if a CDP command fails, the session is usually gone and the recovery would fail too.

One thing I got wrong in my own reasoning, while I'm at it: I argued a rejected down() latches the modifier bit. It does — but up() clears _modifiers before it sends, so the bit is repaired as long as up() is called, whether or not it succeeds. The client-state argument I made is therefore moot.

So unless you can think of a real trigger I can't, #2351 is defensive programming against a failure I can't demonstrate. Happy to close it. The one thing I'd still argue for on its own merit is guarding the release loop so a failing up() can't discard the original error — but that's a small cleanup, not a bug fix, and not worth a PR by itself.

The drag case needs no CDP failure at all — and it's real

This one stands on its own, and it isn't the same root cause. drop() throws for an entirely ordinary reason: the page re-rendered during the drag, which is what every reactive sortable list does. No stubbing, no injected failure:

// ordinary list that re-renders on mousemove, like React/Vue would
await from.drag(to);            // presses the mouse button down
await to.drop(from);            // <- throws

drop threw NATURALLY: Node is detached from document
mouse events  : [ 'down' ]          <- left mouse button still held

drop() re-enters drag(), which calls scrollIntoViewIfNeeded() on the now-detached source node. drag() pressed the button; only drop() releases it; so the button stays down.

The consequence is user-visible, not theoretical. The agent's next click fires the handler twice:

await page.locator('#btn').click();
// mouse events: [ 'up', 'CLICK', 'down', 'up', 'CLICK' ]
//                 ^^^^^^^^^^^^^ phantom click from the stuck button

One click call, two click events on the target. An agent that clicks "Delete" or "Submit" after a failed drag hits it twice. Every subsequent click on the page is affected until the button is released.

And the obvious fix is a trap, which is why I'm asking rather than sending a patch: releasing the mouse in the failure path leaves page._isDragging === true (it's only cleared on drop()'s success path), so the next drag skips mouse.down() and dies with 'left' is not pressed. — verified. Clearing that flag means touching a Puppeteer internal that isn't in types.d.ts.

To me this looks like a Puppeteer bug — drag()/drop() should release the button and clear _isDragging in a finally. Want me to send that upstream, or would you rather work around it here? Repro is ~30 lines of plain puppeteer-core, happy to paste it.

[OrKoN]

I think we should fix such an issue in Puppeteer but can you confirm that the mouse state is not reset after a navigation? How does the state that is not reset affect the caller?

[Liohtml]

Yeah — not reset.

CdpMouse keeps the button bitmask on the Mouse instance, and the Mouse hangs off the Page:

#_state = { position: {x: 0, y: 0}, buttons: 0 };
updateClient(client) { this.#client = client; }

updateClient() swaps the session on navigation and leaves #_state alone. Nothing clears it.

Side note: Mouse.reset() already exists and releases held buttons, but nothing in Puppeteer ever calls it.

For the caller. Reactive list re-renders mid-drag, drop() throws Node is detached from document, button stays down. Then one plain click():

no navigation
  after the failed drag :  [ 'mousedown', 'selectstart' ]
  one click() produced  :  [ 'mouseup', 'click', 'mousedown', 'mouseup', 'click' ]

after goto() to a fresh document
  one click() produced  :  [ 'mouseup', 'mousedown', 'mouseup', 'click' ]

Three things fall out of that:

• The next click fires the target's handler twice. The stale mousedown pairs with the phantom mouseup, the browser makes a real click out of it, then the intended one lands. Agent clicks Delete after a failed drag, it deletes twice. That's the bit I'd care about.
• The held button starts a selection (selectstart), so any later hover/move drags a text selection around the page.
• The press survives the navigation and shows up in the next document as a stray mouseup.

Also I need to walk something back. I said page._isDragging is left true, so releasing the button would break the next drag. That was my stub, not reality — in the natural failure drop() re-enters drag(), and drag's own catch puts it back:

drop threw             : Node is detached from document
_isDragging right after: false

So the flag is fine, and the missing button release is the whole bug. Should have reproduced that one properly the first time.

Fix is a finally around the press in drag()/drop(), or just falling back to Mouse.reset(). Want me to open it on Puppeteer with the above as a test?

[OrKoN]

updateClient() swaps the session on navigation

there should be no updateClient() unless this is a prerendering navigation. Yeah, I think this should be fixed in Puppeteer but cleaning up the state if the element is not longer available. Happy to review a PR!

[Liohtml]

You're right about updateClient() — I pointed at the wrong mechanism. It isn't a session swap that carries the state over; there's simply nothing that clears it. The Mouse hangs off the Page, #_state.buttons lives on the Mouse, and an ordinary same-target navigation replaces the document without touching either. Same conclusion, wrong reason on my part.

Will send the Puppeteer PR. Cleaning up when the element is gone sounds right — drop() re-enters drag(), scrollIntoViewIfNeeded() throws Node is detached from document, and the button that drag() pressed never gets released. I'll release in a finally there rather than special-case the detached error, so a throw anywhere between the press and the drop can't strand it.

Will use the repro from above as the test: list re-renders on mousemove, drop throws, then assert one click() doesn't produce two click events.

I'll link it here.

[Liohtml]

Opened it as a draft: puppeteer/puppeteer#15231. Releases the button in drag() when that call pressed it and the drop will never run, and in drop() when the drag it re-enters throws — neither release masks the original error, success path untouched. Test re-renders the page mid-drag so the drop fails naturally (no stubbing), then asserts the button is no longer pressed. Fails on main, passes with the change.