astefanutti / decktape

PDF exporter for HTML presentations
MIT License
2.16k stars 175 forks source link

fix(fonts): wait for fonts to be fully loaded #337

Open nicojs opened 2 months ago

nicojs commented 2 months ago

Fixes #336

nicojs commented 2 months ago

This seems to work, but it doesn't solve my original problem.

There is an issue with slide 18: http://localhost:3000/input/revealjs-demo/#/18 (after running node test/run-server.js 3000).

image

In the demo pdf, you can see that a different font is used. I'm pretty sure this is a race condition. The page is loading the font in an iframe and the screenshot is taken before those fonts are loaded.

image

I can see 2 solutions:

@astefanutti what do you think?

astefanutti commented 2 months ago

I'd lean towards trying to support iframes if that's not too convoluted. Otherwise I agree we can fallback to updating the test as you suggested.

nicojs commented 2 months ago

I did a quick test, but it won't work. When loading cross-origin iframes, it seems impossible to know when the frame is loaded. The iframe.contentDocument.readyState is always 'complete', no matter what the actual readyState is. This is for security concerns.

I can add this code for the same-origin iframes. It might actually be useful. What do you think?

Detect frame loaded code ```js /** @param {import('@playwright/test').Page} page */ async function loaded(page) { return page.evaluate( async () => { const frames = document.querySelectorAll("iframe"); const promises = []; for (const frame of frames) { if (frame.checkVisibility()) { console.log("visible iframe detected!"); const doc = frame.contentDocument; if (!doc) { console.log("no contentDocument!"); continue; } promises.push( new Promise((res) => { if (doc.readyState === "complete") { console.log("was already complete!"); res(); } else { function eventListener() { frame.removeEventListener("load", eventListener); console.log("not yet complete, but now is!"); res(); } console.log("waiting for load!", frame.readyState); frame.addEventListener("load", eventListener); } }) ); } } await Promise.all(promises); } ); } ```
nicojs commented 2 months ago

This seems to work, but it doesn't solve my original problem.

There is an issue with slide 18:

No matter what I do, the screenshot always comes out this way. I think it has to do with the viewport being too small, so unrelated to fonts or iframes loading. What do you want to do with all this waiting code? We can simply abandon all this or add it... up to you 🤷‍♂️

astefanutti commented 2 months ago

What do you want to do with all this waiting code? We can simply abandon all this or add it... up to you 🤷‍♂️

For the iframes part, I'd be inclined to keep things simple and avoid introducing complex logic that we don't know if it fixes issues, or provide any real benefits.

For waiting for the fonts to be loaded, I think that's redundant with the pause at each next slide. Ideally that pause should be removed, and the logic only rely on such events, but I'm not sure what are the set of events it takes to be sure the slide is ready for export.