Skip to content

fix(bidi): guard nullable navigation fields in event handlers#40763

Open
SebTardif wants to merge 2 commits into
microsoft:mainfrom
SebTardif:fix-bidi-navigation-null
Open

fix(bidi): guard nullable navigation fields in event handlers#40763
SebTardif wants to merge 2 commits into
microsoft:mainfrom
SebTardif:fix-bidi-navigation-null

Conversation

@SebTardif
Copy link
Copy Markdown
Contributor

@SebTardif SebTardif commented May 10, 2026

Summary

  • Guard params.navigation (typed Navigation | null) in _onNavigationStarted and _onNavigationCommitted instead of using non-null assertions
  • Add null check for frame(frameId) in _onNavigationCommitted to handle destroyed frames
  • Aligns with the existing pattern in _onNavigationAborted (line 217), _onNavigationFailed (line 221), and _onFragmentNavigated (line 225)

Introduced in #32434 (2024-09-04), refined in #35820 and #38129.

@pavelfeldman
Copy link
Copy Markdown
Member

pavelfeldman commented May 10, 2026

This would beed a test that is failing w/o the fix.

@SebTardif
test(bidi): add test for null navigation handling
d02929f 
Add test to verify BiDi navigation events with null navigation are
handled gracefully. Tests navigation to about:blank, history.pushState,
and goBack scenarios.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2026

Test results for "MCP"

32 failed
❌ [chrome] › mcp/annotate.spec.ts:173 › user-initiated annotate downloads zip with feedback.md @mcp-macos-latest-chrome
❌ [chrome] › mcp/annotate.spec.ts:173 › user-initiated annotate downloads zip with feedback.md @mcp-windows-latest-chrome
❌ [chrome] › mcp/annotate.spec.ts:230 › should capture annotations via show --annotate @mcp-windows-latest-chrome
❌ [chrome] › mcp/annotate.spec.ts:251 › should start dashboard and annotate when no dashboard is running @mcp-windows-latest-chrome
❌ [chrome] › mcp/annotate.spec.ts:273 › should enter annotate mode on fresh dashboard.tsx mount with -s --annotate @mcp-windows-latest-chrome
❌ [chrome] › mcp/annotate.spec.ts:297 › should annotate via direct browser_annotate MCP call @mcp-windows-latest-chrome
❌ [chrome] › mcp/annotate.spec.ts:330 › should annotate when context has no fixed viewport @mcp-windows-latest-chrome
❌ [chrome] › mcp/annotate.spec.ts:367 › should cancel browser_annotate when the MCP request is aborted @mcp-windows-latest-chrome
❌ [chrome] › mcp/annotate.spec.ts:398 › should cancel browser_annotate when the MCP client disconnects @mcp-windows-latest-chrome
❌ [chrome] › mcp/annotate.spec.ts:427 › should switch screencast to -s session on show --annotate @mcp-windows-latest-chrome
❌ [chrome] › mcp/annotate.spec.ts:476 › should disengage annotate mode when --annotate client disconnects @mcp-windows-latest-chrome
❌ [chrome] › mcp/cli-core.spec.ts:31 › close @mcp-windows-latest-chrome
❌ [chrome] › mcp/cli-devtools.spec.ts:217 › video-start-stop @mcp-windows-latest-chrome
❌ [chrome] › mcp/cli-devtools.spec.ts:231 › video-chapter @mcp-windows-latest-chrome
❌ [chrome] › mcp/cli-json.spec.ts:177 › close-all after open returns closed sessions @mcp-windows-latest-chrome
❌ [chrome] › mcp/cli-session.spec.ts:44 › close named session @mcp-windows-latest-chrome
❌ [chrome] › mcp/cli-session.spec.ts:56 › close-all @mcp-windows-latest-chrome
❌ [chrome] › mcp/cli-session.spec.ts:70 › delete-data @mcp-windows-latest-chrome
❌ [chrome] › mcp/cli-session.spec.ts:82 › delete-data named session @mcp-windows-latest-chrome
❌ [chrome] › mcp/cli-session.spec.ts:99 › session stops when browser exits @mcp-windows-latest-chrome
❌ [chrome] › mcp/cli-session.spec.ts:113 › session reopen with different config @mcp-windows-latest-chrome
❌ [chrome] › mcp/cli-session.spec.ts:130 › workspace isolation - sessions in different workspaces are isolated @mcp-windows-latest-chrome
❌ [chrome] › mcp/cli-session.spec.ts:162 › list --all lists sessions from all workspaces @mcp-windows-latest-chrome
❌ [webkit] › mcp/annotate.spec.ts:273 › should enter annotate mode on fresh dashboard.tsx mount with -s --annotate @mcp-windows-latest-webkit
❌ [webkit] › mcp/annotate.spec.ts:297 › should annotate via direct browser_annotate MCP call @mcp-windows-latest-webkit
❌ [webkit] › mcp/annotate.spec.ts:330 › should annotate when context has no fixed viewport @mcp-windows-latest-webkit
❌ [webkit] › mcp/annotate.spec.ts:367 › should cancel browser_annotate when the MCP request is aborted @mcp-windows-latest-webkit
❌ [webkit] › mcp/annotate.spec.ts:398 › should cancel browser_annotate when the MCP client disconnects @mcp-windows-latest-webkit
❌ [webkit] › mcp/annotate.spec.ts:427 › should switch screencast to -s session on show --annotate @mcp-windows-latest-webkit
❌ [webkit] › mcp/annotate.spec.ts:476 › should disengage annotate mode when --annotate client disconnects @mcp-windows-latest-webkit
❌ [webkit] › mcp/cli-devtools.spec.ts:217 › video-start-stop @mcp-windows-latest-webkit
❌ [webkit] › mcp/cli-devtools.spec.ts:231 › video-chapter @mcp-windows-latest-webkit

7025 passed, 1068 skipped

Merge workflow run.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2026

Test results for "tests 1"

1 flaky ⚠️ [chromium-library] › library/video.spec.ts:719 › screencast › should work with video+trace `@chromium-ubuntu-22.04-node24`

41733 passed, 857 skipped

Merge workflow run.

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

Reviewers

@pavelfeldman pavelfeldman pavelfeldman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@SebTardif @pavelfeldman