Skip to content

[US-2010] Upgrade to Wails v3.0.0 stable with GTK4 on Linux - #40

Merged
unidoc-anom merged 54 commits into
devfrom
feat/wails-v3-stable-upgrade
Nov 5, 2026
Merged

unidoc-anom merged 54 commits into
devfrom
feat/wails-v3-stable-upgrade

Conversation

@unidoc-anom

@unidoc-anom unidoc-anom commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Description and links

Wails v3.0.0 shipped on 2026-10-26 as the first stable v3 release. This moves us off beta.18 on both sides (Go module v3.0.0, @wailsio/runtime exact 3.0.0), raises the Go floor to 1.27.3 for the crypto/x509 fix in GO-2026-6809, and bumps vite to ^8.2.4. Linux now builds against GTK4 / WebKitGTK 6.0, the Wails default since alpha.93, instead of forcing gtk3.

Every upstream changelog entry from beta.19 to v3.0.0 was checked against our code. Most need nothing from us. The one that bites is the new event registration: Wails checks each emit against the type the event was registered with and silently cancels a mismatch.

The upgrade also carries one bug fix and a few small v3 features:

  • Saving an embedded file close to the 50 MB extraction limit failed with a transport error. The bytes went to the window as base64 and came back through SaveBytesToFile, which overflowed the 64 MiB request cap. The save now happens on the backend.
  • File > Open Recent.
  • Section headers in the File and app menus (macOS 14+).
  • Row menus in the structure tree, Pages and Images are now native OS menus. The structure tree gets "Copy object reference" and "Copy path".
  • Linux releases add a .deb (Ubuntu 24.04+, Debian 13, Ubuntu-based systems) and an .rpm (Fedora) next to the tarball. The .deb installs the AppArmor profile WebKit's sandbox needs on Ubuntu 23.10+, without which the GTK4 build exits on start.
  • Navigate > Next Tab / Previous Tab are now Next Document / Previous Document and only enabled with two or more documents open. Go to Page, Find Object, Close Document, Reload from Disk and Compare with File on Disk are disabled with no document open.

Jira: US-2010

Technical changes

  1. Pins: go.mod (go 1.27.3, Wails v3.0.0; tidy moves golang.org/x/image to v0.45.0), frontend/package.json and lockfile, both workflows' wails3 install, tests/wails-version-contract constants. README and CONTRIBUTING updated.
  2. Linux GTK4: CI and release install libgtk-4-dev libwebkitgtk-6.0-dev. The GOFLAGS=-tags=gtk3 step and EXTRA_TAGS=gtk3 are gone, and so are the gtk3 tags in tests/boot-smoke and tests/scaffold. CI runs wails3 doctor on Linux so the job log shows the stack it built against. README lists the runtime packages and the xdg-desktop-portal the file dialogs need. Ubuntu 22.04, Debian 12 and RHEL 9 no longer run the app, and the .deb and .rpm refuse to install there (item 12).
  3. events.go (new): all 23 app events (row-menu and about included) are registered in init(). Events emitted with nil are application.Void, document:disk-status is filewatch.Event, everything else is map[string]any, because that is what every emitter sends today. events_registration_test.go checks each registration against a hand-kept table of the types the emitters send, so a wrong registration fails the test instead of dropping events at runtime. It does not read the emit sites, so changing an emitter's type means updating the table too. The update service registers its own progress event.
  4. Frontend events: App.jsx, TabBar.tsx, UpdateNotifier.tsx and the rest import the generated constants from bindings/events instead of string literals. The document:warning listener in App.jsx had no emitter and is removed. test-setup.ts stubs the generated eventcreate.js, since the tests mock @wailsio/runtime without Create.
  5. Embedded-file save: PDFService.SaveEmbeddedFileToFile reads the attachment, opens the save dialog and writes the file. Read errors return before any dialog is shown, and cancel is silent. The dialog's suggested name keeps only the last path element of the name in the PDF, so ../../evil.sh suggests evil.sh. Characters Windows rejects in a file name, : included, and Unicode control characters, bidi controls such as U+202E and the line and paragraph separators become _ (ZWJ and ZWNJ stay) (C:evil.bat -> C_evil.bat, a.pdf:data -> a.pdf_data), and a Windows device name gets a leading _ (NUL.txt -> _NUL.txt). SaveBytesToFile is removed, along with its mocks in 26 test files.
  6. Open Recent: internal/recentfiles (new) keeps up to 10 paths in pdfdebug/recent.json under the user config dir, saved atomically through the new internal/atomicfile, which updatecheck now shares (rename retry on Windows, stale temp cleanup). On macOS and Windows, paths that differ only in case are one entry when both exist and are the same file; a path that cannot be stat'ed matches only itself, and the stats run outside the list's lock, with a recheck for variants added meanwhile. recentFlusher (recentflush.go) decides when the list is saved and the menu rebuilt: 500ms after the last record once no open is in flight. PDFService.OpenFile, which every entry point goes through, counts opens in flight, so a batch or opens in quick succession save once, and an open still running after 10s stops holding the save. The frontend records each file as it opens, awaited, so the order holds. Clear Menu and a dropped missing file save at once through it. The rebuild runs on the UI thread and skips itself once shutdown has run there, and shutdown waits for a save in progress, then saves a change still waiting. A missing or damaged file starts an empty list. Opens from the Go side record in openFileAndEmitWithWarning. The empty-state Open button and cold-start opens record through the bound RecordRecentFile. Reload and Compare never record. Clicking an entry whose file is gone drops it and shows the normal open error. Labels get the parent folder when two names collide, and & (Windows) / _ (GTK) are doubled so they are not taken as mnemonics.
  7. Row context menus: rowmenus.go registers three native menus (row-reveal, tree-row, tree-row-ref), and RowContextMenu.tsx is gone. The Wails runtime opens a menu from the --custom-contextmenu CSS property of a DOM contextmenu target, so useRowContextMenu sets that property and a fresh token on the panel container when a row is right-clicked, and clears both for rows with no menu (error rows, pages without a reference). Shift+F10 and the ContextMenu key dispatch a contextmenu event at the focused row, so keyboard opens do not depend on the WebView. WebView2 and WebKitGTK also send their own contextmenu for those keys; the next one the hook did not dispatch is cancelled before the Wails runtime sees it, and a pointerdown ends that, so one press opens one menu and a later right-click, touch long-press or pen press still opens one. The guard is set only for the ContextMenu key, since the keydown's preventDefault stops the WebView's own Shift+F10 event, and not on macOS, whose WebKit sends no such event. The menu's hit carries the node's object reference, so Copy object reference does not look the node up again. A clicked item comes back as the typed row-menu:action event with its action and token. The panel acts only if the token is the row it last armed, and a tab or view switch forgets the row. Go never reads anything from the page-supplied data. Copies still go through the Wails clipboard. On Windows, Wails opens every context menu at the mouse pointer, so a keyboard-opened menu shows up there and not under the row.
  8. Navigate and File menu state: navigate:state-changed now also carries documentCount, and the Go handler enables items from it. All of them start disabled. The frontend sends the state on its first render as well, so a reloaded page resets the menu.
  9. build/windows/info.json keys the version block under 0409 (en-US) to match the v3.0.0 template, and the release jq path follows it. A test ties the two together.
  10. Linux boot smoke (ci.yml, tests/boot-smoke). The test runs on a private session bus (dbus-run-session), because on the runner's systemd user bus GTK4's startup D-Bus proxies waited the full 25s timeout and the app never activated. WebKit's sandbox is covered by the packaged AppArmor profile (item 12). The app gets 10s instead of 3s to exit after SIGTERM, since a quit queued behind GTK4's first llvmpipe layout pass on a cold runner can take longer. When shutdown does hang on Linux, the test now prints the process state, the process list, gdb thread stacks and a goroutine dump.
  11. New suite tests/wails-v3-stable-upgrade/ (pins, GTK4 packages, no event-name literals in frontend/src, no SaveBytesToFile, info.json key). Existing suites updated where they grepped for GTK3 packages or quoted event names.
  12. Linux packaging. WebKitGTK 6.0 always starts its web process under bubblewrap, which needs an unprivileged user namespace, and Ubuntu 23.10+ refuses that to programs without an AppArmor profile that allows it. On a stock Ubuntu 24.04 desktop the GTK4 build aborted at start with bwrap: setting up uid map: Permission denied and SIGTRAP; WebKit2GTK 4.1 under GTK3 only sandboxed on request, which is why this never came up. Upstream fix(v3/linux): diagnose blocked WebKit sandbox namespaces wailsapp/wails#6310 only adds a warning. So:
    • build/linux/nfpm/nfpm.yaml is now tracked, with real metadata. It installs the GUI and pdfdebug to /usr/bin, the desktop entry, the icon and LICENSE/NOTICE, and takes the version and arch from the environment.
    • build/linux/apparmor/unidoc-pdf-debugger is an unconfined profile with userns, for /usr/bin/unidoc-pdf-debugger. It ships in the .deb only, as a conffile. postinstall loads it when AppArmor is active; postremove unloads it on remove.
    • CI builds both packages with wails3 task linux:generate:deb|rpm, checks the .deb contents, installs it, checks aa-status, asserts the runner still has the restriction on, and runs the boot smoke against the installed binary (new BOOT_SMOKE_BINARY). The sysctl that lifted the restriction is gone, so a green boot smoke now shows the packaged profile works.
    • The .deb depends on libc6 (>= 2.39) and libgtk-4-1 (>= 4.14), the .rpm on glibc >= 2.39 and gtk4 >= 4.14: the build host's glibc and the GTK that has GtkFileDialog (added in 4.10, used by the Wails GTK4 backend). Without the floors apt installed the .deb on Debian 12 and Ubuntu 22.04, and the app died at the first file dialog.
    • Release stages unidoc-pdf-debugger-<ver>-linux-amd64.deb and .rpm, and EXPECTED_FILES moves from 6 to 8.
    • New suite tests/linux-packaging. README has per-distro install steps, plus the manual profile for the tarball and for source builds on Ubuntu.
  13. Ubuntu runners are pinned to ubuntu-24.04 in ci.yml and release.yml instead of ubuntu-latest. A binary needs the glibc it was built against or newer, and nfpm doesn't add a glibc minimum to the .deb, so a release built on 26.04 would install on 24.04 and Debian 13 and then fail to start. Pinning keeps the build on the oldest supported Ubuntu when GitHub moves ubuntu-latest to 26.04. The README now lists the supported Linux releases.
  14. Linux app identity and About. main.go sets Linux.ApplicationID to com.unidoc.unidoc-pdf-debugger (the single-instance id, now one appID constant), and the desktop entry gets a matching StartupWMClass. Before, Wails derived org.wails.unidoc_pdf_debugger, GNOME could not tie the window to the desktop entry, and the dock showed a gear icon. Windows and Linux also get Help > About, which only existed in the macOS app menu.
  15. About and the splash. About is now a Radix dialog over the main window (about:open carries the version) instead of a second WebviewWindow, which on Linux showed up as its own window with a copy of the menu bar. Choosing About unminimises or focuses the main window first; one still hidden behind the splash stays hidden. A request made before the frontend has mounted is held and sent on its first navigate:state-changed report, so About chosen during the splash opens when the main window appears. The splash stays a window, since it has to paint before the main WebView loads, but on Linux it gets an empty Linux.Menu, starts hidden, and shows on WindowLoadFinished; the main window waits for that, 2s at most and never past the 30s splash timeout (Scheduler.MainWindowReadyWithin stops the timeout as soon as the main window is ready, and a timeout callback already running stands down). Before, WebKitGTK had not painted the splash by the time it was dismissed, so it blinked up blank.

