Repository navigation
Replace OSXAdapter with java.awt.Desktop handlers - #557
Open
tonygermano wants to merge 2 commits into
Open
tonygermano wants to merge 2 commits into
tonygermano wants to merge 2 commits into
Conversation
tonygermano
added
the
licensing
tonygermano
requested review from
gibson9583,
jonbartels,
kayyagari,
kpalang,
mgaffigan,
pacmano1 and
ssrowe
November 3, 2026 20:21
OSXAdapter is Apple's sample adapter, carried under Apple's own license header, that installs the macOS About and Quit handlers by reflection against com.apple.eawt.Application. Its pre-Java-9 path implements com.apple.eawt.ApplicationListener, which JEP 272 removed, and its Java 9+ path only works because the client launcher passes --add-opens java.desktop/com.apple.eawt. Both reach the same java.awt.desktop handlers that java.awt.Desktop has exposed publicly since Java 9. Register the two handlers through Desktop directly, guarded by isSupported so the IS_OS_MAC check is no longer needed; Linux and Windows report both actions unsupported and keep their Help menu and window close behavior. The quit handler keeps the old contract: Frame.logout(true) does not call System.exit, so it calls performQuit when quitMac returns true and cancelQuit otherwise. Delete OSXAdapter.java and drop the --add-opens for com.apple.eawt, which existed only for it, from the JNLP template, its test, and the devClient task. Assisted-by: Claude:claude-fable-5-1 Signed-off-by: Tony Germano <tony@germano.name>
openURL located java.awt.Desktop by reflection and, when that failed, fell back to com.apple.eio.FileManager on macOS or rundll32 on Windows. Java 17 is the baseline, so Desktop is always present, and on macOS and Windows it is always supported on a desktop JVM, so neither branch could be reached. Call Desktop.browse directly and keep only the Unix browser search, which still matters on Linux desktops without the GNOME libraries that Desktop needs. Drop the --add-opens for com.apple.eio from the JNLP template, its test, and the devClient task. Assisted-by: Claude:claude-fable-5-1 Signed-off-by: Tony Germano <tony@germano.name>
tonygermano
force-pushed
the
client/drop-osxadapter
branch
from
November 4, 2026 22:37
38124c6 to
3b73238
Compare
mgaffigan
requested changes
Nov 10, 2026
Comment on lines
+49
to
+55
| String browser = null; | ||
| for (String b : browsers) | ||
| if (browser == null | ||
| && Runtime.getRuntime().exec(new String[] { "which", b }).getInputStream().read() != -1) | ||
| Runtime.getRuntime().exec(new String[] { browser = b, url }); | ||
| if (browser == null) | ||
| throw new Exception(Arrays.toString(browsers)); |
Contributor
There was a problem hiding this comment.
Can we just go for the exec? This seems like it is a TOCTOU. Presumably the exec of the browser will fail sufficiently to avoid which - which is not required to be present as a binary in the first place - it's normally a shell built-in.
Member
Author
There was a problem hiding this comment.
That was preexisting code that came from the upsteam launcher. It's only showing up here because of a whitespace change.
It's iterating through an array of options, so we'd need to wrap it in a new try/catch to ignore all of the exceptions until it exhausts the list. I'm not sure what we would gain since this works in all Linux environments I've tested it in.
Member
Author
There was a problem hiding this comment.
If you review only commit 2 and hide whitespace changes, this change is just a delete and directly importing Desktop instead of finding it by reflection.
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
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.
Summary
OSXAdapteris Apple's sample adapter, under Apple's own license header (relates to #472), that installs the macOS About and Quit handlers by reflection againstcom.apple.eawt.Application. Its pre-Java-9 path implementscom.apple.eawt.ApplicationListener, which JEP 272 removed; its Java 9+ path only works because the launcher passes--add-opens java.desktop/com.apple.eawt. Both end up at the samejava.awt.desktophandlers thatjava.awt.Desktophas exposed publicly since Java 9, so the reflection, the add-opens, and the Apple-licensed file can all go. #544 drew attention to the class while converting its stderr prints.OSXAdapter.javaand registers the two handlers withjava.awt.Desktop.setAboutHandler/setQuitHandler, guarded byisSupportedinstead ofIS_OS_MAC. Linux and Windows report both actions unsupported, so nothing changes there. The quit handler keeps the old contract:Frame.logout(true)doesn't callSystem.exit, so it callsperformQuit()whenquitMac()returns true andcancelQuit()otherwise.BareBonesBrowserLaunch.openURLto callDesktop.browsedirectly and drops itscom.apple.eio.FileManagerandrundll32fallbacks, which can't be reached on Java 17 sinceDesktopis always supported on macOS and Windows. The Unix browser search stays for Linux desktops without the GNOME librariesDesktopneeds.--add-opensforcom.apple.eawtandcom.apple.eiofromserver/mirth-client.jnlp,WebStartServletTest, and thedevClientGradle task. Nothing else referenced those packages.Testing
I verified that the browser launching still works from non-GNOME linux.
This needs a manual check on macOS, which I can't do
That is the behavior the old Java 9+ path provided, so no change is expected.
Claude checked:
./gradlew clean build: all 797 tests pass.com.appleanywhere in the repository.🤖 Drafted with Claude Code