Skip to content

fix: forward scroll options for pointer actions - #3358

Open
Mihail Galay (GalayM) wants to merge 3 commits into
microsoft:mainfrom
GalayM:fix/forward-scroll-options
Open

Mihail Galay (GalayM) wants to merge 3 commits into
microsoft:mainfrom
GalayM:fix/forward-scroll-options

Conversation

@GalayM

@GalayM Mihail Galay (GalayM) commented Sep 13, 2026

Copy link
Copy Markdown

ScrollMode.None currently scrolls offscreen targets into view because the option is lost before reaching the driver. Forward it through Page's option conversions and the Frame/ElementHandle messages for click, double-click, hover, tap, checkbox actions and drag-and-drop.

Adds browser regression coverage for all four API entry points, document and nested scrolling, visible targets, None/Auto/omitted values, and non-forced click retries.

Validated all 211 regression cases on Chromium, Firefox and WebKit. The full CI browser/platform matrix passes. The related Chromium suites on net10.0 pass 304 tests, with 2 existing upstream skips. dotnet build ./src and dotnet format ./src/ --verify-no-changes -v:diag pass.

A separate prerequisite commit moves ResolveNpmExecutable before instance methods without changing its body. This fixes the existing SA1204 formatting failure on the base branch.

Fixes #3357

Copilot AI lite review requested due to automatic review settings September 13, 2026 11:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are narrowly scoped to forwarding an existing option through the last-mile protocol payloads and are backed by targeted regression tests covering the affected action entry points.

Pull request overview

This PR fixes a regression where ScrollMode.None was not preserved for pointer-based actions, causing offscreen targets to be scrolled into view despite the caller explicitly opting out. It forwards the Scroll option through Page-level option conversions and ensures Frame/ElementHandle protocol messages include the scroll field, with new regression tests covering Page/Frame/Locator/ElementHandle entry points and drag-and-drop.

Changes:

  • Forward Scroll from Page*Options into the corresponding Frame*Options conversions in Page.cs.
  • Include scroll in the protocol payloads sent by Frame and ElementHandle for click/dblclick/hover/tap/check/uncheck/setChecked and drag-and-drop.
  • Add a dedicated ScrollOptionsTests regression suite and reorder a tooling helper to satisfy SA1204 member ordering.
File summaries
File Description
src/tools/Playwright.Tooling/DriverDownloader.cs Moves ResolveNpmExecutable above instance members to fix SA1204 ordering without functional changes.
src/Playwright/Core/Page.cs Preserves Scroll when converting Page-level action options into Frame action options.
src/Playwright/Core/Frame.cs Forwards scroll to the driver in Frame action protocol messages (pointer actions + drag-and-drop).
src/Playwright/Core/ElementHandle.cs Forwards scroll to the driver in ElementHandle action protocol messages (pointer actions).
src/Playwright.Tests/ScrollOptionsTests.cs Adds regression coverage validating ScrollMode.None vs Auto/omitted across API entry points, nested/document scrolling, and drag-and-drop.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

[Bug]: ScrollMode.None is ignored by pointer actions

2 participants