fix: bypass Selenium Manager only where it cannot run - #188
Merged
juliandescottes merged 2 commits intoSep 10, 2026
Merged
juliandescottes merged 2 commits into
juliandescottes merged 2 commits into
Conversation
PR mozilla#170 made every platform pass an explicit geckodriver path to firefox.ServiceBuilder. That fixed aarch64 Linux, where Selenium Manager ships an x86-64 binary that cannot execute, but it also regressed browser resolution in 0.10.2 for everyone else. selenium-webdriver's Firefox Driver.createSession() calls getBinaryPaths() only when the supplied DriverService has no executable, and that one call resolves geckodriver *and* the Firefox binary it injects into moz:firefoxOptions.binary. Handing the service a path therefore also opts out of finding Firefox. On a machine with no system Firefox, 0.10.1 launched fine because Selenium Manager downloaded and selected one under ~/.cache/selenium/firefox/; 0.10.2 fails with "Unable to detect Firefox binary automatically" unless --firefox-path is given. This is the same mechanism the Android branch relies on deliberately, where skipping getBinaryPaths() is what stops a desktop binary being injected over androidPackage. Resolve geckodriver ourselves only where Selenium Manager genuinely cannot do the job: win32, where ServiceBuilder() invoked from the MCP hangs (Bug 2040849), and non-x64 Linux (Bug 2062055). Elsewhere leave the executable unset so Selenium Manager resolves both halves as it did in 0.10.1. The test added in mozilla#170 asserted an explicit path on every platform, so it encoded the regression as expected behaviour and could not have caught this. It is replaced with per-platform cases covering both sides, verified to fail against the 0.10.2 code and pass against this change. Reported and diagnosed by @mightykatun on mozilla#170.
juliandescottes
approved these changes
Sep 10, 2026
juliandescottes
left a comment
Collaborator
There was a problem hiding this comment.
Thanks! I will just trim the comment and merge.
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 the browser-resolution regression in 0.10.2 reported by @mightykatun on #170. Their diagnosis was correct and this is essentially the fix they proposed.
What broke
#170 (mine) made every platform pass an explicit geckodriver path to
firefox.ServiceBuilder. That fixed aarch64 Linux, where Selenium Manager ships an x86-64 binary that cannot execute, but it regressed everyone else.selenium-webdriver's FirefoxDriver.createSession()callsgetBinaryPaths(caps)only when the suppliedDriverServicehas no executable. That one call resolves geckodriver and the Firefox binary, which it then injects intomoz:firefoxOptions.binary. Handing the service a path therefore also opts out of finding Firefox.On a machine with no system Firefox and none on
PATH:~/.cache/selenium/firefox/, andlist_pagessucceeds with no--firefox-path.Unable to detect Firefox binary automatically, please provide the full path via --firefox-path.This is the same mechanism the Android branch in
connect()relies on deliberately, where skippinggetBinaryPaths()is what stops a desktop binary being injected overandroidPackage. The comment there describes the behaviour exactly. I changed the desktop path without noticing it applied there too.The fix
Bypass Selenium Manager only where it genuinely cannot do the job:
win32, whereServiceBuilder()invoked from the MCP hangs (Bug 2040849)Everywhere else the executable is left unset, so Selenium Manager resolves both halves as it did in 0.10.1.
On the test
The unit test I added in #170 asserted an explicit service executable on every platform. It encoded the regression as the expected behaviour, so it could never have caught this, and it passed happily while users broke. It is replaced with per-platform cases covering both sides.
I verified these actually fail against the shipped code rather than just passing against mine: reverting
core.tsto the 0.10.2 logic fails the two "leaves the service executable unset" cases and passes the rest.With this change: 686 unit tests pass, lint, prettier and both typecheck configs clean.
Not covered here
The longer-term options @mightykatun raised, an architecture-compatible Selenium Manager binary or our own browser resolver, would let all platforms share one path. This is the minimal restoration of 0.10.1 behaviour for affected users; happy to follow up on either.
Sorry for the breakage, and thanks to @mightykatun for a report that arrived with the root cause already in it.