[Bug]: Regression in rrweb style capture for material ui

[arijit91]

Preflight Checklist

• I have searched the issue tracker for a bug report that matches the one I want to file, without success.

What package is this bug report for?

rrweb

Expected Behavior

rrweb is not capturing styling changes coming from material-ui

Sample app (button color is expected to change from red to blue on click):
https://ckn2h.csb.app/#

This was previously reported and fixed in
#650

It is broken on main

I did a binary search and looks like this was the commit that broke it:
a01c97f

I think the commit is causing duplicate style rules to be created on the DOM node

Actual Behavior

Sample app (button color is expected to change from red to blue on click):
https://ckn2h.csb.app/#

Testcase Gist URL

No response

Additional Information

No response

[Yuyz0112]

Do you think 9ed6767 fixed this?

[arijit91]

@Yuyz0112 I thought so as well actually, I tried that patch but it didn't seem to work :(

The bug exists on main

[Yuyz0112]

ok, going to debug it

[arijit91]

From what I could make out, there is a css rule we pick up on with the css style declaration observer, we add it to the node, and then we try to append a node with the same text content from a DOM mutation, so we have 2 (duplicate) styles on the same node

Then while replaying the DOM mutation, only one of the css rules is updated..

[Yuyz0112]

you are right, the issue was introduced in a01c97f. And I'm sorry I did not add enough information about what it's trying to fix.

Now let us add comprehensive test cases for the stylesheet recording to ensure we handle things correctly without introducing further regression.

• remote link element, use href to load a stylesheet
• style element, apply stylesheet as text content inside it
  • existed when snapshot starting
  • dynamic created and recorded by observers
• style element, apply stylesheet by using CSS APIs, without text content inside it
  • existed when snapshot starting
  • dynamic created and recorded by observers

I think some duplication of case 2 and case 3 leads these issues, 9ed6767 fixed part of it, but not all.

[Yuyz0112]

@Juice10 are there any cases I'm missing here?

I think maybe we can only use CSS APIs' observers to record style, without serializing style elements' text content.

[Juice10]

@Yuyz0112 I have seen some combinations of the the last two cases you mentioned.

Also some style element can have multiple text nodes added inside them (via JS), which can lead to some unexpected behavior.

And another thing to keep in mind are some cssRules (like media queries) can have more cssRules inside them. It's possible that serializing isn't accounting for this and those rules aren't correctly captured.

[rcmarron]

@Yuyz0112 We've been seeing this issue for websites using MaterialUI, and I think it's also impacting websites with ionic components too.

Any thoughts on what a fix might look like? I imagine this is pretty wide spread

And another thing to keep in mind are some cssRules (like media queries) can have more cssRules inside them. It's possible that serializing isn't accounting for this and those rules aren't correctly captured.

I can confirm that we're hitting this specific case with MaterialUI. We get a crash in the player when it attempts to add cssRules to a media query. If it's helpful, I can try to pull together an example.

[ChetanGoti]

@Yuyz0112 would #989 have fixed this issue as well? We are hitting this as we are using MaterialUI web components.