Finalize vanilla JS conversion and event-based refactor#259
Finalize vanilla JS conversion and event-based refactor#259google-labs-jules[bot] wants to merge 8 commits intojs-updatesfrom
Conversation
This commit finalizes the conversion of the `printThis` library to a dependency-free, vanilla JavaScript module. It also completes the refactoring of the core logic to be fully event-driven, replacing all `setTimeout` calls and fixing a critical race condition with the `iframe.onload` event.
Key changes include:
- Fixed a race condition where the `iframe.onload` event could fire before its handler was attached. The handler is now attached *before* the iframe is appended to the document.
- Replaced the deprecated `execCommand('print')` with a modern approach that uses `iframe.contentWindow.focus()` followed by `iframe.contentWindow.print()`.
- Implemented a `Promise.all` to wait for all images and stylesheets to load before printing, making the process more resilient to network conditions.
- Used the `afterprint` event for reliable cleanup of the iframe.
- Replaced the deprecated `mediaQueryList.addListener()` with `mediaQueryList.addEventListener()`.
- Ensured that the asset loading mechanism is resilient to errors, preventing a single failed asset from blocking the print process.
- Removed the `doctypeString` option to prevent a bug where the iframe content would not render.
This commit builds upon the previous work of converting the library to vanilla JavaScript, fixing the Android/Chrome printing bug, and addressing feedback from code reviews.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. New to Jules? Learn more at jules.google/docs. |
|
@jasonday Looks really good at first glance. Especially like the asset loading handler. |
Vanilla JS refactor
|
index.vanilla.html |
This commit addresses a regression where the iframe content was not being rendered. This was caused by two issues: 1. The use of `document.open/write/close` to set the doctype was clearing the iframe document after it had loaded. The `doctypeString` option has been removed to resolve this, as there is no reliable, modern alternative. 2. A race condition existed where the iframe's `load` event could fire before the `onload` handler was attached. This has been fixed by attaching the `onload` handler *before* appending the iframe to the document body. These fixes restore the library's core functionality and improve its reliability.
|
Working (basic kitten demo):
To test:
|
Jules squashed changes
|
Looks like it does not remove the print iframe when complete. Seeing this in:
|
|
Ah, all the demo are set to |
|
With debug disabled: Windows 10 / Edge 113 - ✅ Prints as expected Windows 10 / Chrome 113 - Fails to print. Hidden iframe is created. Print is not triggered. 
|
Interesting. May need to look at pulling back in line 296+ from the old file |
|
Tracked down the Chrome on Windows issue to the Dark Mode extension of all crazy things. I have filed a report with the extension provider. Given the relative popularity of the plugin, we might want to mention it in the Wiki? Windows 10 / Chrome 142 - ✅ Prints as expected |
|
Chrome on any mobile isn't working as expected.
|
|
iOS gets more messy. Some devices are working. Trying to break down the issues and adding some detail:
On the iPhone 15 Pro if I refresh the page and immediately click Print, it works about 1/4 of the time. Never on second click. I was hoping iOS Simulators might be helpful, but they seem to work consistently.
|
|
To make things more complicated, I also tested with the jQuery version on the github.io page
|
|
I added a |
|
In Safari, are we running into the pop-up blocker?
I'd love to not have any setTimeouts, but it's not the end of the world |
|
Current status?
???
|
|
Testing a version with updates based on compatibility issues.
|
|
macOS 15.6.1 / Chrome 142.0.7444.176 - ✅ working |
|
With the latest Saw this in the GSAP forums: |
|
Bah, we had Desktop
iPad
iPhone
|
|
Ugh. Alright, I'm going to strip out options to just the base print function and see how that fares and work back. |
|
|
|
I was wondering if we might need to avoid RAF for iOS. Do you have a preferred method for checking for iOS? The only "reliable" answer I know of is checking the user agent form |
|
No preference. Also digging through chromium references, such as: Which also includes a
|
|
I messed with a number of different implementations and the only success I had was because I temporarily inserted an I'm starting to consider different directions, AKA "bad ideas"...
I'm also going to switch one of my devices to a Developer iOS version to test, and will submit the issue through the Developer Feedback app. Unlikely to provide an answer, but I'm not sure what else to do. |
2 participants
This PR finalizes the conversion of the
printThislibrary to a fully event-driven, vanilla JavaScript module. It fixes a critical race condition with theiframe.onloadevent and addresses all feedback from previous code reviews, resulting in a stable and reliable library.PR created automatically by Jules for task 12272622621294396334 started by @jasonday