Use window.top and window.top.document

[eoghanmurray]

This pull request is to break out of simple same-domain <iframe>s, i.e. when rrweb.js is loaded from within the iframe.

This is the case for GoDaddy.

I'm not sure whether this is generally a good idea for all of rrweb's users, so maybe it needs further consideration before being accepted?

[Juice10]

Great work eoghanmurray!
I’m wondering if this could potentially work cross-domain, for example in electron if rrweb is injected in all the iframes.

[eoghanmurray]

@Juice10 I hadn't envisioned that usecase and haven't tested that.

I just came across this since reading your comment:
rrweb-io/rrweb-snapshot#10
Maybe that is unrelated?

Do you have a chance to test out what would happen with multiple iframes in the same document?

[Juice10]

I just came across this since reading your comment:
rrweb-io/rrweb-snapshot#10
Maybe that is unrelated?

@eoghanmurray I'm not sure to be honest.

Do you have a chance to test out what would happen with multiple iframes in the same document?

Yes I'd be happy to, anything specific you'd like me to do? Just do a recording with this branch? Any specific pages you'd like me to try out?

One thing that might make electron based recording a little trickier is that every iframe/browser-window has its own rrweb.record({ emit(event) { /* send to central location */ } }); so electron receives events from multiple pages at the same time without knowing which one is which.

[eoghanmurray]

The electron (https://www.electronjs.org/ ?) application of rrweb is definitely not on my radar, I was only thinking of the web; so I guess a run through it's paces would be great to make sure this change doesn't break things there.

[eoghanmurray]

Should this be a configuration option for it to be accepted?

[eoghanmurray]

Does #481 change it so that it starts with the top level document, or is it just in whatever document that rrweb is initiated?

rrweb/src/record/index.ts

Lines 439 to 442 in 1dd91f0

	const init = () => {
	takeFullSnapshot();
	handlers.push(observe(document));
	};

I think the latter is the case so maybe this pull request is still relevant?

[eoghanmurray]

I've done a full rework as this PR was originally authored before we had nested iframe support (#481)

This is also based on #716, although that is still in progress, as they are related.

Now with a working test!

[samber]

Any update regarding this PR?

How can we move things forward?

I wonder if it would be better to add window as a property of recordOptions? 🤔 It would let the developer pick the right window: window, window.parent, window.top or child iframe.

[eoghanmurray]

I wonder if it would be better to add window as a property of recordOptions? thinking It would let the developer pick the right window: window, window.parent, window.top or child iframe.

I think both will be good, but I think it should default to the highest window it can find