Emit --disable-features once instead of letting a later one discard it

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

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
 # ---------------------------------------------------------------------------