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

Separate out window monkeypatching into an optional rrweb-init.js #1015

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eoghanmurray
Copy link
Contributor

rrweb-init.js file is intended to be used much earlier in the page loading lifecycle (e.g. early injection from a browser extension) and which communicates with the main rrweb code via window.postMessage

…e which is intended to be used much earlier in the page loading lifecycle (e.g. early injection from a browser extension) and which communicates with the main rrweb code via window.postMessage
@eoghanmurray eoghanmurray force-pushed the recordInit-earlyPatch branch from 869ad34 to 3edeb06 Compare October 7, 2022 14:21
@Juice10
Copy link
Contributor

Juice10 commented Nov 16, 2022

When trying to record the following codepen: https://codepen.io/Showvhick2/pen/ddzpKW I noticed that the following code is also invoked much too late and might make sense to be added to this rrweb-init.js:

if (this.recordCrossOriginIframes) {
window.addEventListener('message', this.handleMessage.bind(this));
}

@eoghanmurray
Copy link
Contributor Author

  • Where this patch might need more work is on the inter-window communication as the new init.js is designed to be included via the run_at: "document_start" hook, with the rest of rrweb using the normal content_script hook — scripts introduced by these different hooks live in different isolated worlds and can't reference each others' context outside of postMessage.
    I had to add a means of referencing DOM nodes, and I came up with the ad-hoc method using css references: canvas::nth-of-type(1) , which could possibly fail if canvas elements were reordered

  • Other todo is to pull more early init stuff (like console.log, and what Justin mentioned) but it all needs testing and there is a question of whether the normal version of rrweb should also rely on these new inter-window communication methods rather than passing callback functions — that is an open question and potentially more work, but would mean that there would be less surface area for testing.

@Juice10
Copy link
Contributor

Juice10 commented Jan 4, 2023

  • Where this patch might need more work is on the inter-window communication as the new init.js is designed to be included via the run_at: "document_start" hook, with the rest of rrweb using the normal content_script hook — scripts introduced by these different hooks live in different isolated worlds and can't reference each others' context outside of postMessage.

Could we make the communication layer configurable? That way this could also be useful for people including rrweb on their webpage and trying to include a tiny package early on in the loading process before the rest arrives

@eoghanmurray
Copy link
Contributor Author

I need to look at the work you did on cross-domain nested iframes — how postMessage is used

@eoghanmurray eoghanmurray self-assigned this Apr 5, 2024
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.

2 participants