Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single style capture #1437

Merged
merged 44 commits into from
Aug 6, 2024
Merged

Conversation

eoghanmurray
Copy link
Contributor

@eoghanmurray eoghanmurray commented Apr 5, 2024

Prep PR for async <style> serialization via assets: refactor stringifyStylesheet to happen in a single place during initial snapshot.

  • initial motivation was to change the approach on stringifying <style> elements to do so in a single place
  • I then learned that we need the style content present on child text elements, in order to support mutation
  • this is still supported, however the css content is now preferably stored in parent <style> element in that _cssText attribute, rather than in the (natural) position in the child text elements of that element. This is to support the stylesheet-assets PR which will delay population of the _cssText attribute, as in both cases the snapshot will be recorded with empty child text elements
  • on rebuild, this content can now be 'spread' out to child text nodes (usually there's only one!) in order to give a correct target for any further text mutations on these children.
  • there is a new complex but comprehensive test 'can record and replay style text mutations' which covers all of this; it has been designed to ensure that it fails if css text is populated in the wrong way (e.g. putting the entire cssText on the first of many text children and leaving the others blank)

Copy link

changeset-bot bot commented Apr 5, 2024

🦋 Changeset detected

Latest commit: 58bfecc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 19 packages
Name Type
rrweb-snapshot Patch
rrweb Patch
rrdom Patch
rrdom-nodejs Patch
rrweb-player Patch
@rrweb/all Patch
@rrweb/replay Patch
@rrweb/record Patch
@rrweb/types Patch
@rrweb/packer Patch
@rrweb/utils Patch
@rrweb/web-extension Patch
rrvideo Patch
@rrweb/rrweb-plugin-console-record Patch
@rrweb/rrweb-plugin-console-replay Patch
@rrweb/rrweb-plugin-sequential-id-record Patch
@rrweb/rrweb-plugin-sequential-id-replay Patch
@rrweb/rrweb-plugin-canvas-webrtc-record Patch
@rrweb/rrweb-plugin-canvas-webrtc-replay Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@Juice10 Juice10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this Eoghan!

packages/rrweb-snapshot/src/rebuild.ts Show resolved Hide resolved
packages/rrweb-snapshot/src/snapshot.ts Outdated Show resolved Hide resolved
packages/rrweb/test/integration.test.ts Outdated Show resolved Hide resolved
packages/rrweb/test/integration.test.ts Outdated Show resolved Hide resolved
packages/rrweb-snapshot/src/rebuild.ts Show resolved Hide resolved
packages/rrweb-snapshot/src/utils.ts Outdated Show resolved Hide resolved
packages/rrweb-snapshot/src/types.ts Outdated Show resolved Hide resolved
@eoghanmurray eoghanmurray force-pushed the single-style-capture branch 3 times, most recently from bd99f6f to 5cc3ee3 Compare July 8, 2024 14:26
eoghanmurray added a commit to eoghanmurray/rrweb that referenced this pull request Jul 8, 2024
… replay them correctly' test into a dedicated 'backwards compatibility' version to ensure older recordings can still be replayed correctly even when the duplicate data generated is fixed in rrweb-io#1437
@eoghanmurray eoghanmurray force-pushed the single-style-capture branch 3 times, most recently from 4b1623c to 75808d8 Compare July 9, 2024 11:04
eoghanmurray and others added 18 commits August 2, 2024 15:23
…t getting rebuilt with `adaptCssForReplay`
…ext content, but I was mistaken; adding as I think it still has value in terms of regression
…te content in mutation between _cssText and a text node
…de after a mutation as it's textContent wasn't getting a serialized id
…ple child nodes

Add a new integration test to verify that style mutations with multiple child nodes are recorded and replayed correctly. The test sets the color of two div elements using CSS and verifies that the color is applied correctly after replaying the recorded events.
 - Don't record css content twice when a <style> element is added in a mutation
 - Fix case where we wouldn't have been able to mutate a <style> text node after a mutation as it's textContent wasn't getting a serialized id
…y style mutations' covers more ground.

Could probably emphasize that these 2 are related to `insertRule` i.e. programmatic mutations as opposed to text mutations
The prior 'dynamic stylesheet' route is now the main route for serializing a stylesheet; dynamic stylesheet were missed out in rrweb-io#1533 but are caught in this PR by the tests added in that PR as the stylesheet handling is simplified/centralised
…ns entire css text when there are too few child nodes' passes

Co-authored-by: Justin Halsall <[email protected]>
…sString` function (thanks Justin). Add test to show when this matters
@eoghanmurray eoghanmurray merged commit 5fbb904 into rrweb-io:master Aug 6, 2024
6 checks passed
@otan
Copy link

otan commented Sep 26, 2024

i don't have an exact page to reproduce this problem, but i find that /* rrweb-split */ is introduced in places which creates invalid CSS, e.g. this beast:

"_cssText": "@keyframes intercom-lightweight-app-launcher { \n  0% { opacity: 0; transform: scale(0.5); }\n  100% { opacity: 1; transform: scale(1); }\n}@keyframes intercom-lightweight-app-gradient { \n  0% { opacity: 0; }\n  100% { opacity: 1; }\n}@keyframes intercom-lightweight-app-messenger { \n  0% { opacity: 0; transform: scale(0); }\n  40% { opacity: 1; }\n  100% { transform: scale(1); }\n}.intercom-lightweight-app { position: fixed/* rr_split */; width: 0px; height: 0px; font-family: intercom-font, \"Helvetica Neue\", \"Apple Color Emoji\", Helvetica, Arial, sans-serif; }.intercom-lightweight-app-gradient { position: fixed; width: 500px; height: 500px; bottom: 0px; right: 0px; pointer-events: none; background: radial-gradient(at right bottom, rgba(29, 39, 54, 0.16) 0%, rgba(29, 39, 54, 0) 72%); animation: 200ms ease-out 0s 1 normal none running intercom-lightweight-app-gradient; }.intercom-lightweight-app-launcher { position: fixed; border: none; bottom: 20px; right: 20px; max-width: 48px; width: 48px; max-height: 48/* rr_split */px; height: 48px; border-radius: 50%; background: rgb(22, 38, 148); cursor: pointer; box-shadow: rgba(0, 0, 0, 0.06) 0px 1px 6px 0px, rgba(0, 0, 0, 0.16) 0px 2px 32px 0px; transition: transform 167ms cubic-bezier(0.33, 0, 0, 1); box-sizing: content-box; padding: 0px !important; margin: 0px !important; }.intercom-lightweight-app-launcher:hover { transition: transform 250ms cubic-bezier(0.33, 0, 0, 1); transform: scale(1.1); }.intercom-lightweight-app-launcher:active { transform: scale(0.85); transition: transform 134ms cubic-bezier(0.45, 0, 0.2, 1); }.intercom-lightweight-app-launcher:focus { outline: none; }.intercom-lightweight-app-launcher-icon { display: flex; align-items: center; justify-content: center; position: absolute; top: 0px; left: 0px; width: 48px; height: 48px; transition: transform 100ms linear, opacity 80ms linear; }.intercom-lightweight-app-launcher-icon-open { opacity: 1; transform: rotate(0deg) scale(1); }.intercom-lightweight-app-launcher-icon-open svg { width: 24px; height: 24px; }.intercom-lightweight-app-launcher-icon-open svg path { fill: rgb(255, 255, 255); }.intercom-lightweight-app-launcher-icon-self-serve { opacity: 1; transform: rotate(0deg) scale/* rr_split */(1); }.intercom-lightweight-app-launcher-icon-self-serve svg { height: 44px; }.intercom-lightweight-app-launcher-icon-self-serve svg path { fill: rgb(255, 255, 255); }.intercom-lightweight-app-launcher-custom-icon-open { max-height: 24px; max-width: 24px; opacity: 1; transform: rotate(0deg) scale(1); }.intercom-lightweight-app-launcher-icon-minimize { opacity: 0; transform: rotate(-60deg) scale(0); }.intercom-lightweight-app-launcher-icon-minimize svg path { fill: rgb(255, 255, 255); }.intercom-lightweight-app-messenger { position: fixed; overflow: hidden; background-color: white; animation: 250ms cubic-bezier(0, 1, 1, 1) 0s 1 normal none running intercom-lightweight-app-messenger; transform-origin: right bottom; width: 400px; height: calc(100% - 40px); max-height: 704px; min-height: 250px; right: 20px; bottom: 20px; box-shadow: rgba(0, 0, 0, 0.16) 0px 5px 40px; border-radius: 16px; }.intercom-lightweight-app-messenger-header { height: 64px; border-bottom: none; background: rgb(255, 255, 255); }.intercom-lightweight-app-messenger-footer { position: absolute; bottom: 0px; width: 100%; height: 80px; background: rgb(255, 255, 255); font-size: 14px; line-height: 21px; border-top: 1px solid rgba(0, 0, 0, 0.05); box-shadow: rgba(0, 0, 0, 0.05) 0px 0px 25px; }@media print {\n  .intercom-lightweight-app { display: none; }\n}"

now fails to replay despite being "valid" on the browser

@otan
Copy link

otan commented Sep 26, 2024

here's a repro - run this on any page with rrweb recording:

const s = document.createElement("style"); s.append(document.createTextNode("a {")); s.append(document.createTextNode("color: red; }"));
document.head.appendChild(s);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants