Render PDF file previews instead of "Preview not available" - #1979
Merged
Conversation
3 tasks
jsilets
force-pushed
the
pdf-file-previews
branch
from
August 20, 2026 15:38
e93c472 to
bce1e3d
Compare
Contributor
Author
|
Rebased onto main and addressed the review feedback in #1985 (comment): the classifier moved to |
ymichael
force-pushed
the
pdf-file-previews
branch
2 times, most recently
from
August 20, 2026 21:14
650c28c to
0bccce9
Compare
buildFilePreview branched on image, known text mime types, a UTF-8 fallback, then video, so application/pdf fell through to the unsupported return and the panel showed "Preview not available for application/pdf." The content routes already serve PDFs as application/pdf, so only the frontend was missing. Add a pdf preview kind that frames the preview URL. Chromium hands an application/pdf response to its own viewer, so this needs no PDF library. The frame takes no sandbox attribute: a sandboxed frame blocks the viewer's scripts and the load fails with ERR_BLOCKED_BY_CLIENT. The pdf check sits ahead of the UTF-8 fallback because a PDF with no compressed streams is all printable ASCII and would otherwise decode as source text. The mobile app maps the new kind to its unsupported state, so its behavior is unchanged; a native PDF view there is follow-up work.
ymichael
force-pushed
the
pdf-file-previews
branch
from
August 20, 2026 21:25
0bccce9 to
a7fb808
Compare
ymichael
added a commit
that referenced
this pull request
Aug 20, 2026
## What was wrong PR #1979 correctly persisted the selected host for a project-backed file in the file-opener params, and the PDF plugin read that source. Root Compose did not: it reconstructed `experimental_Original`, copy-path resolution, and Open in editor from the currently selected environment/host plus `fileOpenerOwner`. Because Root Compose intentionally preserves file tabs across context changes, a PDF opened on host A could keep rendering from host A while those native surfaces routed to host or environment B. ## What changed `useThreadFileTabs` now parses the persisted opener params once and treats them as the routed file identity. `fileOpenerOwner` remains responsible only for native presentation state that is not part of the plugin source, such as line ranges and workspace status. Root Compose gives a persisted project-backed opener source precedence over the live compose selection. The same resolved routing now drives the host-rendered project preview, source root and absolute copy path, and editor-open context. An explicit persisted host stays explicit; an omitted host continues to mean primary-host routing. Native file tabs still follow the existing live compose selection. This is direct data flow from the existing persisted source, so it does not add a second host field or change the tab/server/daemon contract. `HOST_DAEMON_PROTOCOL_VERSION` is unchanged. There are no CLI, guide, SDK, or documentation changes because the documented plugin contract already requires this behavior. ## How you verified Regression coverage demonstrates the old behavior and the fix: - With the old live-context route restored, `pnpm exec turbo run test --filter=@bb/app -- src/views/RootComposeView.test.ts` failed the two new routing cases: explicit host A resolved as selected environment/host B, and omitted-primary routing resolved as host B (67 passed, 2 failed). - `pnpm exec turbo run test --filter=@bb/app -- src/components/secondary-panel/useThreadFileTabs.test.ts src/views/RootComposeView.test.ts` — 88 passed. This includes persisted-tab reload under a different project/environment, explicit-host retention, primary-host omission, and unchanged native-tab routing. - `pnpm exec turbo run test --filter=@bb/app --force` — 404 files passed; 3,126 tests passed and 3 skipped. - `pnpm exec turbo run typecheck build --filter=@bb/app` — passed. - `pnpm exec turbo run lint --filter=@bb/app` — passed with 0 errors and 156 existing warnings outside the changed lines. After rebasing onto `origin/main` at `a77a391a48003f364fd4d26bd8cde95e051eee3b`, the focused opener/Root Compose/shared-panel set passed (3 files, 101 tests), and the Turbo app typecheck and build passed. Related to #1979. > AGENT GENERATED: by GPT-5.6-Sol
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
2 participants
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.
What was wrong
buildFilePreviewtests the mime type in this order: image types, knowntext types, a UTF-8 fallback, then video types.
application/pdfmatchesnone of them, so every PDF returns kind
unsupportedand the panel showsPreview not available for application/pdf.The content routes alreadyserve PDFs as
application/pdfwith the raw bytes; the gap was frontendonly.
What changed
A
pdfpreview kind, rendered by framing the preview URL. Chromium hands anapplication/pdfresponse to its built-in viewer, so this needs no PDFlibrary and no new dependency.
apps/app/src/lib/file-preview.ts: apdfkind forapplication/pdf,tested before the UTF-8 fallback. A PDF with no compressed streams is all
printable ASCII, and the fallback would render its markers as source text.
apps/app/src/components/secondary-panel/FilePreview.tsx: apdfstateand a
FilePreviewPdfiframe. The frame sets nosandbox: a sandboxedframe blocks the viewer's own scripts and the load fails with
ERR_BLOCKED_BY_CLIENT(verified in Electron 41 with this app's exactwebPreferences). This is not the exposure the HTML preview sandboxguards against; Chromium gives the bytes to the viewer instead of parsing
them as a document, so the file never executes in the app origin.
apps/app/src/components/secondary-panel/ThreadStorageFilePreview.tsx:routes the new kind, same shape as the image and video branches.
apps/app/src/hooks/queries/environment-queries.ts: PDFs join images andvideo in the
data:URL rewrite, since the workspace surface readspreviews from a JSON route rather than a byte stream.
All four preview surfaces render PDFs: host files, project files, thread
storage, and workspace files. No wire changes
(
HOST_DAEMON_PROTOCOL_VERSIONunchanged), no CLI or doc surfaces.How you verified
Tests (fail before, pass after):
file-preview.test.ts: builds apdfpreview forapplication/pdf.file-preview.test.ts: keeps an all-ASCII PDF a PDF instead of fallingback to text.
FilePreview.test.tsx: frames the preview with nosandboxattribute.turbo typecheckandturbo lint --filter=@bb/appclean (lint warningsunchanged from base). Full
--filter=@bb/appsuite: 405 files, 3107 passed.Manually rendered the new Ladle story in Electron 41: the viewer loads with
its toolbar, page rail, zoom, download, and print controls inside the normal
preview header.
Fixes #1985