fix: render PDFs with pdf.js instead of a native iframe#2
Open
shanzez wants to merge 3 commits into
Open
Conversation
EmbeddedPDF.svelte used a native iframe pointed at the file URL, which the browser leaves blank when the file server sends X-Frame-Options: SAMEORIGIN (the desktop app runs its UI on a local origin and fetches files cross-origin) and which does not display PDFs at all in Safari/WebKit. Render with pdf.js (canvas) instead, which is immune to X-Frame-Options, CSP frame-ancestors, and per-browser native PDF viewer differences. - EmbeddedHTML.svelte no longer delegates to EmbeddedPDF; it owns its own native iframe plus css-injection implementation (used only for the DOCX-converted-to-HTML preview, which is same-origin blob content and unaffected by the PDF cross-origin issue). - EmbeddedPDF.svelte now loads documents via pdfjs-dist, with withCredentials true (cookie auth, no token in the URL), cMaps and standard fonts for CJK and non-embedded standard-font rendering, loading and error states, and full cleanup (cancel render tasks, destroy the document/loading task, terminate the worker) on unmount and on src change. The css prop, only ever needed by EmbeddedHTML, is removed from its public API. - The pdf.js worker follows the same resilience pattern already used for the recorder worker: primary construction via new Worker with a new URL built from import.meta.url, falling back to a fixed path emitted by an explicit webpack entry if that construction throws. desktop/webpack.config.js gains that entry plus CopyPlugin patterns for pdfjs-dist's cmaps and standard_fonts directories; the plain web build config gains the same cmaps/standard_fonts copying. - Added pdfjs-dist 4.10.38 to packages/ui. services/rekoni already vendors an older pdfjs-dist server-side for unrelated PDF text extraction, so two majors coexisting in the repo is expected. - Added a colocated unit test for the extractable render-scale helper. The worker construction stays inline in the component, matching the pre-existing recorder worker, since import.meta.url is ESM-only syntax that this package's ts-jest/CommonJS test transform cannot parse. Fixes Platform-Collective#1
Follow-up to the pdf.js EmbeddedPDF rewrite, resolving CI and review findings. - rush check: allow pdfjs-dist's two versions (rekoni's 2.12.313 and the new 4.10.38 in packages/ui) via allowedAlternativeVersions in common-versions.json. - Cross-origin auth: drop withCredentials (it maps to credentials:'include', which the file server rejects cross-origin — it responds with Access-Control-Allow-Origin: * and no Access-Control-Allow-Credentials). Instead append the client token as a ?token= query parameter (the server accepts it, and a plain non-credentialed request passes CORS). The token is read from presentation.metadata.Token by the two callers and passed via a new EmbeddedPDF `token` prop; appendToken() adds it with the right separator. - Asset paths: resolve cmaps/, standard_fonts/ and the worker fallback from the webpack public path (getAssetBase) instead of document.baseURI/route-relative paths, which 404 under the app's path-based pushState routing. - Text layer: render a pdf.js TextLayer over each page canvas so text is selectable, searchable (Ctrl+F) and screen-reader accessible. - Rescale vs refetch: split the reactive flow so the document is fetched/parsed only when src/token change; toggling `fit` re-paints existing pages at the new scale without refetching. - Pages render at their exact CSS size and scroll when wider than the viewport (with `align-items: safe center`) rather than being scaled down, which would distort the page and misalign the text layer. - Suppress the load-error console log for renders superseded by a newer generation (no console spam on rapid navigation). - Remove the leftover #view=FitH&navpanes=0 hash from EmbeddedHTML's iframe (it now loads HTML, not a PDF), and drop the now-unused css prop from presentation's PDFViewer.
- Disable the naming-convention rule for the `__webpack_public_path__` ambient declaration (webpack magic global, name is fixed by webpack). - Apply prettier line-wrap and prefer-const auto-fixes flagged by `rush fast-format`.
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
Fixes #1.
EmbeddedPDF.svelterendered PDFs with a native<iframe src={fileUrl}>. That is cross-origin-fragile: the browser refuses to render the framed content when the file server sendsX-Frame-Options: SAMEORIGIN(this breaks the desktop app, whose UI runs on a local origin and fetches files from a separate origin), and it renders blank in Safari/WebKit, which does not support PDF-in-iframe at all.This PR switches
EmbeddedPDFto render with pdf.js (fetch bytes into a<canvas>with a real text layer), which is immune toX-Frame-Options/ CSPframe-ancestorsand to per-browser native-PDF-viewer differences.What changed
EmbeddedHTML.svelte— previously delegated toEmbeddedPDF(passing ablob:URL of converted HTML + acssstring injected into the iframe<head>, which is how DOCX-to-HTML preview works). Decoupled first:EmbeddedHTMLnow owns its own small native-iframe implementation (same behavior). HTML content is same-originblob:, so an iframe is correct there; this freesEmbeddedPDFof thecssprop.EmbeddedPDF.svelte— rewritten to render viapdfjs-dist:Access-Control-Allow-Origin: *with noAccess-Control-Allow-Credentials, so a credentialed (withCredentials) fetch is CORS-blocked. Instead the client token (presentation.metadata.Token) is appended as?token=(server-supported), which a plain non-credentialed cross-origin request can carry. Newtokenprop, read by both callers, appended via the pure, unit-testedappendToken().getAssetBase()(webpack's__webpack_public_path__), notdocument.baseURI/route-relative paths — the app uses path-based pushState routing, under which those would 404 off the root. cmaps/standard_fonts are emitted at the bundle root viaCopyPlugin(desktop + web webpack configs).TextLayer+--scale-factor).fit-aware scaling, loading + error states (with a download fallback), and full cleanup (cancel render tasks, destroy the document/loading task, terminate thefromPortworker) on unmount /srcchange. Wide pages scroll rather than being scaled down (which would distort the page and misalign the text layer). Fetch/parse runs only onsrc/tokenchange;fittoggles repaint without refetching.new Worker(new URL(..., import.meta.url))) with a try/catch fallback plus an explicitpdfjs-workerwebpack entry indesktop/webpack.config.js.common-versions.json—pdfjs-distadded toallowedAlternativeVersions(services/rekonialready vendors2.12.313server-side; the UI uses4.10.38).rush checkpasses.Verification
rush check,rush validate,rush svelte-check,rush fast-format, andpackages/uitests pass; full desktop UI webpack build emits the worker chunk +cmaps/+standard_fonts/.cors()defaults +?token=auth +X-Frame-Options: SAMEORIGIN. Credentialed cross-origin fetch is blocked; token-in-URL fetch is 200; pdf.js renders to canvas with a working text layer; the native iframe is blank.Notes
services/rekoni's older server-sidepdfjs-distis unrelated (PDF text extraction), hence the allowlist entry rather than a forced single version.Credit: this fix was put together by the Exaviz team.