Skip to content

Restructure NativeDesktop: HostServices for links, injected instance, trash outside the preferences - #786

Closed
calixtus wants to merge 14 commits into
hostservices-browse-mail-basefrom
hostservices-browse-mail
Closed

calixtus wants to merge 14 commits into
hostservices-browse-mail-basefrom
hostservices-browse-mail

Conversation

@calixtus

@calixtus calixtus commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Refs #764

Browse and mail no longer use AWT java.awt.Desktop. The remaining AWT usages (trash, Linux/default file open, clipboard) are not part of this PR, so the issue stays open.

The base branch hostservices-browse-mail-base is the upstream commit this branch was created from (721cb13316), so the diff only shows the changes of this PR.

Opening links and mail

  • Web links open via JavaFX HostServices.showDocument instead of AWT Desktop.browse.
    • A custom browser configured for HTML still takes priority.
    • URLs that cannot be parsed, or relative URLs, go to the OS URL handler (explorer.exe, open, xdg-open).
  • Mail (SendAsEMailAction) goes through the OS URL handler instead of AWT Desktop.mail.
    • Not via HostServices: on Windows, it starts the web browser instead of the mail client.
    • Failures are reported to the user again; the isDesktopSupported check is gone.
  • Known gap: when opening a normal web link fails, the "Could not open browser" popup no longer appears, because HostServices never reports failures. Custom browsers and the OS URL handler path still show it.

NativeDesktop design

  • No singleton: NativeDesktop.create(HostServices, GuiPreferences) builds the class for the current OS. It is created in JabRefGUI.start and registered in the injector; HostServices and the preferences arrive through the constructor.
  • The open methods (openBrowser, openFolderAndSelectFile, openConsole, openExternalViewer, ...) are instance methods without preference parameters.
  • Template methods in the base class: the preference logic (custom browser, file browser, terminal) lives once in NativeDesktop; each OS implements only protected hooks (open with default/configured application, open folder with default file browser, open default terminal).
    • Fixes a Windows bug: with no application configured, it tried to start an empty program name.
    • macOS uses ProcessBuilder instead of Runtime.exec.
    • Duplicated process output handling in Linux is merged into one helper.
  • DefaultDesktop (unknown OS) falls back to HostServices for URLs instead of throwing.
  • The static helpers (NativeDesktop.openBrowser(...) etc.) are kept for the about 45 callers, delegate to the injected instance and are @Deprecated, pointing to the instance method. Their preference arguments are ignored; all callers pass parts of the same GuiPreferences.
  • SendAsEMailAction uses the instance directly.
  • EditExternalFileTypeEntryDialog gets NativeDesktop injected; LinkedFilesTab, DetectOpenOfficeInstallation and DeleteFileAction look it up. MultiSelectionMenuBuilder now uses the static helper and thus respects the custom file browser.

Trash support

  • Whether the platform supports the trash is no longer part of the preferences: the moveToTrashSupported() hooks in JabRefCliPreferences and JabRefGuiPreferences are removed.
  • FilePreferences defaults "move to trash" to true; DeleteFileAction checks platform support and deletes if there is no trash.
    • Fixes an UnsupportedOperationException when the preference was on but the platform had no trash.

Default directories

  • The preference classes no longer need NativeDesktop or the getDefaultPath() hook. Defaults come from FilePreferences.getDefault() only, for fresh installs and reset alike.
  • The documents folder lookup (getDefaultFileChooserDirectory, JNA on Windows, xdg-user-dir on Linux) is removed. Its only remaining use would have been the lastUsedDirectory default, which is read only by the attach file dialog when "open file explorer in last used directory" is enabled. Native calls in jablib are not worth that.
  • Behaviour changes:
    • lastUsedDirectory defaults to the home directory, like workingDirectory (was /; reset gave Documents).
    • mainFileDirectory stays unset, as on a fresh install on main; reset now also gives unset (on main, it gave Documents).

Null safety

  • @NullMarked on NativeDesktop, Windows, OSX, Linux, DefaultDesktop, SendAsEMailAction and Directories.
  • Dead null checks in openFolderAndSelectFile and openConsole removed.
  • Fixes an NPE: BackupResolverDialog passed null as DialogService, which crashed with a custom file browser configured. BackupUIManager now passes its DialogService.

Tests

  • NativeDesktopTest uses Mockito mocks instead of a hand-written fake (runs on Linux only).
  • PreferencesSearchCoverageTest, BackupResolverDialogTest and BackupUIManagerTest adapted.
  • Local results (Windows):
    • Everything compiles, no NullAway warnings in changed files.
    • Full jablib and jabgui test runs: four failures (BackupManagerTest ×2, SourceTabTest, KeyBindingViewModelTest) also fail on the base commit; RemoteCommunicationTest failed once under load and passes on its own.

Still to do

  • CHANGELOG entries (backup dialog NPE, mail opening the mail client).
  • Formatter, checkstyle, OpenRewrite, modernizer, checkAllModuleInfo, javadoc, localization checks.

🤖 Generated with Claude Code

calixtus and others added 10 commits October 8, 2026 18:29
Opening URLs and the mail client went through java.awt.Desktop. Both now
use HostServices.showDocument, registered in the Injector at JabRefGUI
startup. As showDocument neither blocks nor reports failures, the
background-thread fallback and the async failure callback are gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The "configured application or OS default" decision was duplicated in
every OS subclass and in openExternalFilePlatformIndependent. It now
lives in NativeDesktop.openFile/openFileWithApplication; subclasses only
implement openFileWithDefaultApplication and openFileWithCustomApplication.
This also fixes Windows trying to start "" when no application is set.

