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

fix: #841 can't record SVG element inside iframe properly #843

Merged
merged 1 commit into from
Feb 24, 2022
Merged

Conversation

YunFeng0817
Copy link
Member

@YunFeng0817 YunFeng0817 commented Feb 23, 2022

This PR is trying to fix issue #841.

SVGElement inside iframe is different from SVGElement outside. So even though an el inside iframe is actually an SVG element, el instanceof SVGElement is still false and the isSVG property is missing in the recorded data.

ownerSVGElement even has better compatibility than mutation observer.
https://caniuse.com/?search=ownerSVGElement
https://caniuse.com/?search=mutationObserver

el instanceof SVGElement is false if the svg element is inside iframe so that the isSVG property is missing in the recorded data
@YunFeng0817 YunFeng0817 changed the title fix: #841 can't record SVG element inside iframe properly fix: #841 can't record SVG element inside iframe properly Feb 23, 2022
@Yuyz0112
Copy link
Member

LGTM, does the diff in the snapshot expected?

@YunFeng0817
Copy link
Member Author

YunFeng0817 commented Feb 23, 2022

LGTM, does the diff in the snapshot expected?

yes, I checked them carefully. The snapshot in the old version is missing the 'isSVG' property

@YunFeng0817
Copy link
Member Author

Oh sorry, I forgot to say that the snapshot is changed because I add the integration test for this situation.

@Yuyz0112 Yuyz0112 merged commit e9531d4 into master Feb 24, 2022
JohnPhamous pushed a commit to highlight/rrweb that referenced this pull request Feb 24, 2022
@YunFeng0817 YunFeng0817 deleted the svg-fix branch March 3, 2022 16:24
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.

None yet

2 participants