fix: add MIME types for OpenDocument formats#41648
Conversation
|
IMO this is an easy "fix", to add these MIME types. |
DeepDiver1975
left a comment
There was a problem hiding this comment.
🤖 Automated code review by Claude Code review agent
Overview
This PR adds MIME-type mappings for the OpenDocument file extensions that were previously unmapped (served as application/octet-stream), plus matching icon aliases. It directly resolves #36467 ("Some MIME types missing for OpenDocument formats"). An earlier automated analysis of that issue identified six missing extensions — odc, otc, odi, oti, odm, oth — and all six are covered here. The PR also goes further and adds otg, otp, ots, ott, which were missing too. Nice.
Correctness (MIME strings)
All MIME strings are the official OASIS OpenDocument types and match exactly:
odc→application/vnd.oasis.opendocument.chart✓otc→application/vnd.oasis.opendocument.chart-template✓odi→application/vnd.oasis.opendocument.image✓oti→application/vnd.oasis.opendocument.image-template✓odm→application/vnd.oasis.opendocument.text-master✓oth→application/vnd.oasis.opendocument.text-web✓otg/otp/ots/ott→ graphics/presentation/spreadsheet/text-template✓
No typos, no accidental removals. Both mimetypemapping.dist.json and mimetypealiases.dist.json remain valid JSON (205 and 88 keys respectively). The new mapping keys are inserted in correct alphabetical position (odc before odf; otc/otg/oth/oti/otp/ots/ott correctly ordered around otf).
Completeness (icons / aliases / changelog)
- Aliases: the new
chart/imageentries map tox-office/drawing. Reasonable (chart/image have no dedicated icon). They are placed in the existingtext-web→graphics-flat-xmlblock, which is fine — the OD section of this file is grouped by icon target rather than strictly alphabetical, so the placement matches the existing convention. - JS-side icon list (optional):
core/js/mimetypelist.jscarries its own copy of these OD→icon mappings and currently also lacks the chart/image entries. This PR does not touch it. The.dist.jsonaliases are the authoritative source, so functionality is correct, but if you want the bundled JS hint list consistent you may consider mirroring the four chart/image aliases intomimetypelist.js. Optional — not blocking. - Changelog: present (
changelog/unreleased/41648) — good. Minor nit: the body lists "odc, odi, odm, otc, and otg" and omitsoth/oti/otp/ots/ottthat the PR also adds. Cosmetic only.
Verdict
Approve with minor nits. The PR correctly and completely fixes #36467 — all six extensions flagged by the prior automated analysis are mapped with the right OASIS MIME types, JSON is valid, ordering is consistent, and a changelog entry is included. The only suggestions are optional (mirror chart/image aliases into mimetypelist.js, and tidy the changelog wording). Thanks @phil-davis.
15b5145 to
6484894
Compare
Add MIME type mappings for additional OpenDocument formats for file types odc, odi, odm, otc, otg, oth, oti, otp, ots and ott. Add MIME type aliases for chart and image formats that were currently missing in the default mimetypealiases.dist.json file. Update mimetypelist.js using ./occ maintenance:mimetype:update-js
6484894 to
540db40
Compare
DeepDiver1975
left a comment
There was a problem hiding this comment.
🤖 Automated code review by Claude Code review agent
Re-reviewed after the HEAD commit changed since the last sweep (prior verdict: approve-with-nits). The new push is a meaningfully expanded and improved version of the change.
What changed since the last review
- Mapping now covers the full OpenDocument template/format set:
odc,odi,odm,otc,otg,oth,oti,otp,ots,ott(previously a narrower set). - The two prior optional nits are both resolved:
- mimetypelist.js icons now mirrored — regenerated via
occ maintenance:mimetype:update-js; chart/image OpenDocument types now have icon entries (x-office/drawing), consistent with the existinggraphicsentries. - changelog present and well-formed (
Bugfix:header + body + PR link), matching repo convention.
- mimetypelist.js icons now mirrored — regenerated via
- Chart/image aliases added to
mimetypealiases.dist.json.
Verification performed
- All 10 MIME strings are the correct official OASIS OpenDocument media types — no typos (
...chart,...chart-template,...image,...image-template,...text-master,...text-web,...graphics-template,...presentation-template,...spreadsheet-template,...text-template). - Both
*.dist.jsonfiles validated as syntactically valid JSON on the PR branch. - No accidental removals — the change is purely additive. The single "deletion" in the diff stat is just the regenerator alphabetizing the JS alias array (
x-office-drawingbeforex-office-presentation). - Ordering: mapping additions are alphabetically placed within their clusters; alias additions follow the file's existing (loose, grouped — not strict-alpha) convention as already present on master. Consistent with repo conventions.
- The unrelated
application/tomlandapplication/x-openvpn-profileentries that appear in the JS diff are pre-existing mappings on master that the regeneration tool correctly surfaced into the JS — not new behavior introduced by this PR. - No regression risk: surfacing existing data + additive MIME mappings.
Clean, complete, and well-formed. No requested changes, no caveats.
Verdict: approve.
Add MIME type mappings for additional OpenDocument formats for file types odc, odi, odm, otc, and otg.
Add MIME type aliases for chart and image formats that were currently missing in the default mimetypealiases.dist.json file.
Update mimetypelist.js using ./occ maintenance:mimetype:update-js
Fixes #36467