browser: pass _default_context_options into new_context#455
Open
SAY-5 wants to merge 1 commit intostrands-agents:mainfrom
Open
browser: pass _default_context_options into new_context#455SAY-5 wants to merge 1 commit intostrands-agents:mainfrom
SAY-5 wants to merge 1 commit intostrands-agents:mainfrom
Conversation
Reported as strands-agents#414. LocalChromiumBrowser accepts a context_options kwarg in its constructor, merges it into self._default_context_options, and stores it correctly. But the base Browser._setup_session_from_browser calls session_browser.new_context() with no arguments, so the stored options never make it to Playwright. Viewport, user_agent, storage_state, locale, timezone_id, geolocation, permissions, color_scheme... all silently ignored. Initialise _default_context_options on the base Browser class (empty dict, no behaviour change for subclasses that don't set it) and unpack it when creating the non-persistent context. LocalChromiumBrowser already populates the dict, so users who passed context_options to its constructor now actually see those options applied. Persistent-context branch is left alone because it receives a user-supplied context object instead of creating one. Fixes strands-agents#414 Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
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 #414.
Problem
LocalChromiumBrowseraccepts acontext_optionskwarg in its constructor, merges it intoself._default_context_options, and stores it correctly. But the base classBrowser._setup_session_from_browserignores the dict and callsnew_context()with no arguments:Result:
viewport,user_agent,storage_state,locale,timezone_id,geolocation,permissions,color_scheme, …all silently ignored. The reporter's proof script showsviewport = 1280x720(Playwright default) instead of the requested1920x1080.Particularly impactful for
storage_state, the standard Playwright mechanism for reusing authentication across browser contexts. There is no public-API workaround; users today have to subclass and override_setup_session_from_browser.Fix
_default_context_options: Dict[str, Any] = {}on the baseBrowser.__init__(empty default → no behaviour change for subclasses that don't populate it).**self._default_context_optionsintosession_browser.new_context(...)in the non-persistent branch of_setup_session_from_browser.LocalChromiumBrowseralready populates the dict duringstart_platform(), so users who passedcontext_optionsto its constructor now actually see those options applied.Back-compat
_default_context_optionsget the empty-dict default, sonew_context(**{})is a no-op equivalent to today'snew_context().Signed off per DCO.