fix(locator): serialize click input events - #2527
Open
alectimison-maker wants to merge 1 commit into
Open
Conversation
🦋 Changeset detectedLatest commit: d46e3e3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
This PR is from an external contributor and must be approved by a stagehand team member with write access before CI can run. |
Contributor
There was a problem hiding this comment.
No issues found across 3 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant Test as Test Suite
participant Locator as Locator.click()
participant CDP as CDP Session
participant Page as Page (Browser Event Loop)
Note over Test,Page: NEW: Serialized mouse event dispatch flow
Test->>Locator: locator.click({ clickCount: 2 })
Locator->>CDP: Input.dispatchMouseEvent (type: "mouseMoved")
Note over Locator,CDP: CHANGED: await before next event
CDP-->>Page: CDP command → browser processes mouse move
CDP-->>Locator: response (success)
alt Click 1
Locator->>CDP: Input.dispatchMouseEvent (type: "mousePressed", clickCount: 1)
CDP-->>Page: browser processes press
Locator->>CDP: Input.dispatchMouseEvent (type: "mouseReleased", clickCount: 1)
CDP-->>Page: browser processes release
end
alt Click 2
Locator->>CDP: Input.dispatchMouseEvent (type: "mousePressed", clickCount: 2)
CDP-->>Page: browser processes press
Locator->>CDP: Input.dispatchMouseEvent (type: "mouseReleased", clickCount: 2)
CDP-->>Page: browser processes release
end
CDP-->>Locator: all responses
Locator-->>Test: undefined
Note over Test,CDP: Regression test: detects overlapping events
Test->>Locator: locator.click() with mock that fails on overlap
Locator->>CDP: Input.dispatchMouseEvent (parallel in old code)
alt If events overlap (old behavior)
CDP-->>Locator: Error("mouse events overlapped")
Locator-->>Test: rejected promise
Test-->>Test: test fails
else If serialized (new behavior)
Locator->>CDP: await each sequentially
CDP-->>Locator: success
Locator-->>Test: undefined
end
Note over Locator,CDP: Page.click() path remains unchanged (not shown)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
why
Locator.click()currently startsmouseMoved,mousePressed, andmouseReleasedCDP commands together and awaits them withPromise.all. Areactive UI can therefore receive the press before its hover/pointer state has
settled, while Stagehand still reports a successful click. This matches the
failure described in #2486.
Stagehand prioritizes reliability over speed, so this change makes the
locator-only input sequence explicit and ordered. The separate coordinate
Page.clickpath is intentionally unchanged.what changed
event
overlap and verifies the full double-click sequence
@browserbasehq/stagehandtest plan
with
mouse events overlapped, then passed after serializationpnpm --filter @browserbasehq/stagehand run lintpnpm --filter @browserbasehq/stagehand run build(ESM and CJS)prettier --checkon all changed filesThe repository's post-test CTRF converter prints a pre-existing
glob/minimatchexport warning after successful standard test runs. RunningVitest directly avoids that converter; the complete core unit suite passes.
Summary by cubic
Serialize
Locator.click()mouse events to enforce ordered hover → press → release. This improves click reliability in reactive UIs;Page.clickremains unchanged.mouseMoved, thenmousePressed, thenmouseReleased(no overlapping CDP commands).@browserbasehq/stagehand.Written for commit d46e3e3. Summary will update on new commits.