bubkoo / html-to-image

✂️ Generates an image from a DOM node using HTML5 canvas and SVG.
MIT License
5.69k stars 531 forks source link

fix: iframe content displayed incorrectly #351

Closed haodaking closed 1 year ago

haodaking commented 1 year ago

This pull request addresses the issue with capturing iframes in the library, where the resulting images were blank or incorrect.

Description

https://stackoverflow.com/questions/26248599/instanceof-htmlelement-in-iframe-is-not-element-or-object

Motivation and Context

Types of changes

Self Check before Merge

vivcat[bot] commented 1 year ago

👋 @haodaking

💖 Thanks for opening this pull request! 💖

Please follow the contributing guidelines. And we use semantic commit messages to streamline the release process.

Examples of commit messages with semantic prefixes:

Things that will help get your PR across the finish line:

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

codecov[bot] commented 1 year ago

Codecov Report

Base: 63.24% // Head: 63.24% // No change to project coverage :thumbsup:

Coverage data is based on head (130f2f9) compared to base (7a83b54). Patch coverage: 60.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #351 +/- ## ======================================= Coverage 63.24% 63.24% ======================================= Files 10 10 Lines 555 555 Branches 129 129 ======================================= Hits 351 351 Misses 146 146 Partials 58 58 ``` | [Impacted Files](https://codecov.io/gh/bubkoo/html-to-image/pull/351?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=%E5%B4%96) | Coverage Δ | | |---|---|---| | [src/util.ts](https://codecov.io/gh/bubkoo/html-to-image/pull/351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=%E5%B4%96#diff-c3JjL3V0aWwudHM=) | `60.86% <ø> (ø)` | | | [src/clone-node.ts](https://codecov.io/gh/bubkoo/html-to-image/pull/351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=%E5%B4%96#diff-c3JjL2Nsb25lLW5vZGUudHM=) | `75.92% <50.00%> (ø)` | | | [src/embed-images.ts](https://codecov.io/gh/bubkoo/html-to-image/pull/351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=%E5%B4%96#diff-c3JjL2VtYmVkLWltYWdlcy50cw==) | `81.81% <83.33%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=%E5%B4%96). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=%E5%B4%96)

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

DustinBrett commented 1 year ago

Thanks for this PR and finding the core issues. I've decided to make my own PR (https://github.com/bubkoo/html-to-image/pull/352) as I feel in some ways I've introduced this issue. I am not sure on the etiquette of conflicting PR's in open source. Sorry if I stepped on toes but I prefer to create my own PR in this case. I felt the tagName approach although effective could maybe be adjusted to use something more generic, and I also didn't see a need for a few other changes in the PR. So I made mine to focus on what I considered required to complete my original PR where I claimed to add iframes as a whole.

haodaking commented 1 year ago

Thanks for this PR and finding the core issues. I've decided to make my own PR (#352) as I feel in some ways I've introduced this issue. I am not sure on the etiquette of conflicting PR's in open source. Sorry if I stepped on toes but I prefer to create my own PR in this case. I felt the tagName approach although effective could maybe be adjusted to use something more generic, and I also didn't see a need for a few other changes in the PR. So I made mine to focus on what I considered required to complete my original PR where I claimed to add iframes as a whole.

Thank you, I think your code implementation is more elegant and learning.