fix: forward type and quality options from toBlob to canvasToBlob - #590
fix: forward type and quality options from toBlob to canvasToBlob#590Kebechet wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new tests likely violate existing spec-file lint conventions and include a null-dereference hazard, and the change exposes an existing quality: 0 handling bug that should be addressed for correctness.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR fixes toBlob so it correctly forwards type/quality export options through to canvasToBlob, aligning blob output with the options already respected by toCanvas and canvasToBlob.
Changes:
- Pass
optionsfromtoBlobintocanvasToBlobso requestedtype/qualityare honored. - Add new
toBlobspecs verifying defaultimage/png, honoringtype, and honoringquality(via size comparison).
File summaries
| File | Description |
|---|---|
| src/index.ts | Forwards options into canvasToBlob so blob encoding honors type/quality. |
| test/spec/to-blob.spec.ts | Adds regression coverage for toBlob defaults and option forwarding. |
Review details
Suppressed comments (1)
test/spec/to-blob.spec.ts:40
- This test dereferences
lowQualityBlob/highQualityBlobwith!without asserting they are non-null first; iftoBlobreturns null (e.g., unsupported type), the test will throw a TypeError instead of failing with a clear expectation message.
.then(([lowQualityBlob, highQualityBlob]) => {
expect(lowQualityBlob!.size).toBeLessThan(highQualityBlob!.size)
done()
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Measured on a real iPhone: WebKit rasterizes the capture's intermediate SVG before large embedded images have decoded, blanking them in the output, and a plain retry or pre-decoding does not recover it - only comparing consecutive captures until they agree does. Captures now repeat until two consecutive results are identical (StabilizeAttempts, default 3), and every capture is bounded by CaptureTimeoutMs because upstream's createImage never settles when Safari rejects HTMLImageElement.decode() under memory pressure. Both mitigations are proposed upstream: bubkoo/html-to-image#589 (hang) bubkoo/html-to-image#591 (stabilization) bubkoo/html-to-image#590 (the toBlob type/quality deviation this wrapper already works around) Post-merge cleanup tracked in #1
Problem
toBlobforwards its options totoCanvas, but then callscanvasToBlobwithout them:canvasToBlobacceptstypeandqualityand defaults them toimage/png/1, sotoBlob(node, { type: 'image/jpeg', quality: 0.5 })silently returns a PNG at quality 1 — the requested type and quality are dropped on the floor.Fix
Pass
optionsthrough tocanvasToBlob. One line;canvasToBlobalready implements the handling.Tests
test/spec/to-blob.spec.ts:image/png;type: 'image/jpeg'produces a blob withtype === 'image/jpeg'— on current master this fails withimage/png;quality: 0.01is smaller than the same capture atquality: 1— on current master both blobs are byte-identical PNGs.