✨(frontend) replace native PDF viewer with custom secure viewer#590
✨(frontend) replace native PDF viewer with custom secure viewer#590
Conversation
NathanVss
force-pushed
the
feat/pdf-viewer
branch
2 times, most recently
from
abd620b to
254999a
Compare
React requires camelCase for SVG attributes. Using fill-rule and clip-rule triggers warnings and may cause rendering issues.
Claude Code generates a local .claude directory for project configuration. It should not be tracked.
The single-line command array was hard to read and modify. Splitting it into multiple lines improves readability.
When testing on local network devices (phones, tablets) the Django CSRF check blocks requests from non-localhost origins. This merges CSRF_TRUSTED_ORIGINS env var into the default list.
Documents how to access the dev environment from other devices on the local network, useful for mobile testing.
Adds react-pdf and react-virtualized for the PDF viewer feature. The next.js config transpiles pdfjs-dist to support Safari < 17.4 which lacks ES2023 static initialization blocks.
Implements a full-featured PDF viewer with thumbnail sidebar, zoom controls, page navigation and external link interception. Includes polyfills for older browsers and a fallback component for unsupported environments.
Alpine-based nginx lacks .mjs in its default mime.types causing pdf.worker.mjs to be served as octet-stream which breaks ES module loading.
Adds external link confirmation and outdated browser warning messages in EN, FR and NL. Updates the generic error description to point users to support.
Dynamically imports the PDF viewer to avoid bundling pdfjs for non-PDF files. Falls back to an outdated browser message if the import fails. Updates the modal to use a blurred backdrop and dvh units.
Tests cover PDF rendering, corrupted file handling, JavaScript injection prevention and external link interception with redirect disclaimer.
NathanVss
force-pushed
the
feat/pdf-viewer
branch
from
254999a to
e69cfeb
Compare
Adds entries for the PDF viewer feature, CSRF settings, nginx MIME type fix, local network docs and SVG attribute fix to the unreleased section.
The bundled PDF worker contains minified code with print() calls that trigger the lint-git check. Since this is a vendored file it should be excluded.
The CI lint step rejects usages of the any type. Replaced them with proper type narrowing.
Firefox does not reliably trigger scroll events from synthetic mouse.wheel calls in Playwright. Using a direct scrollTop assignment ensures the virtualized grid processes the scroll across all browsers.
|
PanchoutNathan
left a comment
PanchoutNathan
left a comment
There was a problem hiding this comment.
That's really great work, really, really cool, well done! I'm not accepting just because you're on vacation and we can discuss the details when you get back, but as soon as it's done, I'll accept right away. It's more of a reminder than anything else :) Well done!
| : BASE_WIDTH; | ||
| }, [size.width]); | ||
|
|
||
| const pageHeight = width * 1.414; |
There was a problem hiding this comment.
1.414 ? Maybe add a comment for this value
|
|
||
| const pageHeight = width * 1.414; | ||
|
|
||
| const rowHeightForIndex = useCallback( |
There was a problem hiding this comment.
I think I understand what this constant is, but I think it's good to add a comment.
| <Page | ||
| pageNumber={index + 1} | ||
| width={width} | ||
| scale={zoom} | ||
| loading={pageSkeleton} | ||
| /> |
There was a problem hiding this comment.
Perhaps it can be explained that this is what renders the page thanks to the Document component and that in this context, it displays the pageNumber =
| useEffect(() => { | ||
| let timer: ReturnType<typeof setTimeout>; | ||
| if (props.isOpen) { | ||
| setUnmount(false); | ||
| timer = setTimeout(() => setIsOpenProxy(true), 100); | ||
| } else { | ||
| setIsOpenProxy(false); | ||
| // The 1.1 is to allow for the transition to finish. | ||
| // It is a safety margin to avoid the component being unmounted too early. | ||
| timer = setTimeout(() => setUnmount(true), TRANSITION_DELAY * 1.1); | ||
| } | ||
| return () => clearTimeout(timer); | ||
| }, [props.isOpen]); |
There was a problem hiding this comment.
You know, I know, we know :p It's a shame this PR is really cool and very well done, it's a shame to leave it as is when it can only be done with CSS.
| className={`pdf-preview__sidebar${!isOpen ? " pdf-preview__sidebar--closed" : ""}`} | ||
| > | ||
| {file ? ( | ||
| <Document |
There was a problem hiding this comment.
Double Document react-pdf (PdfPageViewer.tsx:183 + PdfThumbnailSidebar.tsx:107)
The react-pdf Document component is instantiated twice: once in the main viewer and once in the sidebar. This means the PDF is parsed twice. This is potentially a performance choice (unmounting the sidebar when closed), but it doubles the parsing work when the sidebar is open. Perhaps wrapping the PDF viewer only once would further improve performance?
// PreviewPdf.tsx
<Document file={file} options={pdfOptions} onLoadSuccess={...} onLoadError={...}>
<div className="pdf-preview__body">
<PdfThumbnailSidebar ... />
<PdfPageViewer ... />
</div>
</Document>
| import { useRedirectDisclaimer } from "./useRedirectDisclaimer"; | ||
| import { OutdatedBrowserPreview } from "./OutdatedBrowserPreview"; | ||
|
|
||
| pdfjs.GlobalWorkerOptions.workerSrc = "/pdf.worker.mjs"; |
| // onItemClick handles internal PDF links (e.g. table of contents entries). | ||
| // It is called by react-pdf's Document via a viewer ref that is created once | ||
| // with useRef, so the callback is captured in a stale closure from the first | ||
| // render. We use a ref to always access the latest goToPage (which depends | ||
| // on numPages) so navigation targets the correct page. | ||
| // | ||
| // onClick (handlePdfClick) handles regular DOM clicks on the annotation layer | ||
| // — it intercepts external links to show a redirect disclaimer modal. | ||
| const goToPageRef = useRef(goToPage); | ||
| useEffect(() => { | ||
| goToPageRef.current = goToPage; | ||
| }, [goToPage]); |
There was a problem hiding this comment.
Okay, I understand, why not do it directly in the usePdfNavigation hook?It is only used here
| useEffect(() => { | ||
| const controller = new AbortController(); | ||
|
|
||
| const fetchPdf = async () => { | ||
| setError(null); | ||
|
|
||
| try { | ||
| const response = await fetch(src, { | ||
| credentials: "include", | ||
| signal: controller.signal, | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch PDF: ${response.status}`); | ||
| } | ||
|
|
||
| const blob = await response.blob(); | ||
| const filename = src.split("/").pop() || "document.pdf"; | ||
| const pdfFile = new File([blob], filename, { type: "application/pdf" }); | ||
|
|
||
| setFile(pdfFile); | ||
| } catch (err) { | ||
| if (err instanceof DOMException && err.name === "AbortError") return; | ||
| setError(err instanceof Error ? err.message : "Failed to load PDF"); | ||
| } | ||
| }; | ||
|
|
||
| fetchPdf(); | ||
| return () => controller.abort(); | ||
| }, [src]); |
There was a problem hiding this comment.
I don't like this pattern in React at all, plus we already use React-Query everywhere, couldn't we make it so that when we come back to it it's already there, no need to do a fetch?
// hook usePdfFile.ts
import { useQuery } from "@tanstack/react-query";
const fetchPdfFile = async (src: string): Promise<File> => {
const response = await fetch(src, { credentials: "include" });
if (!response.ok) {
throw new Error(`Failed to fetch PDF: ${response.status}`);
}
const blob = await response.blob();
const filename = new URL(src, window.location.origin).pathname.split("/").pop() || "document.pdf";
return new File([blob], filename, { type: "application/pdf" });
};
export const usePdfFile = (src: string) => {
return useQuery({
queryKey: ["pdfFile", src],
queryFn: () => fetchPdfFile(src),
});
};
2 participants
Summary
Replace the native browser PDF iframe with a fully custom PDF viewer built on
react-pdf(pdfjs-dist). The native<iframe>approach had no control overembedded scripts, link handling, or user experience — each browser rendered its
own PDF UI with inconsistent controls, poor mobile support, and zero security
guardrails.
The new viewer provides a unified, secure, and accessible experience across all
browsers.
Video.mp4
Features
Document rendering
react-virtualized— only visible pages arerendered, keeping memory usage low on large documents ( 1500 pages pdf works smoothly on mobile ! )
Controls bar
directly; out-of-range values are clamped, invalid input resets on blur
scroll position is preserved across zoom changes
Thumbnail sidebar
Link handling
the user must explicitly confirm before a new tab opens
without any modal
Mobile
Accessibility
prefers-reduced-motionsupportSecurity
The native iframe gave the browser full control over PDF execution, including
embedded JavaScript. The custom viewer addresses the following attack vectors:
OpenAction /JavaScript)isEvalSupported: falsein pdfjs options — scripts are never executedjavascript:URI linkshttp:,https:only) inuseRedirectDisclaimer;preventDefault()called before the check so the browser never acts on unsafe URIsdata:/vbscript:URI linksnew URL()parsing in a try/catch — malformed hrefs are silently ignorednoopener,noreferrerE2E test coverage (20 tests)
OpenAction)javascript:URI link blocking (including DOM-injected links)Tech stack
react-pdf10.1.0react-virtualized^9.22.5react-zoom-pan-pinch^3.7.0