feat: support TIFF images in DOCX rendering#2193
feat: support TIFF images in DOCX rendering#2193gpardhivvarma wants to merge 8 commits intosuperdoc-dev:mainfrom
Conversation
TIFF images in DOCX files rendered as broken icons because browsers cannot natively display image/tiff. Convert TIFF to PNG at import time using utif2, following the existing EMF/WMF → SVG conversion pattern. Closes superdoc-dev#2064
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6335817b6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/tiff-converter.js
Show resolved
Hide resolved
Reject TIFF images exceeding 100M pixels before allocating RGBA buffers or canvas, preventing a malicious TIFF with extreme dimensions from freezing or crashing the tab during import.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f0832d14d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/tiff-converter.js
Show resolved
Hide resolved
Move MAX_PIXEL_COUNT check before UTIF.decodeImage/toRGBA8 so oversized TIFFs are rejected before allocating the RGBA buffer. Map .tif extension to image/tiff in Content_Types.xml generation to avoid emitting the invalid MIME type image/tif.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c061d636b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/tiff-converter.js
Outdated
Show resolved
Hide resolved
UTIF.decode populates raw tag entries (t256/t257) but .width/.height are only set after decodeImage. Read from raw tags so the pixel limit guard works before the expensive decode step without rejecting valid files.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
nice work - clean structure, follows the metafile converter pattern well, DoS guard is in the right place.
added a few comments
...es/super-editor/src/core/super-converter/v3/handlers/wp/helpers/encode-image-node-helpers.js
Outdated
Show resolved
Hide resolved
packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/tiff-converter.test.js
Show resolved
Hide resolved
- Use mimeTypeForExt mapping for .tif data URIs (image/tiff not image/tif) - Remove unused size arg from convertTiffToPng call - Add happy-path test asserting valid TIFF produces PNG data URI
…port # Conflicts: # packages/super-editor/src/core/DocxZipper.js
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
@gpardhivvarma all three comments from last round are resolved, and the overall approach looks good. left two small inline suggestions — nothing blocking, just minor cleanup.
...es/super-editor/src/core/super-converter/v3/handlers/wp/helpers/encode-image-node-helpers.js
Show resolved
Hide resolved
packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/tiff-converter.js
Show resolved
Hide resolved
- Remove unused Uint8Array/ArrayBufferView branches (only strings are passed) - Add handleImageNode test verifying convertTiffToPng is called for .tif files
|
@caio-pizzol made the requested additions please take a look at them |
2 participants
Summary
utif2, following the existing EMF/WMF → SVG conversion patterntifto DocxZipper image extension sets so.tiffiles are properly extracted as data URIsoriginalSrc/originalExtensionattributesCloses #2064
Test plan
isTiffExtension(tiff/tif/TIFF/other/null/undefined)convertTiffToPng(invalid data, null, empty, non-TIFF base64)