rrweb icon indicating copy to clipboard operation
rrweb copied to clipboard

isBlocked factors in the selector

Open dbseel opened this issue 2 years ago • 2 comments

Support a list of blocked classes in a CSS selector (blockSelector) in addition to the blockClass. Also ensure only node types are passed into contains. Also support providing a list of CSS properties to ignore on set/remove stylesheet mutations

dbseel avatar May 11 '22 16:05 dbseel

@Yuyz0112 Hey man, we've added a bunch of changes to Rrweb as we are using it on thousands of websites and noticed some flaws. Let us know if certain things are missing for this to get merged in.

IMFIL avatar May 30 '22 16:05 IMFIL

Thanks for the PR @dbseel! When refactoring and speeding up isBlocked for https://github.com/rrweb-io/rrweb/pull/903 I also noticed this was missing from rrweb. Happy you've created a PR for it. Because https://github.com/rrweb-io/rrweb/pull/903 has changed the guts of isBlocked drastically I expect it to create some merge issues for this branch once it gets merged in.

Juice10 avatar May 30 '22 22:05 Juice10

Hey @dbseel, any chance you could fix the merge conflicts?

Juice10 avatar Aug 19 '22 10:08 Juice10

@Juice10 Done. Can we get this merged in? It has been sitting here 3 months, don't want to have to rebase again.

dbseel avatar Aug 19 '22 16:08 dbseel

Thanks for this @dbseel! Really appreciate your contribution. Apologies that it took so long for us to review it

No problem @Juice10! Are you able to merge? I don't have permissions.

dbseel avatar Aug 29 '22 19:08 dbseel

I don't but I've messaged the other members of the core team, so it should get merged soon

Juice10 avatar Aug 29 '22 21:08 Juice10

Late to the party here!

@dbseel what are the use cases for ignoreCSSAttributes? I see color in the test case but curious as to the others. Is the motivation for this privacy, or performance (like slimDOM options).

Also wondering whether ignoreCSSProperties would have been a better name?

eoghanmurray avatar Feb 28 '23 16:02 eoghanmurray

@dbseel what are the use cases for ignoreCSSAttributes?

We have encountered some websites where certain attributes were constantly being added/removed/updated on the DOM, but they did not affect the replay. So there was a huge performance improvement by ignoring these attributes.

dbseel avatar Feb 28 '23 16:02 dbseel

These are CSS rule attributes rather than DOM attributes though? I guess just thinking it must be very unusual that rules would be continually added/removed. Was it a standard part of some framework, or just some once off craziness on a particular website?

eoghanmurray avatar Mar 01 '23 16:03 eoghanmurray

These are CSS rule attributes rather than DOM attributes though? I guess just thinking it must be very unusual that rules would be continually added/removed. Was it a standard part of some framework, or just some once off craziness on a particular website?

In our case a website was using quantum metrics. This plugin was adding/removing attributes thousands of times a second. Attributes like this: --quantum-metric-background-image

dbseel avatar Mar 01 '23 18:03 dbseel

I've just come across this again today; I notice that ignoreCSSAttributes doesn't omit them from capture from the initial snapshot. This wouldn't be significant for your usecase of mutations to --quantum-metric-background-image, but for completeness does anyone think it needs to be added?

eoghanmurray avatar Apr 30 '24 13:04 eoghanmurray