Considerations

The update checker still offers Linux users the tarball, since it matches the -linux-amd64.tar.gz suffix; people who installed the .deb should update with the new .deb.

The .deb and .rpm are unsigned, like the macOS build. There is no apt or dnf repository: users download the package from the release page.

The frontend/bindings/ diff is not reviewable here because the folder is gitignored. The breaking change in it is SaveBytesToFile going away. The rest is additive: the events module, eventdata/eventcreate, and the filewatch model.

Known issue on GNOME (Wayland): opening a PDF from the file manager with Open With while the app is already running opens the document, but the window stays behind and GNOME shows an "'UniDoc PDF Debugger' is ready" notification instead. GNOME only raises a running app when the launch hands it an activation token. GTK takes XDG_ACTIVATION_TOKEN out of the environment when the library loads, so the second instance has to read it from /proc/self/environ and pass it on; even with that, the window did not come forward on the Ubuntu 24.04 VM. Left for a later change.

With no document open, Reload from Disk is disabled in the menu. Cmd/Ctrl+R and F5 still never reload the page: App.jsx cancels them on keydown in every state.

How was this tested?

  • GOFLAGS=-count=1 scripts/test-all.sh on the final tree: go vet, golangci-lint 0 issues, root module, every tests/*/ suite, frontend typecheck, lint and Vitest. All suites passed.
  • go test -race -count=1 on the root package, internal/recentfiles, internal/pdfservice and internal/updateservice passed.
  • tests/boot-smoke passes on macOS, and task package PRODUCTION=true builds and signs the macOS app.
  • go mod tidy -diff is clean and npm ci --dry-run succeeds.
  • CI green on all three runners. On Linux the GTK4 boot smoke passes under xvfb, and wails3 doctor reports no issues: Ubuntu 24.04.5 amd64, gtk4 4.14.5, webkitgtk-6.0 2.52.6, Go 1.27.3, Wails CLI v3.0.0.
  • Native row menus checked by hand on a macOS production build. Not tried on Windows or Linux, including the cancel of the WebView's own keyboard contextmenu.
  • GTK4 build started on an Ubuntu 24.04 desktop VM (arm64) once an AppArmor profile was in place: window, menu bar and empty state render.

Not yet done:

  • macOS production pass: quit via menu, Cmd+Q and SIGTERM; Open Recent; section headers; menu enable states; cmp of a saved embedded file.
  • An hour on Windows.
  • Install the .deb on an Ubuntu 24.04 desktop and run the Linux checklist (dialogs, drop, menus, context menus, Open Recent, quit).

3ace added 2 commits October 28, 2026 22:25
- Pin Wails v3.0.0 and @wailsio/runtime 3.0.0, go 1.27.3, vite 8.2.4+
- Linux CI and release build against GTK4 / WebKitGTK 6.0
- Save embedded files from the backend, removing SaveBytesToFile
- Register typed app events and use generated event constants
- Add Open Recent, menu section headers and tree copy items
@unidoc-anom unidoc-anom changed the title Upgrade to Wails v3.0.0 stable with GTK4 on Linux [US-2010] Upgrade to Wails v3.0.0 stable with GTK4 on Linux Oct 28, 2026
@unidoc-anom
unidoc-anom marked this pull request as draft October 28, 2026 17:39
@unidoc-anom
unidoc-anom marked this pull request as ready for review November 2, 2026 16:28

@anovik anovik 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.

Review notes below. The first three matter most: the ContextMenu key double-open, the Linux splash timeout, and the drive-relative embedded file name.

Comment thread main.go Resolved
Comment thread internal/pdfservice/service.go Resolved
Comment thread main.go Resolved
Comment thread internal/recentfiles/recentfiles.go Resolved
Comment thread internal/recentfiles/recentfiles.go Resolved
Comment thread main.go Resolved
Comment thread frontend/src/App.jsx Resolved
Comment thread frontend/src/components/TreePanel.tsx Resolved
Comment thread frontend/src/components/TreePanel.tsx Resolved

@unidoc-alip unidoc-alip left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: COMMENT - 0 must-fix, 1 new should-fix (F1); of anovik's 10 open comments, 6 hold as should-fix and 4 as nits (one of them demoted)

This PR moves the desktop app from Wails v3.0.0-beta.18 to v3.0.0 stable and switches Linux to GTK4 / WebKitGTK 6.0. It adds .deb and .rpm packages with an AppArmor profile, and carries the backend embedded-file save, File > Open Recent, native row menus, document-dependent menu enable state, About as an in-window dialog, and the Linux splash and app-id fixes.

anovik reviewed this same SHA (35c1d4c) and left 10 inline comments. I do not repeat them here. Under "anovik's comments, checked" each one gets a status (PoC-confirmed, confirmed by source read, or demoted) and a severity, so the threads can be triaged. What this review adds is F1, a packaging issue nobody has raised yet, and the claim-by-claim check of the description. No must-fix. F1 reproduces every time, but it only affects systems the README already lists as unsupported. Each of anovik's items either has a narrow trigger or depends on platform behaviour I could not run. Findings are keyed F1, F2, ... and anovik's comments A1-A10 in the order they were posted, so none of them autolink to an issue.

The PoCs ran on macOS (arm64) and in Docker. The code for each is in the appendix (P1-P5), with the command to run it.


F1 [should-fix] build/linux/nfpm/nfpm.yaml line 41

The .deb depends on libgtk-4-1 and libwebkitgtk-6.0-4 with no version. Debian 12 has both: libgtk-4-1 4.8.3+ds-2+deb12u1 and libwebkitgtk-6.0-4 2.50.6-1~deb12u2 (checked with madison for bookworm). So apt install ./unidoc-pdf-debugger-<ver>-linux-amd64.deb succeeds there. The binary then cannot work: Wails v3.0.0's GTK4 backend (pkg/application/linux_cgo.go) calls gtk_file_dialog_new, gtk_file_dialog_open and the other GtkFileDialog functions, which GTK added in 4.10. The build host's glibc is also newer (2.39 against Debian 12's 2.36). The README and CHANGELOG say Debian 12 is unsupported, but the package lets it install anyway, so the user's first sign of a problem is a broken app rather than an apt error. The description's own item 13 makes the same point about glibc and ubuntu-latest. Ubuntu 22.04 is in the same position: jammy-updates carries libwebkitgtk-6.0-4 2.50.4-0ubuntu0.22.04.1 and libgtk-4-1 4.6.9, so the .deb installs there too (checked with madison; not run in Docker).

PoC-confirmed in Docker, linux/amd64 (Appendix P1). On ubuntu:24.04 I built a small cgo binary that calls gtk_file_dialog_new, the same as Wails' backend. I packed it twice with dpkg-deb: once with this PR's Depends line verbatim, once with the fix below. These are not packages built by wails3 tool package from an edited nfpm.yaml, so the yaml snippet below is untested as yaml. Then I installed each with apt:

[Debian GNU/Linux 12 (bookworm)] head.deb: apt install OK (libgtk-4-1 4.8.3+ds-2+deb12u1, libc6 2.36-9+deb12u14)
  start:  rc=0 started; runtime gtk 4.8.3
  dialog: rc=127 poc-gtkdep: symbol lookup error: poc-gtkdep: undefined symbol: gtk_file_dialog_new
[Debian GNU/Linux 12 (bookworm)] fixed.deb: apt install REFUSED
[Ubuntu 24.04.5 LTS] head.deb: apt install OK (libgtk-4-1 4.14.5+ds-0ubuntu0.10)
  dialog: rc=0 file dialog created

The PoC binary uses lazy binding and needs only GLIBC_2.34. It starts, then dies at the first file dialog. I did not inspect the real app binary's binding mode or glibc symbol versions, so on Debian 12 it either fails at start or at the first Open/Save. Both are broken.

Suggested fix: state the build host's floor, so apt refuses an unsupported system:

depends:
  - libc6 (>= 2.39)
  - libgtk-4-1 (>= 4.14)
  - libwebkitgtk-6.0-4
...
overrides:
  rpm:
    depends:
      - glibc >= 2.39
      - gtk4 >= 4.14
      - webkitgtk6.0

4.14 is what CI and releases build against (wails3 doctor in the ubuntu-24.04 job reports gtk4 4.14.5). The hard minimum for GtkFileDialog is 4.10. With these depends, Ubuntu 24.04 (4.14.5) and Debian 13 (4.18) still install. The rpm lines are source-read only; I did not test the rpm override.


anovik's comments, checked

A1 [should-fix] frontend/src/components/navigatorHooks.ts:162 - ContextMenu key arms two tokens

Consequence confirmed by PoC (Appendix P2); the premise is not verified. I ran the real useRowContextMenu in jsdom: a ContextMenu keydown on a row, then a second contextmenu at the same row, then a click carrying the first menu's token. Result: head: tokens differ | acted=[]. The second event mints a new token, and the click from the first menu is dropped. Whether WebView2 and WebKitGTK actually send that second event after the hook's own one can't be reproduced from macOS or in jsdom. The PR description says Windows and Linux row menus were not tried by hand.

With a one-line change, the click works whichever menu it comes from (fixed: tokens same | acted=[reveal:obj:7:0]): when the row being re-armed is the node already pending, reuse its token.

const prev = pending.current;
const token = prev && prev.nodeId === hit.nodeId ? prev.token : String(++nextRowMenuToken);

That stops the dropped action, not the second menu. Stopping the second menu needs anovik's platform check and someone to try it on Windows and Linux.

A2 [should-fix] main.go:988 - Linux splash wait can pass the 30s timeout

PoC-confirmed (Appendix P3). I copied the Linux mainWindowReady closure and the WindowLoadFinished handler verbatim into a test, against the real Scheduler and the package's fake clock. The only change is time.AfterFunc becoming clock.AfterFunc. Outcome per variant (mainShown | timeout | forceQuit):

  • main ready at 1s, splash never loads: base, head and fixed all show the main window.
  • main ready at 28.5s, splash never loads: base shows it; head mainShown=false timeout=true forceQuit=true; fixed shows it.
  • main ready at 29s, splash loads at 31s: base shows it; head quits; fixed shows it.
  • main ready at 29s, splash loads at 29.5s: all show it.

So a Linux launch whose main WebView becomes ready in the last 2s before the timeout, with the splash page not yet loaded, is quit 60s later even though it loaded. That window is narrow. If WindowLoadFinished turned out not to fire for a Hidden: true window on some WebKitGTK, every launch would take the 2s path, and the window would still be the last 2s before the timeout. Fix tested in the PoC: cap the wait to the time left before the timeout.

splashStart := time.Now() // next to NewScheduler
...
wait := min(2*time.Second, time.Until(splashStart.Add(30*time.Second-100*time.Millisecond)))
if wait <= 0 {
	splashScheduler.MainWindowReady()
	return
}
time.AfterFunc(wait, splashScheduler.MainWindowReady)

(30s is splashTimeoutMs. Exporting it, or a Scheduler.Deadline(), would keep the two from drifting apart.)

A3 [should-fix] internal/pdfservice/service.go:698 - drive-relative and stream names pass through

PoC-confirmed for the name (Appendix P4); the Windows dialog's handling is not verified. I called embeddedSaveName directly. ../../evil.sh, ..\..\evil.bat and C:\Windows\evil.bat come out as bare file names. C:evil.bat, D: and invoice.pdf:payload (an NTFS alternate data stream) come out unchanged. The function's doc says the dialog never starts from a name that points into another directory, and the name comes from the PDF. What the Windows save dialog then does with C:evil.bat (anovik: resolve it against drive C's current directory) I could not observe from macOS. The suggested fix below passes all seven cases. It replaces : instead of dropping everything before it, so a macOS name like Notes 10:30.txt keeps its full text:

if i := strings.LastIndexAny(name, `/\`); i >= 0 {
	name = name[i+1:]
}
name = strings.ReplaceAll(name, ":", "_")

A row for C:evil.bat in TestSaveEmbeddedFileSeedsTheDialogWithTheLastPathElement would keep it fixed.

A4 [should-fix] main.go:617 - About does not bring the main window forward

Confirmed by source read. The removed openAboutWindow called Restore() and Focus(). The new handlers only emit about:open, which the main window's frontend receives whether or not the window is visible. Not run.

A5 [nit] internal/recentfiles/recentfiles.go:146 - a second atomic-write implementation

Confirmed by source read. List.Save has neither the rename retry nor the removeStaleTemps that updatecheck.Cache.write has. Neither Save nor Cache.write fsyncs, so the word "fsync" in anovik's main.go:824 comment does not apply. The file is small, and losing one update writes the list again on the next open. Nit.

A6 [should-fix] internal/recentfiles/recentfiles.go:64 - one file listed twice under different case

PoC-confirmed (Appendix P5) on a default case-insensitive APFS volume, using the real List. Add(".../Docs/a.pdf") then Add(".../docs/A.pdf"), with os.SameFile true for the two, gives entries=2 labels=["A.pdf" "a.pdf"], and Remove of one leaves the other. The base names differ in case, so Labels does not add the parent folder either. Fixed (strings.EqualFold in without when GOOS is darwin or windows): entries=1. On macOS that would merge two distinct files only on a case-sensitive volume, which is rare.

A7 [should-fix] main.go:824 - one save and one menu-bar rebuild per opened file

Confirmed by source read. RecordRecentFile runs inside each per-file goroutine of openFileAndEmitWithWarning, so a batch of N fires N Save calls and N rebuilds, serialised only by recentMu. The Wails v3.0.0 source shows each rebuild redoes the whole menu bar on every platform. On macOS, Menu.Update clears and rebuilds the NSMenu in InvokeSync. On Windows, setMenu builds a new HMENU and frees the old one. On Linux, setMenu runs menu.Update(), which does g_menu_remove_all and re-adds every item. Not measured.

A8 [nit] frontend/src/App.jsx:146 - first state is never sent

Confirmed; demoted to nit on reachability. The PR's own App.menuState.test.tsx case "nothing is reported while no document is open" asserts exactly the behaviour anovik describes: a fresh mount sends nothing, so after a reload the Go menu keeps its last state. The fix (useRef(null)) would flip that test. In a production build, though, both reload keys are taken: App.jsx preventDefaults F5 and Cmd/Ctrl+R on keydown wherever focus is. That leaves dev reloads and any other WebView reload path.

A9 [nit] frontend/src/components/TreePanel.tsx:369 - second lookup by backendId

Demoted by source read. The only same-tab write of treeDataRef that drops nodes is the effect keyed on rootNode, rootChildren and activeTabId. A reload gives the tab a new id: reloadDocument.ts dispatches the reopened document under result.tabId, and OPEN_DOCUMENT replaces the tab with that filePath and makes the new id active. A background reload leaves the active tab alone. The closeMenu effect on activeTabId therefore clears pending before any click. A copy that does nothing after a reload would come from that deliberate forget, and carrying objectRef in pending would not change it. What remains is the simplification: carry objectRef and skip the second walk.

A10 [nit] frontend/src/components/TreePanel.tsx:38 - doc comment does not match the palette

Confirmed by source read. CommandPalette.tsx:111 shows path.slice(0, -1).join(' > ') (target dropped), while Copy path joins the whole path. The CHANGELOG line "the same breadcrumb the Cmd/Ctrl+K palette shows" is off for the same reason.


PR description verification

  • "Go module v3.0.0, @wailsio/runtime exact 3.0.0, Go 1.27.3, vite ^8.2.4" - confirmed (go.mod, frontend/package.json). The GO-2026-6809 reference was not verified; I did not check the vulnerability database.
  • "Wails checks each emit against the type the event was registered with and silently cancels a mismatch" - confirmed: validateCustomEvent in pkg/application/events.go cancels the event. The error does go to the app's error handler, so it is silent in the UI rather than in the log. Every Go emit site passes the exact registered type.
  • "the bytes ... overflowed the 64 MiB request cap" - confirmed: maxAssembledBytes = 64 * 1024 * 1024 in transport_http.go. 50 MiB as base64 is about 69.9 MB.
  • "all 22 app events are registered in init()" - partially confirmed: events.go registers 23 (row-menu and about included), plus the update service's one. The binding generator reports 24.
  • "events_registration_test.go pins each event to the exact type its emitters use, so a wrong registration fails the test" - partially confirmed: it compares registrations with a hand-written table, not with the emit sites. If an emit site's type changes and the table does not, the test still passes and the event is cancelled at runtime.
  • "The document:warning listener in App.jsx had no emitter" - confirmed: at 077196f the name occurs only in that listener and in a test's string list.
  • "test-setup.ts stubs the generated eventcreate.js" - confirmed.
  • "Read errors return before any dialog is shown, and cancel is silent ... ../../evil.sh suggests evil.sh" - confirmed. Drive-relative names are not covered (A3). "mocks in 25 test files": 26 by my count.
  • Open Recent: "up to 10 paths ... saved atomically", "missing or damaged file starts an empty list", "Reload and Compare never record", "& / _ doubled" - confirmed by source. Wails v3.0.0 does no mnemonic escaping of its own, so the doubling is needed. Case variants are not merged (A6).
  • "The Wails runtime opens a menu from the --custom-contextmenu CSS property" - confirmed (@wailsio/runtime 3.0.0 dist/contextmenu.js). "Go never reads anything from the page-supplied data" - confirmed: the token is only echoed back. "On Windows, Wails opens every context menu at the mouse pointer" - not verified.
  • "Section headers in the File and app menus (macOS 14+)" - confirmed: AddSectionHeader is a no-op elsewhere.
  • "build/windows/info.json keys the version block under 0409 to match the v3.0.0 template" - confirmed against internal/commands/updatable_build_assets/windows/info.json.tmpl.
  • "CI ... installs it, checks aa-status, asserts the runner still has the restriction on, and runs the boot smoke against the installed binary" - confirmed from the ubuntu-24.04 job log: Setting up unidoc-pdf-debugger (0.0.0~ci), and the app process shows label="unidoc-pdf-debugger (unconfined)", so the packaged profile is attached. The boot smoke passed.
  • "wails3 doctor reports no issues: ... gtk4 4.14.5, webkitgtk-6.0 2.52.6" - confirmed from the same log.
  • "CI green on all three runners" - confirmed (gh pr checks).
  • "Ubuntu 22.04, Debian 12 and RHEL 9 no longer run the app" - partially confirmed: true of the binary, but the .deb still installs on Debian 12 and on Ubuntu 22.04 with jammy-updates (F1).
  • "[the splash] starts hidden, and shows on WindowLoadFinished; the main window waits for that (2s at most)" - not verified. Nothing here shows that WindowLoadFinished fires for a Hidden: true window, and the Linux boot smoke would pass on the 2s fallback alone. The timeout interaction is A2.
  • "main.go sets Linux.ApplicationID ... desktop entry gets a matching StartupWMClass" - confirmed. Wails also uses that id as the GTK program name, which sets the Wayland app_id (application_linux_appid.go). The dock icon effect is not verified.
  • "The update checker still offers Linux users the tarball, since it matches the -linux-amd64.tar.gz suffix" - confirmed: assetFor uses HasSuffix, so the .deb and .rpm can never be picked.
  • "On Windows, Ctrl+R with no document open now reaches WebView2 ... If WebView2 reloads the page" - not verified, and probably moot: App.jsx preventDefaults Cmd/Ctrl+R and F5 on keydown in every state.
  • "go mod tidy -diff is clean" - confirmed (re-run).
  • "go test -race -count=1 on the root package, internal/recentfiles, internal/pdfservice and internal/updateservice passed" - confirmed on macOS arm64 (the root package needs a built frontend/dist). "All 75 suites passed" was not re-run in full.

Appendix: PoC code

Each PoC is given in full, with the command to run it and its output. They were not committed to the PR branch.

P1 - F1: the .deb installs on Debian 12 and the binary fails there

Three files in one directory (poc-deb/). Run from that directory:

D=$PWD
docker run --rm --platform linux/amd64 -v "$D:/poc" ubuntu:24.04 /poc/build.sh
for v in head fixed; do docker run --rm --platform linux/amd64 -v "$D:/poc" debian:bookworm-slim /poc/try.sh $v; done
docker run --rm --platform linux/amd64 -v "$D:/poc" ubuntu:24.04 /poc/try.sh head

src/main.go:

// PoC for PR 40 F1: a cgo binary that, like the Wails v3.0.0 GTK4 backend
// (pkg/application/linux_cgo.go), calls the GTK 4.10 GtkFileDialog API.
package main

// #cgo pkg-config: gtk4
// #include <gtk/gtk.h>
// #include <stdio.h>
// static void* newDialog(void) { return gtk_file_dialog_new(); }
// static const char* ver(void) { static char b[32]; snprintf(b, sizeof b, "%u.%u.%u", gtk_get_major_version(), gtk_get_minor_version(), gtk_get_micro_version()); return b; }
import "C"

import (
	"fmt"
	"os"
)

func main() {
	fmt.Println("started; runtime gtk", C.GoString(C.ver()))
	if len(os.Args) > 1 {
		C.newDialog()
		fmt.Println("file dialog created")
	}
}

build.sh:

#!/bin/sh
# Stage 1, runs in ubuntu:24.04 (the release build host): build the PoC binary
# and pack it twice - with the PR's nfpm depends verbatim (head) and with
# versioned depends (fixed). Writes only into /poc (the bind-mounted scratch dir).
set -eu
export DEBIAN_FRONTEND=noninteractive
apt-get update -qq >/dev/null
apt-get install -y -qq --no-install-recommends golang-go gcc libc6-dev libgtk-4-dev pkg-config dpkg-dev binutils >/dev/null
cd /poc/src
[ -f go.mod ] || go mod init poc >/dev/null 2>&1
GOFLAGS=-buildvcs=false go build -o /poc/poc .
echo "build host: $(ldd --version | head -1); gtk $(pkg-config --modversion gtk4)"
echo "highest GLIBC symbol version the binary needs: $(objdump -T /poc/poc | grep -o 'GLIBC_[0-9.]*' | sort -uV | tail -1)"
echo "gtk_file_dialog dynamic refs: $(objdump -T /poc/poc | grep -c gtk_file_dialog)"
echo "BIND_NOW flags: $(readelf -d /poc/poc | grep -E 'FLAGS' | tr -s ' ' | tr '\n' ' ')"
arch=$(dpkg --print-architecture)
mk() {
  name=$1 depends=$2
  root=/poc/pkg-$name
  mkdir -p "$root/DEBIAN" "$root/usr/bin"
  cp /poc/poc "$root/usr/bin/poc-gtkdep"
  printf 'Package: poc-gtkdep\nVersion: 1.0\nArchitecture: %s\nMaintainer: poc <poc@example.invalid>\nDepends: %s\nDescription: PR 40 depends PoC\n' "$arch" "$depends" > "$root/DEBIAN/control"
  dpkg-deb --build --root-owner-group "$root" "/poc/$name.deb" >/dev/null
  echo "$name.deb Depends: $(dpkg-deb -f /poc/$name.deb Depends)"
}
# build/linux/nfpm/nfpm.yaml at 35c1d4c: depends: [libgtk-4-1, libwebkitgtk-6.0-4]
mk head 'libgtk-4-1, libwebkitgtk-6.0-4'
mk fixed 'libc6 (>= 2.39), libgtk-4-1 (>= 4.14), libwebkitgtk-6.0-4'

try.sh:

#!/bin/sh
# Stage 2: install a PoC .deb with apt on the target image and run the binary.
# $1 = head|fixed
set -u
export DEBIAN_FRONTEND=noninteractive
apt-get update -qq >/dev/null
. /etc/os-release
if apt-get install -y -qq --no-install-recommends "/poc/$1.deb" >/tmp/apt.log 2>&1; then
  echo "[$PRETTY_NAME] $1.deb: apt install OK (libgtk-4-1 $(dpkg-query -W -f '${Version}' libgtk-4-1), libc6 $(dpkg-query -W -f '${Version}' libc6))"
  out=$(poc-gtkdep 2>&1); echo "  start:  rc=$? $out"
  out=$(poc-gtkdep dialog 2>&1); echo "  dialog: rc=$? $out"
else
  echo "[$PRETTY_NAME] $1.deb: apt install REFUSED"
  grep -E "Depends|unmet|^E:" /tmp/apt.log | head -4 | sed 's/^/  /'
fi

Output (Docker's platform-mismatch warnings and debconf noise removed):

build host: ldd (Ubuntu GLIBC 2.39-0ubuntu8.9) 2.39; gtk 4.14.5
highest GLIBC symbol version the binary needs: GLIBC_2.34
gtk_file_dialog dynamic refs: 1
BIND_NOW flags: 
head.deb Depends: libgtk-4-1, libwebkitgtk-6.0-4
fixed.deb Depends: libc6 (>= 2.39), libgtk-4-1 (>= 4.14), libwebkitgtk-6.0-4
[Debian GNU/Linux 12 (bookworm)] head.deb: apt install OK (libgtk-4-1 4.8.3+ds-2+deb12u1, libc6 2.36-9+deb12u14)
  start:  rc=0 started; runtime gtk 4.8.3
  dialog: rc=127 started; runtime gtk 4.8.3
poc-gtkdep: symbol lookup error: poc-gtkdep: undefined symbol: gtk_file_dialog_new
[Debian GNU/Linux 12 (bookworm)] fixed.deb: apt install REFUSED
  E: Unable to correct problems, you have held broken packages.
[Ubuntu 24.04.5 LTS] head.deb: apt install OK (libgtk-4-1 4.14.5+ds-0ubuntu0.10, libc6 2.39-0ubuntu8.9)
  start:  rc=0 started; runtime gtk 4.14.5
  dialog: rc=0 started; runtime gtk 4.14.5
file dialog created

P2 - A1: a second contextmenu at the same row drops the first menu's action

Two files in frontend/src/components/. poc_pr40_navigatorHooks.fixed.ts is a copy of navigatorHooks.ts with this one change:

--- navigatorHooks.ts
+++ poc_pr40_navigatorHooks.fixed.ts
@@ -146,7 +146,10 @@
     const hit = id ? resolveRef.current(id) : null;
     if (!id || !hit) return;
     treeRef.current?.select(id);
-    const token = String(++nextRowMenuToken);
+    // Re-arming the node already pending keeps its token, so a second contextmenu
+    // for one key press does not orphan the first menu.
+    const prev = pending.current;
+    const token = prev && prev.nodeId === hit.nodeId ? prev.token : String(++nextRowMenuToken);
     pending.current = { token, nodeId: hit.nodeId };
     container.style.setProperty('--custom-contextmenu', hit.menu);
     container.style.setProperty('--custom-contextmenu-data', token);

poc_pr40_rowmenu_double.test.tsx:

/**
 * PoC for PR 40 (anovik's navigatorHooks.ts:162 comment): if one ContextMenu
 * key press yields two contextmenu events at the same row (the hook's own
 * synthetic one on keydown, then the WebView's), the second arms a new token
 * and a click in the first menu is dropped.
 *
 * head  = the real useRowContextMenu at 35c1d4c (navigatorHooks.ts).
 * fixed = poc_pr40_navigatorHooks.fixed.ts: the same file with one change,
 *         re-arming the node already pending keeps its token.
 * base  = 077196f drew its own React menu and had no tokens; a second event
 *         reopened the same menu (source-read, not run here).
 *
 * Not modelled: whether the WebView really sends that second event. That is
 * platform behaviour (WebView2 on keyup, WebKitGTK via popup-menu) and is not
 * reproducible in jsdom; the second event is fired by hand.
 *
 * Run: cd frontend && npx vitest run src/components/poc_pr40_rowmenu_double.test.tsx
 */
