Compact style mutation

[eoghanmurray]

I had one recording in the wild that turned into a monster at 12GB.
One of the sources was continuous DOM Mutations caused by a 3rd party 'snow animation' library that was animating individual snowflakes multiple times per second via JavaScript, causing a constant stream of very large mutations.

Each mutation was of the format
"style":"color: rgb(255, 255, 255); position: absolute; width: 8px; height: 8px; font-family: arial, verdana; overflow: hidden; font-weight: normal; z-index: 0; display: block; bottom: auto; opacity: 1; padding: 0px; margin: 0px; font-size: 10px; line-height: 10px; text-align: center; vertical-align: baseline; left: 242.807px; top: 85.7332px;"

Even though just the left or top style attributes had changed.
The new format for this part of the attribute mutation looks like:
"style": {"left": "242.807px", "top": "85.7332px"}

There's a threshold to the attribute length before this change applies, currently set at a style attribute length of 25 characters (this value could be tweaked) (the threshold would have caused errors, so I've removed it)

Diffs are generated by creating a dummy node to house the old style and then comparing each style (sub)attribute in turn using getPropertyValue and getPropertyPriority. Attributes on the old value are also iterated over to see if any of them have been removed.

At playback time, these sub-style mutations are applied via setProperty with either a single value argument, or 2 arguments in the case that the !important priority flag is present in the change.

[eoghanmurray]

(Sorry still iterating on this as tests are not passing; my dev test environment isn't working so uploading changes here to check)

[eoghanmurray]

@Yuyz0112 would you be able to take a look at the failing build e.g. https://travis-ci.com/github/rrweb-io/rrweb/builds/213675796

i.e. there should be a text attribute change as follows:
"style": ""left: 8px; width: 46.2109px; top: 115.5px; bottom: auto; display: block;""

[Edit] #468 has the fix for why the style changes weren't showing up [End Edit]

[eoghanmurray]

Okay, I've got all the tests sorted now and have added one extra test for !important as this is treated separately.

Aside: during replay, it won't break if a previous string value for the style attribute is present from a prior recording.

[Juice10]

For code hygiene it's probably better to camelCase these variable names.

Suggested change

	const new_value = target.style.getPropertyValue(pname);
	const new_priority = target.style.getPropertyPriority(pname);
	const newValue = target.style.getPropertyValue(pname);
	const newPriority = target.style.getPropertyPriority(pname);

[eoghanmurray]

Thanks; I take it I cannot just commit this suggestion as-is?
I'll make a new commit.

[eoghanmurray]

Updated the original commit with your suggested change as was force-pushing anyhow.

[Juice10]

Looks good to me, only thing I can think of would be doing some checking in the replay, if the element has a style attribute. Just in case something went wrong with the recorded ids and we are accidentally referencing a text node. I believe there is a try/catch around the get/setAttributes for that reason on the string type of style mutation.

[Yuyz0112]

@eoghanmurray Looks like this approach will not trigger reflow/repaint during diff, because the dummy node is not in the DOM tree, am I right?

And is it worth a little benchmark test to show the time of diff?

[eoghanmurray]

Ya not added to the DOM; I felt safe creating the dummy node as we are doing that elsewhere in the code e.g. at https://github.com/rrweb-io/rrweb-snapshot/blob/master/src/snapshot.ts#L204

I made a quick performance.now() comparison and got the following results

Firefox (over 277 events):
Old avg. 0.184ms
New avg 0.234ms
Slower 1.27x

Chrome (over 256 events):
Old avg 0.073ms
New avg 0.224ms
Slower 3.03x

Looks like the difference is due to the existing transformAttribute code path being just as slow in Firefox.

This is happening in the DOMMutation callback which is 'off the main loop', right?

[Yuyz0112]

LGTM

@eoghanmurray This happens off the main loop, but still executing in sync mode, so it may block following execution. I think we can:

1. support running record logic in web worker
2. do record logic in request idle callback

[eoghanmurray]

for 1. I thought web workers didn't have access to the DOM?
2. looks good where browser supports it

[razor-x]

Hello. I'm coming here as a PostHog user. It uses this lib to power session recording. My app is able to run a strict CSP, specifically without 'unsafe-inline' in the policy. However, I noticed a lot of CSP errors generated like the one below:

After some digging, I believe this change broke support for users who run a strict CSP. Setting the style attribute on the dummy node will be blocked without 'unsafe-inline'.

Would you prefer I start a new issue or continue discussion here on possible solutions? Best.

[Juice10]

@razor-x thanks for bringing that issue up!
It probably makes most sense to start a new issue for that.