Fix DOCX image import and render images inside table cells#120
Merged
Conversation
Importing a .docx with embedded images failed with HTTP 400 from the backend /images endpoint, and even when the upload is bypassed the editor silently dropped any image that lives inside a table cell. form2.docx hit both bugs: three PNGs in a deeply nested row of the main project table were invisible after import. - DocxImporter.uploadImages() handed JSZip's untyped Blob (type '') straight to the uploader. FormData then sent the multipart part as application/octet-stream, which ImageService.upload() rejects against its png/jpeg/gif/webp allowlist. The importer now repackages the bytes with a MIME derived from the .rels target extension before calling the uploader. - renderTableContent() in the table renderer had no image branch and was calling fillText(run.text) on every run, even image inlines whose run.text is the Object Replacement Character. The body path in doc-canvas handled this correctly; the table path was simply missing the mirror drawImage branch. Extract the shared imageCache / getOrLoadImage into packages/docs/src/view/ image-cache.ts so both renderers use a single load cache, and teach renderTableContent to bottom-align and drawImage image runs, scheduling a repaint via requestRender once the load resolves. Adds regression tests on both paths: the DOCX importer test now asserts the uploader receives a Blob with a concrete image/* type, and the table-renderer test asserts an image run is never painted via fillText(ORC) and that the drawImage entry point is reached. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUpdated DOCX image import and Canvas rendering: importer now derives and sets image MIME types for uploads; image caching/loading moved to a shared module; table renderer draws inline images (no ORC glyphs); docs and tests added to record lessons and verify fixes. Changes
Sequence DiagramsequenceDiagram
participant Importer as DOCX Importer
participant Uploader as File Uploader
participant Renderer as Table Renderer
participant Cache as Image Cache
participant Canvas as Canvas Context
Importer->>Importer: extract image bytes from JSZip (Blob.type empty)
Importer->>Importer: derive ext from .rels target (e.g., .png)
Importer->>Importer: repackage bytes into Blob type:image/png
Importer->>Uploader: upload repackaged image Blob
Renderer->>Cache: getOrLoadImage(src, onLoad)
alt cached & loaded
Cache-->>Renderer: return HTMLImageElement
Renderer->>Canvas: drawImage(img, x,y,w,h)
else not cached or pending
Cache->>Cache: create Image, start load, store pending onLoad
Cache-->>Renderer: return null
Note over Cache: when image loads, invoke pending callbacks
Cache-->>Renderer: onLoad callback triggers
Renderer->>Canvas: drawImage(loadedImg, x,y,w,h)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Verification: verify:selfResult: ✅ PASS in 113.9s
Verification: verify:integrationResult: ✅ PASS |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/tasks/archive/2026/04/20260412-docx-image-import-render-todo.md`:
- Around line 11-13: The fenced code block containing the text "Unsupported file
type: application/octet-stream" is missing a language specifier; update that
backtick fence to include a language (e.g., change ``` to ```text) so the block
becomes a labeled code fence and resolves MD040, leaving the inner content
unchanged.
In `@packages/docs/test/view/table-renderer.test.ts`:
- Around line 255-277: The test titled "invokes requestRender once while the
image is still loading" checks drawImage but never asserts on requestRender;
update the test (inside the same it block using makeRecordingCtx(),
makeImageCellTable() and renderTableContent(..., requestRender)) to assert that
requestRender was called once (e.g.,
expect(requestRender).toHaveBeenCalledTimes(1)) so the regression path is
actually covered; keep the existing drawImage assertion as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 28814b46-da3d-4d61-9a4c-29ee966096cf
📒 Files selected for processing (10)
docs/tasks/README.mddocs/tasks/archive/2026/04/20260412-docx-image-import-render-lessons.mddocs/tasks/archive/2026/04/20260412-docx-image-import-render-todo.mddocs/tasks/archive/README.mdpackages/docs/src/import/docx-importer.tspackages/docs/src/view/doc-canvas.tspackages/docs/src/view/image-cache.tspackages/docs/src/view/table-renderer.tspackages/docs/test/import/docx-importer.test.tspackages/docs/test/view/table-renderer.test.ts
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Two minor follow-ups on the DOCX image import / table-cell render fix: - Rename the 'invokes requestRender once' test to actually match what it verifies and add the missing negative assertion on requestRender. Under jsdom the image load is asynchronous, so during a single renderTableContent call neither drawImage nor requestRender should fire — the test now asserts both. - Add a `text` language tag to the fenced error-message block in the archived task doc so markdownlint MD040 passes. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Importing a
.docxwith embedded images hit two separate bugs that together made image-bearing files unusable in the docs editor.~/Downloads/form2.docxreproduced both on the local dev server — three PNGs nested inside the main "신청 프로젝트 정보" table were invisible after import./imageson every embedded image.DocxImporter.uploadImages()handed JSZip's untypedBlob(type'') straight to the uploader.FormDatathen sent the multipart part asapplication/octet-stream, whichImageService.upload()rejects against itspng/jpeg/gif/webpallowlist. The importer now repackages the bytes with a MIME derived from the.relstarget extension before calling the uploader.renderTableContent()unconditionally calledfillText(run.text)on every run — including image runs whoserun.textis the Object Replacement Character (\uFFFC). The body path indoc-canvashandled this viadrawImage; the table path was simply missing the mirror branch. Extracted the sharedimageCache/getOrLoadImageintopackages/docs/src/view/image-cache.tsso both renderers share a single load cache, and taughtrenderTableContentto bottom-align image runs and calldrawImage, scheduling a repaint viarequestRenderonce the load resolves.Test plan
pnpm --filter @wafflebase/docs test— 491 → 493 (two new regression cases)pnpm verify:fastform2.docxon local dev server:type="image/png"→ HTTP 201Regression tests
packages/docs/test/import/docx-importer.test.ts— asserts the uploader receives aBlobwithtype === 'image/png'and the correct filename.packages/docs/test/view/table-renderer.test.ts— asserts an image run is never drawn viafillText(ORC)and thatdrawImageis never called while the image is still loading (entry point for the cache).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests