fix: inserted styles lost when moving elements#233
Merged
Conversation
fix code for nodejs tests change fix direction to avoid issues with duplicate styles format issues swap waitForTimeout for waitForRAF in test that flaked Add unit tests for new functions Fix broken test causes by file formatting removing spaced --------- Co-authored-by: jaj1014 <[email protected]> Co-authored-by: jaj1014 <[email protected]>
billyvg
changed the title
Member
Author
|
Applying rrweb-io#1357 to our fork |
chargome
approved these changes
Dec 18, 2024
| if (!elementCssRules || !elementCssRules.length) return; | ||
| // style sheet w/ innerText styles to diff with actual and get only inserted styles | ||
| const tempStyleSheet = new CSSStyleSheet(); | ||
| tempStyleSheet.replaceSync(styleElement.innerText); |
Member
There was a problem hiding this comment.
This could be an issue for some safari users: https://caniuse.com/mdn-api_cssstylesheet_replacesync but I'd leave it in.
Member
Author
There was a problem hiding this comment.
Good callout - I think this is OK for us since rrdom is only used on playback
billyvg
deleted the
feat-merge-upstream-2891c362a3933b81175d8ea250432ff84535176b
branch
billyvg
added a commit
to getsentry/sentry
that referenced
this pull request
Jan 7, 2025
We have an org that has a small handful of replays where the replayStepper causes massive perf issues to the extent that it freezes the browser. I narrowed it down to the `diff()` code inside of `rrdom` and a recent upstream PR (getsentry/rrweb#233) seems to have exacerbated the problem. I have not been able to figure out the root cause for the perf issues, but it seems to be related to CSS and the mutations that add `style` elements. We will want to try to identify what exactly in these replays are causing the perf issues. In the meantime we can filter out these mutations. Since we are only interested in generating and extracting the HTML for certain breadcrumb events, the styles should have no affect on the data we are interested in using. Closes #82221
billyvg
added a commit
to getsentry/sentry
that referenced
this pull request
Jan 7, 2025
…83016) We have an org that has a small handful of replays where the replayStepper causes massive perf issues to the extent that it freezes the browser. I narrowed it down to the `diff()` code inside of `rrdom` and a recent upstream PR (getsentry/rrweb#233) seems to have exacerbated the problem. I have not been able to figure out the root cause for the perf issues, but it seems to be related to CSS and the mutations that add `style` elements. We will want to try to identify what exactly in these replays are causing the perf issues. In the meantime we can filter out these mutations. Since we are only interested in generating and extracting the HTML for certain breadcrumb events, the styles should have no affect on the data we are interested in using. Closes #82221
billyvg
added a commit
that referenced
this pull request
Jan 10, 2025
fix code for nodejs tests change fix direction to avoid issues with duplicate styles format issues swap waitForTimeout for waitForRAF in test that flaked Add unit tests for new functions Fix broken test causes by file formatting removing spaced --------- Co-authored-by: jaj1014 <[email protected]> Co-authored-by: jaj1014 <[email protected]>
billyvg
added a commit
that referenced
this pull request
Jan 14, 2025
fix code for nodejs tests change fix direction to avoid issues with duplicate styles format issues swap waitForTimeout for waitForRAF in test that flaked Add unit tests for new functions Fix broken test causes by file formatting removing spaced --------- Co-authored-by: jaj1014 <[email protected]> Co-authored-by: jaj1014 <[email protected]>
billyvg
added a commit
that referenced
this pull request
Jan 14, 2025
fix code for nodejs tests change fix direction to avoid issues with duplicate styles format issues swap waitForTimeout for waitForRAF in test that flaked Add unit tests for new functions Fix broken test causes by file formatting removing spaced --------- Co-authored-by: jaj1014 <[email protected]> Co-authored-by: jaj1014 <[email protected]>
andrewshie-sentry
pushed a commit
to getsentry/sentry
that referenced
this pull request
Jan 22, 2025
…83016) We have an org that has a small handful of replays where the replayStepper causes massive perf issues to the extent that it freezes the browser. I narrowed it down to the `diff()` code inside of `rrdom` and a recent upstream PR (getsentry/rrweb#233) seems to have exacerbated the problem. I have not been able to figure out the root cause for the perf issues, but it seems to be related to CSS and the mutations that add `style` elements. We will want to try to identify what exactly in these replays are causing the perf issues. In the meantime we can filter out these mutations. Since we are only interested in generating and extracting the HTML for certain breadcrumb events, the styles should have no affect on the data we are interested in using. Closes #82221
billyvg
added a commit
that referenced
this pull request
Jan 23, 2025
fix code for nodejs tests change fix direction to avoid issues with duplicate styles format issues swap waitForTimeout for waitForRAF in test that flaked Add unit tests for new functions Fix broken test causes by file formatting removing spaced --------- Co-authored-by: jaj1014 <[email protected]> Co-authored-by: jaj1014 <[email protected]>
billyvg
pushed a commit
to getsentry/sentry-javascript
that referenced
this pull request
Feb 24, 2025
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
2 participants
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.
fix code for nodejs tests
change fix direction to avoid issues with duplicate styles
format issues
swap waitForTimeout for waitForRAF in test that flaked
Add unit tests for new functions
Fix broken test causes by file formatting removing spaced
Co-authored-by: jaj1014 [email protected]
Co-authored-by: jaj1014 [email protected]