diff --git a/crawl4ai/browser_manager.py b/crawl4ai/browser_manager.py index f4ab0aa32..514c83b05 100644 --- a/crawl4ai/browser_manager.py +++ b/crawl4ai/browser_manager.py @@ -27,7 +27,10 @@ "--disable-component-extensions-with-background-pages", "--disable-default-apps", "--disable-extensions", - "--disable-features=TranslateUI", + # "TranslateUI" is not a feature name in current Chromium - the literal is + # absent from the shipped binary, so the switch matched nothing and the + # translate UI was never actually disabled. The feature is "Translate". + "--disable-features=Translate", "--disable-hang-monitor", "--disable-ipc-flooding-protection", "--disable-popup-blocking", @@ -40,6 +43,53 @@ "--use-mock-keychain", ] +# Chromium switches whose value is a comma-separated feature list. Chrome parses +# them last-wins: given the switch twice it keeps the last value and silently +# drops every earlier one, whole. Any code path that appends one of these rather +# than merging into the existing value therefore throws away what was declared +# before it, with no warning and no way to notice from the options object. +_FEATURE_LIST_SWITCHES = ("--disable-features=", "--enable-features=") + + +def merge_feature_switches(flags: List[str]) -> List[str]: + """Collapse repeated ``--disable-features`` / ``--enable-features`` switches. + + Each switch is emitted once, at the position of its first occurrence, so the + surrounding flag order is untouched. Feature names keep first-seen order and + are de-duplicated. + + >>> merge_feature_switches( + ... ["--a", "--disable-features=X,Y", "--b", "--disable-features=Y,Z"] + ... ) + ['--a', '--disable-features=X,Y,Z', '--b'] + """ + merged: Dict[str, List[str]] = {} + for flag in flags: + for switch in _FEATURE_LIST_SWITCHES: + if flag.startswith(switch): + names = merged.setdefault(switch, []) + for name in flag[len(switch) :].split(","): + name = name.strip() + if name and name not in names: + names.append(name) + break + + if not merged: + return list(flags) + + out: List[str] = [] + emitted = set() + for flag in flags: + for switch in _FEATURE_LIST_SWITCHES: + if flag.startswith(switch): + if switch not in emitted: + emitted.add(switch) + out.append(switch + ",".join(merged[switch])) + break + else: + out.append(flag) + return out + class ManagedBrowser: """ @@ -121,8 +171,9 @@ def build_browser_flags(config: BrowserConfig) -> List[str]: flags.append(f"--proxy-server={config.proxy}") elif config.proxy_config: flags.append(f"--proxy-server={config.proxy_config.server}") - # dedupe - return list(dict.fromkeys(flags)) + # dedupe, then fold the feature lists together so a later + # --disable-features does not discard an earlier one + return merge_feature_switches(list(dict.fromkeys(flags))) browser_type: str user_data_dir: str @@ -865,7 +916,8 @@ async def _start_impl(self): launch_kwargs = { "headless": self.config.headless, - "args": list(dict.fromkeys(cli_args)), # dedupe + # dedupe, then merge the feature lists + "args": merge_feature_switches(list(dict.fromkeys(cli_args))), "viewport": { "width": self.config.viewport_width, "height": self.config.viewport_height, @@ -1134,9 +1186,10 @@ def _build_browser_args(self) -> dict: if self.config.extra_args: args.extend(self.config.extra_args) - # Deduplicate args - args = list(dict.fromkeys(args)) - + # Deduplicate args, then merge the feature lists so a later + # --disable-features does not discard an earlier one + args = merge_feature_switches(list(dict.fromkeys(args))) + browser_args = {"headless": self.config.headless, "args": args} # On Windows, passing channel='chromium' (the default) causes Playwright diff --git a/tests/regression/test_reg_browser.py b/tests/regression/test_reg_browser.py index dac55a841..6ae942583 100644 --- a/tests/regression/test_reg_browser.py +++ b/tests/regression/test_reg_browser.py @@ -131,6 +131,83 @@ async def test_browser_context_manager(local_server): # If we get here without exception, cleanup succeeded +# --------------------------------------------------------------------------- +# Launch flags +# --------------------------------------------------------------------------- + + +def _feature_switches(args): + """The --disable-features switches in `args`, in order.""" + return [a for a in args if a.startswith("--disable-features=")] + + +def _feature_names(args): + """Only the names Chrome actually applies: the value of the last switch.""" + switches = _feature_switches(args) + if not switches: + return [] + return [n for n in switches[-1].split("=", 1)[1].split(",") if n] + + +def test_merge_feature_switches_folds_repeats(): + """Chrome parses --disable-features last-wins: given the switch twice it + keeps the last value and silently drops the earlier one whole. Collapse + repeats into one switch, in place, so nothing declared is lost.""" + from crawl4ai.browser_manager import merge_feature_switches + + merged = merge_feature_switches( + ["--a", "--disable-features=X,Y", "--b", "--disable-features=Y,Z", "--c"] + ) + # one switch, at the position of the first occurrence, order otherwise intact + assert merged == ["--a", "--disable-features=X,Y,Z", "--b", "--c"] + + # --enable-features is parsed the same way and is folded separately + merged = merge_feature_switches( + ["--enable-features=A", "--disable-features=X", "--enable-features=B"] + ) + assert merged == ["--enable-features=A,B", "--disable-features=X"] + + # nothing to fold: the list is returned unchanged + assert merge_feature_switches(["--a", "--b"]) == ["--a", "--b"] + + +def test_build_browser_flags_emits_one_feature_switch(): + """ManagedBrowser.build_browser_flags must not emit --disable-features + twice. light_mode appends BROWSER_DISABLE_OPTIONS, whose own entry used to + land after the default one and take the browser with it.""" + from crawl4ai.browser_manager import ManagedBrowser + + for light_mode in (False, True): + config = BrowserConfig(headless=True, light_mode=light_mode) + flags = ManagedBrowser.build_browser_flags(config) + assert ( + len(_feature_switches(flags)) == 1 + ), f"light_mode={light_mode}: {_feature_switches(flags)}" + # the defaults survive light_mode instead of being displaced by it + names = _feature_names(flags) + for expected in ("OptimizationHints", "MediaRouter", "DialMediaRouteProvider"): + assert expected in names, f"light_mode={light_mode} lost {expected}" + + +def test_extra_args_feature_switch_merges_with_defaults(): + """A user --disable-features passed through extra_args used to be appended + after the default switch, so Chrome applied the user's names and none of + crawl4ai's. Both must survive.""" + from crawl4ai.browser_manager import BrowserManager + + config = BrowserConfig( + headless=True, + extra_args=["--disable-features=CalculateNativeWinOcclusion"], + ) + args = BrowserManager(browser_config=config)._build_browser_args()["args"] + + assert len(_feature_switches(args)) == 1, _feature_switches(args) + names = _feature_names(args) + assert "CalculateNativeWinOcclusion" in names, "user's own name dropped" + for expected in ("OptimizationHints", "MediaRouter", "DialMediaRouteProvider"): + assert expected in names, f"default {expected} displaced by extra_args" + + # --------------------------------------------------------------------------- # Viewport configuration # ---------------------------------------------------------------------------