import { useRef } from 'react';
import { render, screen, fireEvent } from '@testing-library/react';
import { describe, test, expect } from 'vitest';
import { useRowContextMenu as headHook } from './navigatorHooks';
import { useRowContextMenu as fixedHook } from './poc_pr40_navigatorHooks.fixed';
import { armedRowMenu, clickRowMenuItem } from './rowMenuTestUtils';

type Hook = typeof headHook;

function Panel({ hook, acted }: { hook: Hook; acted: string[] }) {
  const treeRef = useRef(undefined);
  const { onContextMenu, onKeyDown } = hook(
    treeRef,
    '[data-row-id]',
    'data-row-id',
    (rowId) => ({ menu: 'row-reveal', nodeId: `obj:${rowId}:0` }),
    (action, nodeId) => acted.push(`${action}:${nodeId}`),
  );
  return (
    <div onContextMenu={onContextMenu} onKeyDown={onKeyDown}>
      <div role="treeitem" tabIndex={0} data-testid="item">
        <span data-row-id="7">row 7</span>
      </div>
    </div>
  );
}

function run(hook: Hook): string {
  const acted: string[] = [];
  const { unmount } = render(<Panel hook={hook} acted={acted} />);
  const item = screen.getByTestId('item');
  const row = screen.getByText('row 7');
  // One ContextMenu key press: the hook dispatches its own contextmenu ...
  fireEvent.keyDown(item, { key: 'ContextMenu' });
  const first = armedRowMenu(row)?.token ?? '';
  // ... and the WebView sends one too.
  fireEvent.contextMenu(row);
  const second = armedRowMenu(row)?.token ?? '';
  // The user picks the item in the menu that opened first.
  clickRowMenuItem('reveal', first);
  unmount();
  return `tokens ${first === second ? 'same' : 'differ'} | acted=[${acted.join(',')}]`;
}

