Skip to content

chore: pass markTargets explicitly into action methods#40705

Open
dgozman wants to merge 1 commit intomicrosoft:mainfrom
dgozman:mark-targets-pr
Open

chore: pass markTargets explicitly into action methods#40705
dgozman wants to merge 1 commit intomicrosoft:mainfrom
dgozman:mark-targets-pr

Conversation

@dgozman
Copy link
Copy Markdown
Collaborator

@dgozman dgozman commented May 7, 2026

Summary

  • Server-side action implementations no longer read progress.metadata.id to mark DOM elements for trace highlighting; the token is now an explicit markTargets?: string option threaded through action methods and helpers.
  • Frame / ElementHandle dispatchers spread markTargets: progress.metadata.id into the call.
  • Injected markTargetElements parameter renamed from callId to markTargets for consistency.
  • New MarkTargetsOptions base type in server/types.ts; StrictOptions extends it so CommonActionOptions / PointerActionWaitOptions inherit the field automatically.

Server-side action implementations read progress.metadata.id directly
to highlight resolved DOM elements in trace snapshots. Make the token
an explicit option (`markTargets`) on the action methods, and have the
frame / element-handle dispatchers pass it from the call metadata.

Renames the parameter from `callId` to `markTargets` in
injectedScript.markTargetElements and along the call chain.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Test results for "MCP"

5 failed
❌ [chrome] › mcp/annotate.spec.ts:57 › should capture multiple screenshots in one annotation @mcp-windows-latest-chrome
❌ [chrome] › mcp/annotate.spec.ts:110 › should abort annotation when last screenshot is removed @mcp-windows-latest-chrome
❌ [chrome] › mcp/annotate.spec.ts:137 › should abort MCP annotation when last screenshot is removed @mcp-windows-latest-chrome
❌ [msedge] › mcp/test-run.spec.ts:126 › test_run should stop when aborted @mcp-windows-latest-msedge
❌ [webkit] › mcp/config.spec.ts:138 › browser_get_config returns merged config from file, env and cli @mcp-windows-latest-webkit

7013 passed, 1068 skipped


Merge workflow run.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Test results for "tests 1"

4 flaky ⚠️ [chromium-library] › library/video.spec.ts:682 › screencast › should capture full viewport on hidpi `@ubuntu-22.04-chromium-tip-of-tree`
⚠️ [chromium-library] › library/popup.spec.ts:261 › should not throw when click closes popup `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:682 › screencast › should capture full viewport on hidpi `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:719 › screencast › should work with video+trace `@chromium-ubuntu-22.04-node20`

41651 passed, 851 skipped


Merge workflow run.


async getAttribute(params: channels.ElementHandleGetAttributeParams, progress: Progress): Promise<channels.ElementHandleGetAttributeResult> {
const value = await this._elementHandle.getAttribute(progress, params.name);
const value = await this._elementHandle.getAttribute(progress, params.name, { markTargets: progress.metadata.id });
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This does not make much sense to me! Why are you passing this?

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.

2 participants