fix(web_browser): fall back to scale factor 1 on macOS when AppKit is unavailable#4479
fix(web_browser): fall back to scale factor 1 on macOS when AppKit is unavailable#4479he-yufeng wants to merge 10 commits into
Conversation
… unavailable
`get_screen_scale_factor()` guards the Windows and Linux branches with
try/except and defaults to 1.0, but the macOS branch imported AppKit and
dereferenced `NSScreen.mainScreen()` unconditionally. Two reachable cases
crashed instead of degrading:
- `pyobjc-framework-AppKit` is an optional dependency ("required to run
headfully on macOS"), so a headful run without it raised ImportError.
- `NSScreen.mainScreen()` returns None with no display attached (headless /
over SSH), so `.backingScaleFactor()` raised AttributeError.
Wrap the import and null-guard the screen so macOS degrades to 1.0 like the
other platforms. Adds unit tests for the working, headless, and missing-AppKit
paths.
24851e6 to
8291f7c
Compare
…tor-fallback # Conflicts: # CHANGELOG.md
dragonstyle
left a comment
dragonstyle
left a comment
There was a problem hiding this comment.
I think that we should catch a ModuleNotFoundError and warn once before we return the default value.
…-factor probe Per review: catch the missing-AppKit ModuleNotFoundError specifically and log a one-time warning (pointing at `pip install pyobjc-framework-AppKit`) before returning the default scale factor of 1, instead of degrading silently. The darwin probe is extracted into a helper and the warning is gated with functools.cache so it fires at most once per process.
…tor-fallback # Conflicts: # CHANGELOG.md
|
Done — the darwin probe now catches the missing-AppKit |
| def _warn_missing_appkit() -> None: | ||
| # @cache makes this warn exactly once per process, however many headful macOS | ||
| # calls hit the missing optional AppKit dependency. | ||
| logger.warning( |
There was a problem hiding this comment.
You can just use def warn_once(logger: Logger, message: str) -> None:
There was a problem hiding this comment.
Done — swapped the @cache one-off for a warn_once(logger, message) helper.
One wrinkle worth flagging: inspect_tool_support runs inside the sandbox container where inspect_ai isn't importable (there's already a # TODO: cloned from inspect_ai repo code that is unavailable in the container note in _util/json_rpc_helpers.py), so I mirrored the inspect_ai/_util/logger.py warn_once locally rather than importing it. Same signature you suggested.
2 participants
This PR contains:
What is the current behavior?
get_screen_scale_factor()(called by the web browser tool when running headful) guards its Windows and Linux branches withtry/exceptand defaults to1.0, but the macOS branch importsAppKitand dereferencesNSScreen.mainScreen()unconditionally. Two reachable cases crash instead of degrading:pyobjc-framework-AppKitis an optional dependency — its own comment notes it's "required to run headfully on macOS" — so a headful run without it raisesModuleNotFoundError(reproduced on macOS with the package absent).NSScreen.mainScreen()returnsNonewhen no display is attached (headless / over SSH), so.backingScaleFactor()raisesAttributeErroreven with AppKit installed.What is the new behavior?
The macOS branch wraps the
AppKitimport and null-guardsmainScreen(), falling back to a scale factor of1.0— consistent with the Windows and Linux branches and the function's documented default. Adds unit tests for the working, headless, and missing-AppKit paths.Does this PR introduce a breaking change?
No.
Other information:
Reproduced on macOS without
pyobjc-framework-AppKit: the probe raisedModuleNotFoundError: No module named 'AppKit'; after the fix it returns1.0. The three new tests cover the working, headless (mainScreen()isNone), and missing-import paths.