-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add emitFromIframe option to support iframe in iframe #1726
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
base: master
Are you sure you want to change the base?
Conversation
|
512f809
to
bbb0aaf
Compare
bbb0aaf
to
3328fcf
Compare
@@ -89,6 +89,7 @@ function record<T = eventWithTime>( | |||
recordDOM = true, | |||
recordCanvas = false, | |||
recordCrossOriginIframes = false, | |||
emitFromIframe = false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since you could have more than one nested iframe that you want to record but will only ever have one parent, i wonder if it makes more sense to have a isParentIframe
or isRootIframe
instead
config then gets slightly simpler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't know if there are already tests on the iframe behaviour but since this would let us break the behaviour we should probably add some tests that when "i am a parent" config is set it emits and when "i am an iframe" config is set it passes the message up to the parent
I think this is a great idea! Thank you for contributing.
Yes I'd like to echo @pauldambra's comments about tests, they are really important in this situation. Also I'm not sure about the naming of these things at the moment, might have to think some more about this. Thanks for contributing though, I definitely think this should be part of rrweb |
Thank you for maintaining rrweb and supporting the modern web dev tooling ecosystem
Description
This PR introduces a new
emitFromIframe
boolean in rrweb'srecordOptions
that allows iframe-in-iframe monitoring when hosted in a parent that doesn't have rrweb.Problem
Consider a "host website" (which doesn't have rrweb) which hosts a "parent iframe" (which has rrweb) which nests another "child iframe" (which also has rrweb).
Currently, the
recordCrossOriginIframes
option would need to be enabled in rrweb in both parent and child iframes in order to monitor iframe-in-iframe traffic correctly. However we cannot monitor the parent iframe as therecordCrossOriginIframes
option is ALSO used to pass all events to the host website since the parent iframe is not the top level page. i.e: TherecordCrossOriginIframes
is used for both receiving events from a child iframe and passing those events without recording, to the host website.Thus the parent iframe will emit events to the host website since
recordCrossOriginIframes
is true and the host website will ignore these events as it doesn't have rrweb.Solution
Adding a
emitFromIframe
option allows the parent iframe to record events directly without passing events to the host website while usingrecordCrossOriginIframes
to collect events from child iframes.Status
This is a proof-of-concept but it should already work in production. I would like to get feedback on whether this is something that aligns with the rrweb team's roadmap before investing more effort in adding documentation and tests.