{{ message }}
Repository navigation
fix(desktop): open default browser on startup - #2095
Merged
Merged
Conversation
The Go desktop binary printed its URL but never launched a browser. Add a stdlib-only, GOOS-dispatched opener, bind the listener before opening to avoid a startup race, and add --no-browser for headless use. Browser-launch failure remains non-fatal. Fixes maplibre#934.
Collaborator
|
Thanks for taking the time to open this PR! Is there a way to do it without being os specific? Also worth de-vibing the excessive comments... |
Collaborator
|
I'm also not sure the tests are really valuable here... |
Address review from maplibre#2095: replace the hand-rolled GOOS switch with github.com/pkg/browser so this package carries no OS-specific launch code of its own; run it in a goroutine since xdg-open is known to hang on some headless Linux setups, so a stuck opener can't stall server startup. Trim comments and collapse the tests to the one invariant that actually matters: a failed/absent opener doesn't block or crash the server.
Contributor
Author
|
Thanks for the review — pushed 5d8048b addressing all three points:
Let me know if you'd rather see this simplified further (e.g. drop the goroutine/test entirely and accept the rare hang risk) — happy to go either way. |
HarelM
reviewed
Aug 22, 2026
Collaborator
There was a problem hiding this comment.
This is a thin wrapper on browser package, I think it's not worth the tests...
Contributor
Author
There was a problem hiding this comment.
Agreed — pushed e6ffc5b: removed both browser.go and browser_test.go, inlined the goroutine directly into maputnik.go's app.Action. It really was just a thin wrapper with no logic worth isolating or testing separately.
HarelM
approved these changes
Aug 22, 2026
Collaborator
Per review: browser.go was a 21-line wrapper around pkg/browser with no logic of its own, and browser_test.go was testing that wrapper rather than project behavior. Inlined the goroutine directly into maputnik.go's app.Action and removed both files.
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.
Fixes #934
Summary
The Go desktop binary started its local server and printed the URL,
but never opened a browser automatically.
This PR:
--no-browserfor headless/Docker useThe listener is created before launching the browser so the browser
cannot race the server startup path.
Platform behavior
rundll32 url.dll,FileProtocolHandleropenxdg-openNo shell command strings are used; arguments are passed directly through
exec.Command.Testing
go test ./...go vet ./...go build ./...git diff --checkbrowser opened and hit the server, no duplicate launches)
--no-browserverification (confirmed no launch attempt occurs)nonexistent opener binary
macOS/Linux launch paths were not runtime-tested on this Windows machine —
they follow the standard
open/xdg-openconvention used across other GoCLIs but are unverified here. The full packaged release flow (
gox,go.rice,go-winres) was not exercised locally.