Skip to content

fix(pdf): a pdf now dont forget its name when it gets downloadet #10048

Open
Excubitorum wants to merge 4 commits into
TriliumNext:mainfrom
Excubitorum:fix/pdf-download-filename
Open

fix(pdf): a pdf now dont forget its name when it gets downloadet #10048
Excubitorum wants to merge 4 commits into
TriliumNext:mainfrom
Excubitorum:fix/pdf-download-filename

Conversation

@Excubitorum

@Excubitorum Excubitorum commented Jun 4, 2026

Copy link
Copy Markdown

Thanks for your work this Project looks really promissing and nice

  • Replaces trilium-request-download postMessage with a direct call to /api/notes/:id/download
  • The server endpoint sets Content-Disposition: attachment; filename="<note title>.pdf" via BNote.getFileName(), so the browser uses the Trilium note title as the default download filename
  • Adds Electron support via electronApi.shell.downloadURL()
  • Fixes read-only PDFs where the iframe handler (manageDownload) was never registered (it only runs when editable=1)

Test plan

  • Open a PDF note, click download → filename should match the note title
  • Works for read-only PDFs (no #readOnly label required)
  • Works in browser (server mode)
grafik grafik

Replace the postMessage-based download (trilium-request-download) with a
direct call to /api/notes/:id/download, which sets Content-Disposition with
the note title. Adds Electron support and fixes read-only notes where the
iframe handler was never registered.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jun 4, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the PDF preview widget to handle custom downloads directly. Instead of posting a message to an iframe, it now triggers a download using window.electronApi.shell.downloadURL in Electron environments, or by updating window.location.href in web environments. A review comment correctly identifies that the constructed URL may be relative, which will cause the Electron download API to fail, and suggests resolving this by converting the URL to an absolute path using the new URL constructor.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread apps/client/src/widgets/type_widgets/file/Pdf.tsx Outdated
@greptile-apps

greptile-apps Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes PDF downloads so the browser uses the Trilium note title as the default filename instead of a generic name. It replaces the old trilium-request-download iframe postMessage approach (which bypassed the server's Content-Disposition header) with a direct call to /api/notes/:id/download via the existing open.getUrlForDownload()/open.download() helpers.

  • Electron and browser environments are both handled correctly: open.getUrlForDownload() returns an absolute URL in Electron and a relative path in web mode, while open.download() routes to electronApi.shell.downloadURL() or window.location.href accordingly.
  • The customDownload handler is now unconditionally registered in the parent component, fixing downloads on read-only PDFs where the iframe's manageDownload callback was never wired up.

Confidence Score: 5/5

The change is safe to merge — it replaces a broken iframe postMessage path with the already-tested open.ts helpers used consistently across all other file download flows.

The fix is narrow and surgical: three lines in one file replace the old postMessage approach with open.getUrlForDownload() and open.download(), which are already used for every other file type in the codebase.

No files require special attention.

Important Files Changed

Filename Overview
apps/client/src/widgets/type_widgets/file/Pdf.tsx Replaces iframe postMessage download with a direct call to open.getUrlForDownload()/open.download(), fixing filename preservation and read-only PDF downloads.

Sequence Diagram

sequenceDiagram
    participant U as User click Download
    participant O as open.ts downloadFileNote
    participant P as Pdf.tsx useTriliumEvent
    participant S as Server api notes id download

    U->>O: downloadFileNote(note)
    O->>P: triggerEvent customDownload ntxId
    P->>P: open.getUrlForDownload
    alt Electron
        P->>P: electronApi.shell.downloadURL(absoluteUrl)
    else Browser
        P->>P: "window.location.href = relativeUrl"
    end
    P->>S: GET api/notes/id/download?timestamp
    S-->>U: Content-Disposition attachment filename note-title.pdf
Loading

Reviews (2): Last reviewed commit: "Merge branch 'fix/pdf-download-filename'..." | Re-trigger Greptile

Comment thread apps/client/src/widgets/type_widgets/file/Pdf.tsx Outdated
excubitor and others added 2 commits June 4, 2026 09:25
Use getUrlForDownload() and download() from open.ts instead of inline
logic to ensure the correct absolute URL is passed to the Electron
downloader.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant