Uh oh!
There was an error while loading. Please reload this page.
fix(replay): use live click attributes in breadcrumbs - #20262
Merged
Conversation
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Cloudflare
Core
Deps
Other
Bug Fixes 🐛Deno
Other
Internal Changes 🔧Ci
Deps
Other
🤖 This preview updates automatically when you update the PR. |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Replay click breadcrumbs so recorded node attributes prefer the current/live DOM element attributes (to avoid stale rrweb mirror metadata), while still using rrweb metadata for the rest of the breadcrumb node payload. Adds a regression unit test covering attribute mutation/staleness.
Changes:
- Update DOM breadcrumb construction to source
attributesfrom the liveElementwhen available. - Keep rrweb snapshot metadata as the source for
tagName/textContent(when present). - Add a unit test ensuring live attributes win over stale mirror metadata.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/replay-internal/src/coreHandlers/handleDom.ts | Prefer live element attributes for breadcrumb node attributes; add helper to serialize Element.attributes. |
| packages/replay-internal/test/unit/coreHandlers/handleDom.test.ts | Add regression test asserting live attributes override stale rrweb meta; restore mocks after each test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Contributor
size-limit report 📦
|
Uh oh!
There was an error while loading. Please reload this page.
billyvg
reviewed
Apr 14, 2026
billyvg
left a comment
Member
There was a problem hiding this comment.
This makes sense to me, my only question is should we be mutating the event rather than returning a new event?
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-Authored-By: GPT-5 <noreply@anthropic.com>
Co-Authored-By: GPT-5 <noreply@anthropic.com>
Co-Authored-By: GPT-5 <noreply@anthropic.com>
Co-Authored-By: GPT-5 <noreply@anthropic.com>
Co-Authored-By: GPT-5 <noreply@anthropic.com>
logaretmforce-pushed
the
awad/js-2135-replay-click-live-attributes
branch
from
April 14, 2026 14:48
f435b1a to
e5bbdb0Comparelogaretm
commented
Apr 14, 2026
MemberAuthor
@billyvg we are mutating the mirrored state using the event as an input, not the event itself. Do you have another approach in mind? |
billyvg
commented
Apr 14, 2026
Member
@logaretm oh sorry misunderstood — do you know why mirror becomes stale? |
logaretm
commented
Apr 14, 2026
MemberAuthor
@billyvg AFAIK (take it with lots of salt), replay stores an initial snapshot and then a stream of mutation events. Eventually it won't be stale. But because breadcrumbs read those attributes from the snapshot they will be always outdated since it doesn't fold the streamed mutation events to the values it records. So this approach does this folding on the emit event, I tried initially finding the real element and reading from it but we will then have to mask every single attribute again since they will be unmasked, which felt like more work than folding the mutations in a flat loop. |
billyvg
approved these changes
Apr 14, 2026
Uh oh!
There was an error while loading. Please reload this page.
mydea pushed a commit
that referenced
this pull request
Apr 15, 2026
Fixes replay element attributes grabbing a potentially stale version of the attributes, we basically now prefer the live element if available, otherwise we keep the old behavior. closes#20238 --------- Co-authored-by: GPT-5 <noreply@anthropic.com>
46 tasks
3 tasks
Closed
1 task
logaretm added a commit
that referenced
this pull request
Aug 25, 2026
…attributes (#23426) Stops the replay mirror sync from rewriting recording events that were already emitted. rrweb hands us the same serialized node object it put in an earlier `adds` payload, and `EventBufferArray` holds events unserialized until flush, so patching it in place edited recorded history. Worst case was `style`, where rrweb sends a partial diff object rather than the full string, which is why Radix popovers lost `position: fixed` and rendered off screen. Regressed in #20262. Second commit is the same bug in `getAttributesToRecord`, which predates it. closes#23393
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 freeto 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.
Fixes replay element attributes grabbing a potentially stale version of the attributes, we basically now prefer the live element if available, otherwise we keep the old behavior.
closes#20238