NativeDesktop is created in Launcher, passed to the preferences and
registered in the Injector instead of being re-created on every get().
The multi-selection "open folder" now honours the custom file browser.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Whether the platform offers a trash is a runtime capability, not a user
preference. The preference now only stores the user's choice (default:
true) and DeleteFileAction checks the capability when deleting. This
also avoids an UnsupportedOperationException when the preference is set
on a platform without trash support.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The main file and last used directory were initially read with the
FilePreferences defaults, but reset to the Documents folder looked up
via NativeDesktop. FilePreferences.getDefault() is now the only source,
so JabRefGuiPreferences no longer depends on NativeDesktop and the
default file chooser directory lookup is gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The documents directory lookup moves from NativeDesktop to
Directories, so FilePreferences.getDefault() can use it for the main
file and last used directory. Fresh installs and resets now agree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- NativeDesktop receives HostServices via constructor and is created in
  JabRefGUI#start, removing the Injector lookup and its temporal coupling
- Mail goes through the OS URL handler, as HostServices starts the web
  browser for mailto: on Windows; failures are reported again
- DefaultDesktop falls back to HostServices for URLs
- mainFileDirectory defaults to unset again; documents lookup is memoized
- Tests use mocks instead of FakeDesktop

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…esolverDialog

BackupResolverDialog passed null as DialogService, which caused a
NullPointerException when a custom file browser was configured.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Looking up the documents directory needs JNA on Windows and an external
process on Linux, which does not belong into jablib for a value that is
only read by the attach file dialog.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The instance receives GuiPreferences through its constructor, so the
open methods no longer take preference parameters. The static helpers
delegate to the injected instance and are deprecated. The OS-specific
folder and console hooks become protected, like the file open hooks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@calixtus calixtus changed the title Use JavaFX HostServices instead of AWT to browse and mail Restructure NativeDesktop: HostServices for links, injected instance, trash outside the preferences Oct 9, 2026

@koppor koppor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe, we can easily keep the Files-based existance check of "Documents".

If we want to support linux, we could do

String xdgDocumentsDir = System.getenv("XDG_DOCUMENTS_DIR");

the process builder with xdg-user-dir is optional.


I am unsure if the initial impression of JabRef should be nerdy or more user-centric. I think, "Documents" is more user-centric and "Home" more nerdy, but I might be wrong.

Claude:

  • The Documents-folder lookup (JNA on Windows, xdg-user-dir on Linux). Consequence: "Reset preferences" leaves the main file directory unset instead of Documents, and the last-used directory starts at the home directory. Fresh installs already behaved like this on main, so the loss is small. The JNA platform module is no longer required by jabgui, which is a nice side effect.

I was not aware of this.


Behaviour shifts to be aware of

  • Trash default is now a hard true in FilePreferences instead of "whatever the platform supports". In the GUI the effective default
    was already true on all three desktop platforms, so users will not notice. On a platform without trash, the preferences checkbox
    now shows checked but disabled, which is slightly odd but harmless since DeleteFileAction checks support before calling
    moveToTrash. In jabkit the value is never used.
  • URL opening moved onto the calling thread. The old Desktop.browse was deliberately run on a background executor because it can
    block on some Linux desktops. JavaFX's Linux showDocument probes browser candidates with which synchronously. Short, but it now
    happens on the FX thread. Low risk.

Good:

DefaultDesktop now actually honours a custom application (ProcessBuilder(application, file)) instead of silently ignoring it.
That is a fix, not a loss.

@github-actions github-actions Bot added the status: changes-required Pull requests that are not yet complete label Oct 22, 2026
koppor and others added 3 commits November 4, 2026 00:32
explorer.exe truncates its argument at MAX_PATH, so opening a linked file
whose absolute path has 260 characters or more handed the shell a name that
matches nothing. Explorer has no error path for that: it shows the user's
Documents folder. "Reveal in file explorer" failed the same way, with
`/select,` silently ignored.

Both openers now go through one command builder. Long paths and URLs take
the URL protocol handler, which has no such limit; revealing a long path
opens the containing folder, losing only the selection.

The limit counts the terminating NUL character, so 260 characters already
fail. The URL branch used `> 260` and was off by one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Conflict resolutions, all toward this branch's instance-based `NativeDesktop`
while keeping what upstream changed:

- `Windows`: `shellOpenCommand` replaces upstream's `urlOpenCommand`. It covers
  URLs as well as file paths and uses `>= 260`, where `urlOpenCommand` had
  `> 260` and let an argument of exactly the limit fail.
- `NativeDesktop`: the custom terminal command stays upstream's `List<String>`
  from `CommandLineParser`, dispatched through the instance hook.
- `BackupUIManager`: upstream's `showForLibrary` wrapper keeps this branch's
  `DialogService` argument to `BackupResolverDialog`.
- `JabRefCliPreferences`: upstream's `removeDeclinedCustomEntryTypes`, without
  the `getDefaultPath` hook this branch removed.
- `NativeDesktopTest`, `WindowsTest`: this branch's versions; `FakeDesktop` is
  gone, and `WindowsTest` covers both URL cases.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Carries over what `c7cf7553c1` does for the static method on the `directory-*`
branches: a directory library's root is a location a terminal can open at, so
only a file falls back to its parent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@koppor koppor added this to the 2 - for-upstream milestone Nov 4, 2026
Documents why Windows' documented long path opt-in does not apply: it governs
the process declaring it, while the truncation happens in `explorer.exe`, which
JabRef starts. Number 0081 because 0080 is taken by the open PR 17673.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@calixtus

calixtus commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

Moved to JabRef#17752

@calixtus calixtus closed this Nov 9, 2026
@calixtus
calixtus deleted the hostservices-browse-mail branch November 9, 2026 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: changes-required Pull requests that are not yet complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants