fix: clone iframe nodes better

[DustinBrett]

My previous PR adding iframe cloning was incomplete.

Description

There were 2 specific reasons it didn't work:

• It doesn't work properly with instanceof across realms (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/instanceof#instanceof_and_multiple_realms)
• Child nodes for the iframe need to come from contentDocument.body.childNodes

To solve instanceof I've changed to comparing such as: Object.getPrototypeOf(node).constructor.name === instance.name where instance would be a prototype such as HTMLImageElement. This will also check recursively to see for example if something like HTMLDivElement also has Element in it's prototype chain and is therefore an instance of it.

Another solution suggested in MDN could be something like myNode instanceof myNode.ownerDocument.defaultView.SVGElement, but I had no luck (Illegal invocation) implementing this and I feel the method I have is a bit more ideal than just checking tagName's.

Motivation and Context

I want to fix this as I added iframe capture changes recently which caused this new issue to be visible.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Enhancement (changes that improvement of current feature or performance)
• Refactoring (changes that neither fixes a bug nor adds a feature)
• Test Case (changes that add missing tests or correct existing tests)
• Code style optimization (changes that do not affect the meaning of the code)
• Docs (changes that only update documentation)
• Chore (changes that don't modify src or test files)

Self Check before Merge

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[codecov]

Codecov Report

Base: 63.36% // Head: 63.24% // Decreases project coverage by -0.12% ⚠️

Coverage data is based on head (728755c) compared to base (d58db90).
Patch coverage: 65.00% of modified lines in pull request are covered.

❗ Current head 728755c differs from pull request most recent head 79d631d. Consider uploading reports for the commit 79d631d to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #352      +/-   ##
==========================================
- Coverage   63.36%   63.24%   -0.12%     
==========================================
  Files          10       10              
  Lines         565      555      -10     
  Branches      134      129       -5     
==========================================
- Hits          358      351       -7     
+ Misses        147      146       -1     
+ Partials       60       58       -2     

Impacted Files	Coverage Δ
src/util.ts	60.86% <ø> (ø)
src/clone-node.ts	75.92% <56.25%> (-0.44%)	⬇️
src/embed-images.ts	81.81% <100.00%> (+3.76%)	⬆️
src/embed-webfonts.ts	30.63% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[haodaking]

Can you take a look at the demo I wrote? The height of the iframe is missing, which is not as expected. thanks

https://codesandbox.io/s/sweet-knuth-3n57ws?file=/index.html

[DustinBrett]

Can you take a look at the demo I wrote? The height of the iframe is missing, which is not as expected. thanks

https://codesandbox.io/s/sweet-knuth-3n57ws?file=/index.html

I'll take a look as I didn't have this specific issue.

[DustinBrett]

Can you take a look at the demo I wrote? The height of the iframe is missing, which is not as expected. thanks

https://codesandbox.io/s/sweet-knuth-3n57ws?file=/index.html

To me this doesn't seem so bad but maybe I am missing something. It did clone the iframe and the image inside, but didn't respect the height of the iframe you are saying? I'm not sure I have a solution for this atm and also I am wondering if this issue is more related to my previous merged PR where iframe cloning began. Could you clarify the expected results and suggestions if you have any? I'm not sure if this PR needs to solve this specific issue unless the code changes caused it.

[haodaking]

To me this doesn't seem so bad but maybe I am missing something. It did clone the iframe and the image inside, but didn't respect the height of the iframe you are saying? I'm not sure I have a solution for this atm and also I am wondering if this issue is more related to my previous merged PR where iframe cloning began. Could you clarify the expected results and suggestions if you have any? I'm not sure if this PR needs to solve this specific issue unless the code changes caused it.

The solution for #351 PR is to replace the iframe tag with a div tag, set the div as the style of the iframe, and then clone the content in the iframe body

[DustinBrett]

To me this doesn't seem so bad but maybe I am missing something. It did clone the iframe and the image inside, but didn't respect the height of the iframe you are saying? I'm not sure I have a solution for this atm and also I am wondering if this issue is more related to my previous merged PR where iframe cloning began. Could you clarify the expected results and suggestions if you have any? I'm not sure if this PR needs to solve this specific issue unless the code changes caused it.

The solution for #351 PR is to replace the iframe tag with a div tag, set the div as the style of the iframe, and then clone the content in the iframe body

Ok thanks for clarification. It would be good if the iframe could be cloned without it being a div, unless that is the ideal solution. I'll look into it more.

[DustinBrett]

To me this doesn't seem so bad but maybe I am missing something. It did clone the iframe and the image inside, but didn't respect the height of the iframe you are saying? I'm not sure I have a solution for this atm and also I am wondering if this issue is more related to my previous merged PR where iframe cloning began. Could you clarify the expected results and suggestions if you have any? I'm not sure if this PR needs to solve this specific issue unless the code changes caused it.

The solution for #351 PR is to replace the iframe tag with a div tag, set the div as the style of the iframe, and then clone the content in the iframe body

I've looked into this and it turns out that although #351 had a solution, it was not because of the div but because of changing the iframe (inline frame) to display as block. After looking at https://stackoverflow.com/a/28954135/5895982 I think it might make sense for this use case. I've applied that change in the latest commit and tested on your codesandbox and it seemed to give the correct result now.

[vivcat]

🎉 This PR is included in version 1.11.6 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