fix: honor font-family in PDF export without LibreOffice (#8245) - #8249
Conversation
`ExportPdfNative.ts` — the in-process PDF renderer that runs on every install with no `soffice` configured, which is the default — only ever drew with pdfkit's Helvetica and Courier. Any `font-family` applied in the pad was dropped from the PDF, while HTML, ODT and DOCX carried it correctly (ep_font_family #173). pdfkit's built-in PDF "standard 14" fonts cover three families (Helvetica, Times, Courier) with regular/bold/italic/bold-italic variants and need no font files on disk; anything else must be registered from a TTF/OTF, which core cannot ship without taking on a font licensing decision. So the renderer now resolves a CSS font-family list to a built-in by category — sans-serif faces to Helvetica, serif faces to Times, monospace faces to Courier — which keeps serif, sans and monospace text visually distinct and covers every family ep_font_family offers. Bold and italic pick the matching variant. Operators who need an exact face register font files through the new `exportPdfFonts` setting, keyed by CSS family name. A missing, unreadable or malformed font file logs a warning and degrades to a built-in font; an export is never failed over a font. Families that match nothing inherit the enclosing font, so a pad with no font styling produces byte-identical output to before. Font names reaching the renderer come from pad content, so they are normalised and looked up in an allow-list (with own-property checks, so names like `constructor` cannot resolve) and never used as a file path or passed to pdfkit verbatim. Only the operator-controlled setting can name a file, which keeps the export-HTML restrictions from GHSA-6vx2-3gwr-958v intact. Adds backend coverage for the category mapping, bold/italic variants, font-family list walking, the `data-font-family` form plugins using exportHtmlAdditionalTagsWithData emit, unknown-family fallback, unchanged unstyled output, and every `exportPdfFonts` degradation path. Documents the setting in settings.json.template and doc/faq.md. Fixes #8245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EVGSvGqsCVzHFPneVbLVrB
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoHonor font families in native PDF exports
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
1.
|
| const styleFamily = resolveFamilyList(parseFontFamily(attribs.style)); | ||
| if (styleFamily) { | ||
| next.fontFamily = styleFamily; | ||
| } else { |
There was a problem hiding this comment.
2. Exported code ignores inherited fonts 🐞 Bug ≡ Correctness
onopentag retains the tag-assigned Courier family when an explicit font-family declaration resolves to no known family, rather than restoring the enclosing state's family. This occurs on code, pre, tt, kbd, and samp elements with an unknown family, so code nested inside a serif or configured-font span renders as Courier despite the documented inheritance behavior.
Agent Prompt
## Issue description
Unknown explicit font families on code-like tags retain the tag's Courier default instead of inheriting the enclosing font.
## Fix Focus Areas
- src/node/utils/ExportPdfNative.ts[421-434]
- src/node/utils/ExportPdfNative.ts[483-499]
## Recommended Fix
Distinguish an absent font declaration from a present but unresolved declaration. When an explicit style or data font value is present but no candidate resolves, restore `cur.fontFamily`; otherwise preserve the tag-specific default.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
3.3.6 shipped upstream, so the changelog entry this branch added under the 3.3.6 heading is dropped; release notes are written at release time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw
Review feedback on #8249. `parseFontFamily()` read only the first `font-family` declaration in a style attribute and kept a trailing `!important` in the value, so `font-family: serif !important` produced the lookup key `serif !important`, which matches nothing and silently inherited the enclosing font. It now walks every declaration, strips the `!important` flag before normalising, and picks the winner the way CSS does: the last declaration, unless an earlier one is flagged `!important`. Also documents the other half of the fallback rule, which review read as a bug: a family that resolves to nothing leaves the element on whatever font it would otherwise use. For ordinary elements that is the enclosing font; for `code`/`pre`/`tt`/`kbd`/`samp` it stays Courier, deliberately — an unusable font name is no reason to render code in a proportional face. Covered by tests either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw
|
Thanks @qodo-free-for-open-source-projects — both findings actioned in bc6bff4. 1. 2. Code-like tags with an unknown family (Medium) — intended, now documented and tested. The behaviour is deliberate rather than an oversight: when a family resolves to nothing the renderer leaves the element on whatever font it would otherwise use. For ordinary elements that is the enclosing font; for The branch is also merged up to current 🤖 Generated with Claude Code |
Thanks for the detailed follow-up. The changes in |
Review feedback on #8249. `readFontFile()` cached both font buffers and read failures forever, keyed only by path. An operator who corrected a wrong `exportPdfFonts` path, or replaced a face in place, kept getting the old bytes — or the fallback — until Etherpad was restarted. The cache now keys on the file's mtime and size as well as its path, and failures are not cached at all, so a font that appears or changes at a configured path is picked up by the next export. One `stat` per font variant per export. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw
|
Third finding actioned in 7db606b. 3. Stale font-file cache (Medium) — fixed. Correct: 🤖 Generated with Claude Code |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw
What
The built-in PDF export path —
src/node/utils/ExportPdfNative.ts, which runs on every install that has nosofficeconfigured, i.e. the default — only ever drew with pdfkit's Helvetica and Courier. Anyfont-familyapplied in the pad was silently dropped from the PDF, while HTML, ODT and DOCX carried it correctly (ep_font_family #173).The mapping, and why
pdfkit's built-in PDF "standard 14" fonts cover three families — Helvetica, Times and Courier — each with regular/bold/italic/bold-italic variants, and they need no font files on disk. Anything else has to be registered from a TTF/OTF, which core cannot do out of the box without bundling fonts, and that is a licensing decision rather than a technical one.
So the renderer resolves a CSS font-family list to a built-in by category:
sans-serif, Arial, Helvetica, Calibri, Verdana, Avant Garde, …serif, Times New Roman, Georgia, Garamond, Palatino, Bookman, …monospace, Courier, Courier New, Consolas, Menlo, …A pad set in Garamond renders as Times rather than Helvetica — not the exact face, but the right kind of face. That covers every family
ep_font_familyoffers and is the difference between a readable export and one that loses the distinction entirely. Bold and italic select the matching variant, so<b>inside a serif span gets Times-Bold.The family list is walked in order, so
font-family: 'Fancy Face', Georgia, serifresolves to Times. A family that matches nothing inherits the enclosing font rather than resetting it, which is why a pad with no font styling produces byte-identical output to before (verified againstdevelopfor plain text, headings, bold/italic, lists,<code>/<pre>, alignment and links).Exact faces: the
exportPdfFontssettingOperators who need a real face register font files themselves:
Keys are CSS family names, matched case-insensitively and treating
-,_and spaces alike, soTimes New Romanand thetimes-new-romanthatep_font_familyemits are the same key. A bare string is used for every variant; a variant with no file configured degrades to the regular face. Relative paths resolve against the Etherpad root. Registered families take priority over the built-in mapping, so an install with a real Arial can use it.Every failure path degrades: a missing file, an unreadable file or a malformed TTF logs a warning and falls back to
fallback(or Helvetica). An export is never failed because of a font. Etherpad ships no fonts for this setting.What remains unsupported
Exact non-standard faces without operator configuration. Shipping them would mean bundling font files in an Apache-2.0 project, so that is deliberately left to the operator.
Security
Family names reaching the renderer come from pad content (plugins emit
<span style="font-family:…">viagetLineHTMLForExport). They are normalised and looked up in an allow-list using own-property checks — soconstructor,__proto__and friends cannot resolve to anything — and are never used as a file path or passed to pdfkit verbatim. Only the operator-controlled setting can name a file. Nothing unsanitised is reintroduced into the PDF path; the export-HTML restrictions from GHSA-6vx2-3gwr-958v are untouched.Tests
New backend coverage in
src/tests/backend/specs/export.ts, asserting on the/BaseFontentries of the produced PDF:Times-Roman/Helvetica/Courierdata-font-family(the formexportHtmlAdditionalTagsWithDataplugins produce) is honouredHelvetica/Helvetica-Bold/Helvetica-Oblique, and<code>still usesCourierObject.prototypeproperty names resolve to nothingexportPdfFonts: file embedded, per-variant files, bare-string form, missing file → configured fallback, non-font file → fallback, override of a built-in familyVerification
Real pad on a local instance with
ep_font_familyinstalled: three lines, one set to Times New Roman, one to Monospace, one left plain.GET /p/<pad>/export/pdfreturns a PDF whose base fonts areTimes-Roman,CourierandHelvetica. WithexportPdfFontspointing the Times family at a TTF, the same export embeds that font file instead.Full backend suite, vitest and
tsc --noEmitrun locally.Fixes #8245
🤖 Generated with Claude Code
https://claude.ai/code/session_01EVGSvGqsCVzHFPneVbLVrB