Conversation
7e7f5a6 to
23addd0
Compare
Juice10
reviewed
Sep 16, 2022
Juice10
reviewed
Sep 16, 2022
| texts: [], | ||
| attributes: [], | ||
| }); | ||
| if ('_cssText' in (childSn as elementNode).attributes) |
Member
There was a problem hiding this comment.
Is _cssText always going to be set, also if a link element hasn't finished loading yet?
Member
Author
There was a problem hiding this comment.
According the code below
rrweb/packages/rrweb-snapshot/src/snapshot.ts
Lines 653 to 659 in 08ab7ab
I think it will set _cssText when the link has been loaded. Otherwise, recorder will only keep its original attributes.
Juice10
requested changes
Sep 16, 2022
Member
Juice10
left a comment
There was a problem hiding this comment.
Thanks for finding and fixing this bug! I found a one bug and maybe another potential bug. Could you fix those and I'd be happy to re-review & approve!
8e74045 to
6bde25a
Compare
Member
Author
6bde25a to
d311687
Compare
1. make Mirror's replace function act the same with the original one when there's no existed node to get replaced. 2. when replacing with the link/style elements, keep their existing attributes to prevent potential bugs
d311687 to
8aefe2b
Compare
Member
Author
|
I rebased the latest master branch and this pull request is ready to get reviewed and merged. |
Yuyz0112
approved these changes
Sep 30, 2022
This was referenced Jul 6, 2023
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
Apr 22, 2024
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
Apr 23, 2024
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
Apr 24, 2024
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
May 14, 2024
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
May 22, 2024
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
May 22, 2024
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
May 22, 2024
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
May 23, 2024
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 free
to 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.


In PR #909, an async mechanism for inlining link elements is introduced but it still has a bug and some negative impacts on the replayer side.
The Bug
In the stylesheet-manager.ts, when the link element loaded the styles, the manager emits an incremental mutation event to inline styles. But the mutation's parentId is wrong.
rrweb/packages/rrweb/src/record/stylesheet-manager.ts
Line 34 in abc035f
As a result, the event looks like this:
The parentNode of the link element is itself and it can't be appended to the dom actually.
Negative Impacts
This mechanism makes elements absent in the normal snapshot stage and causes performance issues and many warnings in the console.
Here's a diagram to explain this problem.
Changes
I built this PR based on #989 so that it contains all changes of that PR. This PR has to get merged after #989.
I made some changes to the current mechanism. In the snapshot stage, the unloaded Link elements will still get serialized without inline styles. After they are loaded. an incremental mutation of attributes is appended which includes an attribute
_cssText. Its value is the inlined CSS styles.This way, the unloaded Link elements can be inlined without causing the above-mentioned side effects.
On the replaying side, when the replayer encounters the '_cssText' attribute change and its target is the Link element, the replayer can replace the old un-inlined one with the inlined loaded Link element.
This method can make it easy to implement the TODO feature as well.
rrweb/packages/rrweb/src/record/stylesheet-manager.ts
Lines 19 to 24 in abc035f
This PR can also fix #981.