describe('ContextMenu key with a second native contextmenu', () => {
  test('head drops the first menu action, fixed keeps it', () => {
    const got = `head: ${run(headHook)} || fixed: ${run(fixedHook)}`;
    console.log(got);
    expect(got).toBe('head: tokens same | acted=[reveal:obj:7:0] || fixed: tokens same | acted=[reveal:obj:7:0]');
  });
});

Output (the assertion states the correct behaviour, so head failing is the reproduction):

head: tokens differ | acted=[] || fixed: tokens same | acted=[reveal:obj:7:0]

P3 - A2: Linux splash wait past the 30s timeout

internal/splash/poc_pr40_linux_ready_test.go (uses the package's fakeClock from splash_test.go):

package splash

// PoC for PR 40 (anovik's main.go:988 comment): the Linux splash wiring can
// push MainWindowReady past the 30s timeout, after which the main window is
// never shown and the 60s force-quit closes an app that loaded.
//
// The Linux closure and the WindowLoadFinished handler are transplanted
// verbatim from main.go lines 1062-1082 at 35c1d4c, with one substitution:
// time.AfterFunc becomes clock.AfterFunc so the fake clock drives it. The
// Scheduler is the real one (NewScheduler's 400ms floor and 30s timeout).
//
//   base  = 077196f: WindowRuntimeReady calls MainWindowReady directly.
//   head  = 35c1d4c: wait up to 2s for the splash's WindowLoadFinished.
//   fixed = head, with the 2s wait capped to the time left before the timeout.
//
// Run: go test ./internal/splash -run TestPoCPR40 -v

import (
	"fmt"
	"sync/atomic"
	"testing"
	"time"
)

type pocWiring struct {
	mainWindowReady    func()
	windowLoadFinished func()
}

func pocBase(s *Scheduler, _ *fakeClock, _ time.Time) pocWiring {
	return pocWiring{mainWindowReady: s.MainWindowReady, windowLoadFinished: func() {}}
}

func pocHead(splashScheduler *Scheduler, clock *fakeClock, _ time.Time) pocWiring {
	var splashLoaded, mainReady atomic.Bool
	return pocWiring{
		windowLoadFinished: func() {
			if splashScheduler.Shown() {
				// splashWindow.Show()
			}
			splashLoaded.Store(true)
			if mainReady.Load() {
				splashScheduler.MainWindowReady()
			}
		},
		mainWindowReady: func() {
			mainReady.Store(true)
			if splashLoaded.Load() {
				splashScheduler.MainWindowReady()
				return
			}
			clock.AfterFunc(2*time.Second, splashScheduler.MainWindowReady)
		},
	}
}

func pocFixed(splashScheduler *Scheduler, clock *fakeClock, created time.Time) pocWiring {
	var w pocWiring
	var splashLoaded, mainReady atomic.Bool
	w.windowLoadFinished = func() {
		splashScheduler.Shown()
		splashLoaded.Store(true)
		if mainReady.Load() {
			splashScheduler.MainWindowReady()
		}
	}
	w.mainWindowReady = func() {
		mainReady.Store(true)
		if splashLoaded.Load() {
			splashScheduler.MainWindowReady()
			return
		}
		// Never wait past the splash timeout: leave 100ms of the 30s budget.
		wait := min(2*time.Second, created.Add(splashTimeoutMs*time.Millisecond-100*time.Millisecond).Sub(clock.Now()))
		if wait <= 0 {
			splashScheduler.MainWindowReady()
			return
		}
		clock.AfterFunc(wait, splashScheduler.MainWindowReady)
	}
	return w
}

// run drives one launch: the main WebView reports ready at mainAt, the splash
// page finishes loading at splashAt (negative: never). It returns the joined
// outcome after 95s of virtual time, so the 60s force-quit would have fired.
func run(variant func(*Scheduler, *fakeClock, time.Time) pocWiring, mainAt, splashAt time.Duration) string {
	start := time.Unix(0, 0)
	clock := newFakeClock(start)
	var dismissed, timedOut, forceQuit atomic.Bool
	var splashFailed atomic.Bool
	s := NewScheduler(clock, func() {
		dismissed.Store(true) // onSplashDismiss: mainWindow.Show()
	}, func() {
		splashFailed.Store(true)
		timedOut.Store(true)
		clock.AfterFunc(60*time.Second, func() {
			if splashFailed.Load() {
				forceQuit.Store(true) // app.Quit()
			}
		})
	})
	w := variant(s, clock, start)
	type ev struct {
		at time.Duration
		fn func()
	}
	evs := []ev{{mainAt, w.mainWindowReady}}
	if splashAt >= 0 {
		evs = append(evs, ev{splashAt, w.windowLoadFinished})
	}
	if len(evs) == 2 && evs[1].at < evs[0].at {
		evs[0], evs[1] = evs[1], evs[0]
	}
	// fakeClock.Advance fires every due timer in registration order and skips
	// timers registered during the same call, so time moves in 50ms steps.
	var now time.Duration
	step := func(to time.Duration) {
		for now < to {
			d := min(50*time.Millisecond, to-now)
			clock.Advance(d)
			now += d
		}
	}
	for _, e := range evs {
		step(e.at)
		e.fn()
	}
	step(95 * time.Second)
	return fmt.Sprintf("mainShown=%v timeout=%v forceQuit=%v", dismissed.Load(), timedOut.Load(), forceQuit.Load())
}

func TestPoCPR40LinuxSplashReady(t *testing.T) {
	ok := "mainShown=true timeout=false forceQuit=false"
	bad := "mainShown=false timeout=true forceQuit=true"
	cases := []struct {
		name               string
		mainAt, splashAt   time.Duration
		wantBase, wantHead string
	}{
		{"fast launch, splash loads after main", 1 * time.Second, 1200 * time.Millisecond, ok, ok},
		{"fast launch, splash never loads", 1 * time.Second, -1, ok, ok},
		{"slow launch, main ready 27.9s, splash never loads", 27900 * time.Millisecond, -1, ok, ok},
		{"slow launch, main ready 28.5s, splash never loads", 28500 * time.Millisecond, -1, ok, bad},
		{"slow launch, main ready 29s, splash loads at 31s", 29 * time.Second, 31 * time.Second, ok, bad},
		{"slow launch, main ready 29s, splash loads at 29.5s", 29 * time.Second, 29500 * time.Millisecond, ok, ok},
	}
	for _, c := range cases {
		for _, v := range []struct {
			name string
			fn   func(*Scheduler, *fakeClock, time.Time) pocWiring
			want string
		}{{"base", pocBase, c.wantBase}, {"head", pocHead, c.wantHead}, {"fixed", pocFixed, ok}} {
			got := v.name + " | " + c.name + " | " + run(v.fn, c.mainAt, c.splashAt)
			want := v.name + " | " + c.name + " | " + v.want
			t.Log(got)
			if got != want {
				t.Errorf("\n got  %s\n want %s", got, want)
			}
		}
	}
}

Output:

base | fast launch, splash loads after main | mainShown=true timeout=false forceQuit=false
head | fast launch, splash loads after main | mainShown=true timeout=false forceQuit=false
fixed | fast launch, splash loads after main | mainShown=true timeout=false forceQuit=false
base | fast launch, splash never loads | mainShown=true timeout=false forceQuit=false
head | fast launch, splash never loads | mainShown=true timeout=false forceQuit=false
fixed | fast launch, splash never loads | mainShown=true timeout=false forceQuit=false
base | slow launch, main ready 27.9s, splash never loads | mainShown=true timeout=false forceQuit=false
head | slow launch, main ready 27.9s, splash never loads | mainShown=true timeout=false forceQuit=false
fixed | slow launch, main ready 27.9s, splash never loads | mainShown=true timeout=false forceQuit=false
base | slow launch, main ready 28.5s, splash never loads | mainShown=true timeout=false forceQuit=false
head | slow launch, main ready 28.5s, splash never loads | mainShown=false timeout=true forceQuit=true
fixed | slow launch, main ready 28.5s, splash never loads | mainShown=true timeout=false forceQuit=false
base | slow launch, main ready 29s, splash loads at 31s | mainShown=true timeout=false forceQuit=false
head | slow launch, main ready 29s, splash loads at 31s | mainShown=false timeout=true forceQuit=true
fixed | slow launch, main ready 29s, splash loads at 31s | mainShown=true timeout=false forceQuit=false
base | slow launch, main ready 29s, splash loads at 29.5s | mainShown=true timeout=false forceQuit=false
head | slow launch, main ready 29s, splash loads at 29.5s | mainShown=true timeout=false forceQuit=false
fixed | slow launch, main ready 29s, splash loads at 29.5s | mainShown=true timeout=false forceQuit=false

P4 - A3: drive-relative and stream names pass through embeddedSaveName

internal/pdfservice/poc_pr40_embedded_name_test.go:

package pdfservice

// PoC for PR 40 (anovik's service.go:698 comment): does the suggested save
// name still point outside the folder the dialog shows?
//
// head  = embeddedSaveName at 35c1d4c (internal/pdfservice/service.go), called directly.
// base  = 077196f: the frontend passed selected.name to SaveBytesToFile verbatim.
// fixed = head plus ':' replaced, so no drive prefix or NTFS stream survives.
//
// Run: go test ./internal/pdfservice -run TestPoCPR40EmbeddedSaveName -v

import (
	"fmt"
	"strings"
	"testing"
)

func pocBaseSaveName(name string) string { return name }

func pocFixedSaveName(name string) string {
	if i := strings.LastIndexAny(name, `/\`); i >= 0 {
		name = name[i+1:]
	}
	name = strings.ReplaceAll(name, ":", "_")
	name = strings.TrimSpace(name)
	if name == "" || name == "." || name == ".." {
		return "embedded-file"
	}
	return name
}

// escapes reports whether a suggested name can resolve outside the folder the
// Windows save dialog shows: it carries a separator or a drive/stream colon.
func escapes(name string) bool { return strings.ContainsAny(name, `/\:`) }

func TestPoCPR40EmbeddedSaveName(t *testing.T) {
	cases := []struct{ in, want string }{
		{"report.pdf", "report.pdf"},
		{"../../evil.sh", "evil.sh"},
		{`..\..\evil.bat`, "evil.bat"},
		{`C:\Windows\evil.bat`, "evil.bat"},
		{"C:evil.bat", "C_evil.bat"},
		{"D:", "D_"},
		{"invoice.pdf:payload", "invoice.pdf_payload"},
	}
	for _, c := range cases {
		for _, v := range []struct {
			name string
			fn   func(string) string
		}{{"base", pocBaseSaveName}, {"head", embeddedSaveName}, {"fixed", pocFixedSaveName}} {
			out := v.fn(c.in)
			got := fmt.Sprintf("%s | %q -> %q | escapes=%v", v.name, c.in, out, escapes(out))
			t.Log(got)
			if v.name == "fixed" {
				if want := fmt.Sprintf("fixed | %q -> %q | escapes=false", c.in, c.want); got != want {
					t.Errorf("got  %s\nwant %s", got, want)
				}
			}
		}
	}
}

Output:

base | "report.pdf" -> "report.pdf" | escapes=false
head | "report.pdf" -> "report.pdf" | escapes=false
fixed | "report.pdf" -> "report.pdf" | escapes=false
base | "../../evil.sh" -> "../../evil.sh" | escapes=true
head | "../../evil.sh" -> "evil.sh" | escapes=false
fixed | "../../evil.sh" -> "evil.sh" | escapes=false
base | "..\\..\\evil.bat" -> "..\\..\\evil.bat" | escapes=true
head | "..\\..\\evil.bat" -> "evil.bat" | escapes=false
fixed | "..\\..\\evil.bat" -> "evil.bat" | escapes=false
base | "C:\\Windows\\evil.bat" -> "C:\\Windows\\evil.bat" | escapes=true
head | "C:\\Windows\\evil.bat" -> "evil.bat" | escapes=false
fixed | "C:\\Windows\\evil.bat" -> "evil.bat" | escapes=false
base | "C:evil.bat" -> "C:evil.bat" | escapes=true
head | "C:evil.bat" -> "C:evil.bat" | escapes=true
fixed | "C:evil.bat" -> "C_evil.bat" | escapes=false
base | "D:" -> "D:" | escapes=true
head | "D:" -> "D:" | escapes=true
fixed | "D:" -> "D_" | escapes=false
base | "invoice.pdf:payload" -> "invoice.pdf:payload" | escapes=true
head | "invoice.pdf:payload" -> "invoice.pdf:payload" | escapes=true
fixed | "invoice.pdf:payload" -> "invoice.pdf_payload" | escapes=false

P5 - A6: one file listed twice under different case

internal/recentfiles/poc_pr40_case_test.go (run on a case-insensitive volume):

package recentfiles

// PoC for PR 40 (anovik's recentfiles.go:64 comment): on a case-insensitive
// file system one file can be listed twice in Open Recent.
//
//   head  = the real List at 35c1d4c (Add/Remove/Labels).
//   fixed = Add with entries compared by strings.EqualFold on darwin/windows.
//   base  = 077196f had no Open Recent; not applicable.
//
// Run (on a default, case-insensitive APFS or NTFS volume):
//   go test ./internal/recentfiles -run TestPoCPR40 -v

import (
	"fmt"
	"os"
	"path/filepath"
	"runtime"
	"strings"
	"testing"
)

func pocFixedAdd(paths []string, path string) []string {
	p := normalize(path)
	same := func(a, b string) bool {
		if runtime.GOOS == "darwin" || runtime.GOOS == "windows" {
			return strings.EqualFold(a, b)
		}
		return a == b
	}
	out := []string{p}
	for _, q := range paths {
		if !same(q, p) {
			out = append(out, q)
		}
	}
	return out
}

func TestPoCPR40CaseVariants(t *testing.T) {
	dir := t.TempDir()
	if err := os.MkdirAll(filepath.Join(dir, "Docs"), 0o755); err != nil {
		t.Fatal(err)
	}
	a := filepath.Join(dir, "Docs", "a.pdf")
	b := filepath.Join(dir, "docs", "A.pdf")
	if err := os.WriteFile(a, []byte("%PDF"), 0o644); err != nil {
		t.Fatal(err)
	}
	ia, errA := os.Stat(a)
	ib, errB := os.Stat(b)
	if errA != nil || errB != nil || !os.SameFile(ia, ib) {
		t.Skipf("volume is case-sensitive here (%v, %v); the PoC needs a case-insensitive one", errA, errB)
	}

	l := Load(filepath.Join(dir, "recent.json"))
	l.Add(a)
	l.Add(b)
	head := l.Paths()
	l.Remove(a)
	afterRemove := l.Paths()

	fixed := pocFixedAdd(pocFixedAdd(nil, a), b)

	got := fmt.Sprintf("head entries=%d labels=%q afterRemove=%d | fixed entries=%d",
		len(head), Labels(head), len(afterRemove), len(fixed))
	t.Log(got)
	want := `head entries=1 labels=["A.pdf"] afterRemove=0 | fixed entries=1`
	if got != want {
		t.Errorf("\n got  %s\n want %s", got, want)
	}
}

Output (head fails the correct-behaviour assertion):

got  head entries=2 labels=["A.pdf" "a.pdf"] afterRemove=1 | fixed entries=1
want head entries=1 labels=["A.pdf"] afterRemove=0 | fixed entries=1

@unidoc-anom
unidoc-anom requested a review from anovik November 3, 2026 15:13

@anovik anovik 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.

Thanks for the second round. Re-review at 3288082: all 6 findings from the last round are fixed or mostly fixed. The splash race now has a readyPending flag and a test, About waits until the frontend has mounted, a missing file no longer matches a case variant and the Stat runs outside the lock, macOS and pointer presses no longer leave the key-menu flag set, and format characters are mapped in the save name. Nothing blocking is left. Inline comments cover a few smaller issues, mostly in the new fixes. The shutdown race in the Open Recent flush matters most.

Comment thread main.go Resolved
Comment thread main.go Resolved
Comment thread internal/pdfservice/service.go Resolved
Comment thread internal/recentfiles/recentfiles.go Resolved
@unidoc-anom
unidoc-anom requested a review from anovik November 3, 2026 16:19

@anovik anovik 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.

Re-review at 0e1e7ab, covering the third round of fixes.

Fixed

  • Save-name rule: only Bidi_Control, Zl and Zp are mapped now, and tests check that ZWNJ, ZWJ and the soft hyphen are kept.
  • Key-menu echo guard: it is set only for the ContextMenu key.
  • Case-variant race in Open Recent: lockSameAs rechecks under l.mu. I checked the lock order (saveMu → mu, no InvokeSync under the flusher lock) and found no deadlock.

Partly fixed

  • Shutdown: recentFlusher.Close now stops the timer, waits for a save in progress and saves a pending change. Two gaps are left: a rebuild that started just before Close still runs during teardown, and Clear Menu and the missing-file removal bypass the flusher (inline).
  • Batch coalescing: openFilesBatch holds the flusher again, and the frontend loops record once at the end. But the OnSecondInstanceLaunch loop (main.go:462, not in the diff so not inline) opens each path through openFileAndEmit without Hold/Release. pdfdebug big1.pdf big2.pdf big3.pdf against a running instance, with each file taking longer than 500 ms, still saves and rebuilds once per file. The CHANGELOG sentence "Opening several files at once saves the list and rebuilds the menu once ... however long each takes" names "a second launch" as one of the routes. Wrapping that loop in Hold/Release when there is more than one path would make it true.

The new inline comments are mostly about the frontend recording now happening after the loop. The ordering and lost-on-quit ones matter most.

Comment thread frontend/src/App.jsx Resolved
Comment thread frontend/src/components/EmptyState.tsx Resolved
Comment thread recentflush.go Resolved
Comment thread main.go Resolved
Comment thread main.go Resolved
@unidoc-anom
unidoc-anom requested a review from anovik November 4, 2026 10:13

@anovik anovik 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.

Re-review at e35de9a, covering the fourth round of fixes.

Fixed

  • Open Recent order: both loops await each recordRecentFile.
  • Files lost when quitting mid-batch: each file is recorded as it opens, and Close saves what is still waiting.
  • Rebuild after quit: the rebuild runs on the main thread and checks isClosed there, and Close runs on the main thread, so a late rebuild does nothing. I found no deadlock.
  • Clear Menu and missing-file removal: both go through SaveNow.
  • The openFilesBatch doc comment is back on its function, and the flusher gets save/rebuild in its constructor.
  • Shutdown (main.go 886): Close stops the timer, waits for a save in progress and saves a pending change.

Partly fixed

  • Batch coalescing (main.go 861/881) and the second launch: a launch carrying several paths is held now, but files that arrive one per process or event are not (inline).

Requesting changes for the startup nil dereference on Linux and the app-wide frontend hold that can stop Open Recent saving for the rest of the session (inline). Several of the new comments come from the frontend Begin/End protocol. Counting opens in flight on the Go side, which every open path goes through, would fix them and make that protocol unnecessary (see the comment on main.go:463).

Comment thread main.go Resolved
Comment thread main.go Resolved
Comment thread recentflush.go Resolved
Comment thread frontend/src/hooks/usePDFService.ts Resolved
Comment thread recentflush_test.go Resolved
@unidoc-anom
unidoc-anom requested a review from anovik November 4, 2026 15:15

@anovik anovik 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.

Re-review at 194bdcb. Counting opens in flight in PDFService.OpenFile works well: all five findings from the last round are fixed, and nothing here blocks the merge.

Fixed

  • Startup nil dereference: the second-launch callback no longer touches the flusher, and OpenFile checks the tracker for nil.
  • One-file-per-process arrivals: every entry point goes through the in-flight count.
  • Stuck frontend hold: the Begin/End/Reset protocol is gone.
  • Progress dialog waiting on the save: nothing is awaited around the open loop now.
  • SaveNow timing: the started/finished counters make it wait for its own save, and that test is deterministic now.
  • Entry-point coverage (main.go 861).

I also checked that every OpenStarted gets exactly one OpenEnded via defer on every path, that the cap never changes inFlight (so a late end can't drive it negative), and that f.mu is never held across the save or InvokeSync. The flusher tests pass with -count=20 locally.

Partly fixed

  • Slow batches (main.go 881): they no longer save once per file, but the 10 s cap still splits a long run into several saves (inline on recentflush.go:147).

The inline comments are about the cap's timing, the CHANGELOG wording, two timing-dependent tests and some smaller points.

Comment thread recentflush.go Resolved
Comment thread recentflush.go Resolved
Comment thread CHANGELOG.md Resolved
Comment thread internal/pdfservice/service.go Resolved
Comment thread internal/pdfservice/service.go Resolved
Comment thread recentflush_test.go Resolved
Comment thread frontend/src/hooks/usePDFService.ts Resolved
@unidoc-anom
unidoc-anom requested a review from anovik November 4, 2026 16:11

@anovik anovik 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.

Re-review at 6958ed2. The per-open cap fixes the cap timing and the stuck-open issue, the earlier timing-dependent tests are fixed, and slow batches now save once (main.go 881). I checked the new tracking: each open gets its own id with exactly one end, defer s.openStarted()() still ends on a recovered panic, arm() picks the newest open's deadline and re-arms when a newer open starts, an end after Close does nothing, and there are no lock-order or InvokeSync problems. I found no regressions in the code.

What's left

  • The new test code is timing-dependent again. One test failed in a load run (inline). This is the one I'd fix before merge, since it will make CI unreliable.
  • CHANGELOG / flusher doc wording: "saves once however long each file takes to open" followed by the 10 s exception still reads as a contradiction. A batch with one file slower than 10 s saves partway through. "however long each file takes, up to 10 seconds" (in recentflush.go:10-11 too) would match. Leaving the existing CHANGELOG thread open for that.
  • Minor: an open's end re-arms the full delay even when it no longer holds the save (inline).

The declined service.go 195/196 threads are fine with me as reasoned. I'll leave them for you to resolve.

Comment thread recentflush_test.go Resolved
Comment thread recentflush_test.go Resolved
Comment thread recentflush.go Resolved
@unidoc-anom
unidoc-anom requested a review from anovik November 5, 2026 15:06
@unidoc-anom
unidoc-anom merged commit 3903c56 into dev Nov 5, 2026
3 checks passed
@unidoc-anom
unidoc-anom deleted the feat/wails-v3-stable-upgrade branch November 5, 2026 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants