Skip to content

fix(trace): capture attribute mutations after a popup document swap#41086

Merged
yury-s merged 3 commits into
microsoft:mainfrom
yury-s:fix-40895
Jun 3, 2026
Merged

fix(trace): capture attribute mutations after a popup document swap#41086
yury-s merged 3 commits into
microsoft:mainfrom
yury-s:fix-40895

Conversation

@yury-s

@yury-s yury-s commented Jun 1, 2026

Copy link
Copy Markdown
Member

Summary

  • A popup reuses its initial about:blank document; after it navigates, the snapshotter's MutationObserver stayed bound to the stale document and never reported attribute mutations (e.g. a hiding CSS class added via classList.add/setAttribute), so DOM snapshots showed stale attributes.
  • Re-bind the observer to the live document before each capture.

Fixes #40895

@yury-s yury-s requested a review from dgozman June 1, 2026 23:35
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

ensureCachedData(mutation.target).attributesCached = undefined;
}

private _ensureObservingCurrentDocument() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmm... We already have a mechanism that tries to achieve this in _refreshListenersWhenNeeded(). Is it wrong? Should we update both? Should we unify both?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch — they target different failure modes, so I unified them rather than leaving two parallel detectors.

_refreshListenersWhenNeeded() recovers the target-marking listeners across a documentElement replacement within the same document object — its childList observer stays valid because the document node is unchanged. It cannot handle a full document-object swap (a popup's initial about:blank being replaced by the navigated document while the window is reused), because both that childList observer and the main attribute observer were bound to the now-discarded document.

So I made the capture-time swap check the single entry point: the constructor now goes through _ensureObservingCurrentDocument() too, and on a swap it re-binds the attribute observer and re-runs _refreshListenersWhenNeeded(), which re-arms the documentElement watcher on the live document. The existing watcher couldn't self-recover from a swap since it relied on an observer bound to the swapped-away document.

yury-s added 2 commits June 2, 2026 12:05
A popup reuses its initial about:blank document, so the snapshotter's
MutationObserver could stay bound to the stale document after navigation
and never report attribute mutations (e.g. a hiding CSS class added via
classList.add/setAttribute) for the live one. Re-bind the observer to the
current document before each capture.

Fixes: microsoft#40895
Route both the constructor setup and the capture-time re-binding through
_ensureObservingCurrentDocument(), and on a document swap re-run
_refreshListenersWhenNeeded() so the documentElement watcher is re-armed on
the live document too. The existing watcher cannot recover on its own because
it relies on a MutationObserver bound to the swapped-away document.
@yury-s yury-s requested a review from dgozman June 2, 2026 19:24
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Test results for "MCP"

7230 passed, 1103 skipped


Merge workflow run.

@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

39523 passed, 775 skipped


Merge workflow run.

@yury-s yury-s merged commit 9fbc0a6 into microsoft:main Jun 3, 2026
46 checks passed
@yury-s yury-s deleted the fix-40895 branch June 3, 2026 17:59
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]: Trace viewer does not capture some dynamically added CSS classes

2 participants