From e4c75bf486c8d9dfd32ea8b206ce48e788d61ea1 Mon Sep 17 00:00:00 2001 From: dgtlmoon Date: Thu, 17 Sep 2026 20:15:20 +0200 Subject: [PATCH] Browser picker - offer each browser once, not twice On any upgraded install update_35 has migrated the old per-engine request timeout and User-Agent into browser configs keyed by the engine name ('html_requests', 'html_webdriver'), so those ids exist in BOTH the built-in engine list and browsers.json. list_watch_browser_choices() concatenated the two sources, so the watch edit page rendered the same browser as two radios with the same label - and the group override select did the same, from its own copy of that concatenation. Deduplicated by value at the source, keeping each value's first position (built-ins stay in engine order) with its last label - the saved one, since it is the same browser under the name the user can actually change. The group override select and the Add-Watch list now derive from that single list instead of rebuilding or re-deduplicating it. Co-Authored-By: Claude Opus 5 (1M context) --- .../blueprint/add_watch_ui/browser_config.py | 9 ++---- changedetectionio/blueprint/tags/__init__.py | 18 +++++------ changedetectionio/model/browser_config.py | 14 ++++++--- .../tests/test_browser_config.py | 30 +++++++++++++++++++ 4 files changed, 49 insertions(+), 22 deletions(-) diff --git a/changedetectionio/blueprint/add_watch_ui/browser_config.py b/changedetectionio/blueprint/add_watch_ui/browser_config.py index 2e6949a71..f95d80698 100644 --- a/changedetectionio/blueprint/add_watch_ui/browser_config.py +++ b/changedetectionio/blueprint/add_watch_ui/browser_config.py @@ -91,13 +91,8 @@ def list_visual_browser_choices(datastore): """ from changedetectionio.model.browser_config import list_watch_browser_choices - # A saved browser config deliberately shares its id with the built-in engine it was - # migrated from (see update_35), so the candidate list can name the same browser twice - - # keep the later (saved) label, which is the one the user can rename. - candidates = {value: label for value, label in list_watch_browser_choices(datastore) - if value != SYSTEM_DEFAULT} - choices = [(value, str(label)) for value, label in candidates.items() - if is_visual_capable(value, datastore)] + choices = [(value, str(label)) for value, label in list_watch_browser_choices(datastore) + if value != SYSTEM_DEFAULT and is_visual_capable(value, datastore)] logger.debug(f"Add-watch browsers offered for the live preview: " f"{[value for value, _label in choices] or 'none'}") return choices diff --git a/changedetectionio/blueprint/tags/__init__.py b/changedetectionio/blueprint/tags/__init__.py index 5355e469a..a8a8c8612 100644 --- a/changedetectionio/blueprint/tags/__init__.py +++ b/changedetectionio/blueprint/tags/__init__.py @@ -10,17 +10,13 @@ from changedetectionio.llm.evaluator import get_llm_config as _get_llm_config def _browser_config_choices(datastore): - """Browsers the group can override with: the always-present built-in engines, then the - user's saved browsers - the same set a watch can pick. No 'None' entry: the enabler - checkbox (browser_config_overrides_watch) is what turns the override on/off, and the - select is disabled when it's unchecked.""" - from changedetectionio.model.browser_config import list_builtin_browsers - choices = [] - for b in list_builtin_browsers(): - choices.append((b['id'], b['label'])) - for cid, entry in datastore.browser_config_store.all().items(): - choices.append((cid, entry.get('label') or cid)) - return choices + """Browsers the group can override with: exactly what a watch can pick, minus 'system'. + + No 'None'/'system' entry: the enabler checkbox (browser_config_overrides_watch) is what + turns the override on/off, and the select is disabled when it's unchecked.""" + from changedetectionio.model.browser_config import list_watch_browser_choices + return [(value, label) for value, label in list_watch_browser_choices(datastore) + if value != 'system'] def construct_blueprint(datastore: ChangeDetectionStore): diff --git a/changedetectionio/model/browser_config.py b/changedetectionio/model/browser_config.py index e4c38e0b1..1b0379e08 100644 --- a/changedetectionio/model/browser_config.py +++ b/changedetectionio/model/browser_config.py @@ -485,13 +485,19 @@ def is_valid_browser_selector(value, datastore, allow_empty=True): def list_watch_browser_choices(datastore): """(value, label) choices for the watch-level 'Browser' picker: system default, the always-present built-in engine browsers, then the user's saved browsers. + + Deduplicated by value, because a saved config legitimately shares a built-in engine's id - + update_35 migrated the old per-engine request timeout / User-Agent into configs keyed + 'html_requests' and 'html_webdriver' - and listing one browser twice is not a choice. The + saved label wins: it is the same browser, and that is the name the user can change. """ - choices = [('system', _system_default_label(datastore))] + choices = {'system': _system_default_label(datastore)} for b in list_builtin_browsers(): - choices.append((b['id'], b['label'])) + choices[b['id']] = b['label'] for cid, entry in datastore.browser_config_store.all().items(): - choices.append((cid, entry.get('label') or cid)) - return choices + choices[cid] = entry.get('label') or cid + # dict keeps each value's first position (built-ins stay in engine order) with its last label + return list(choices.items()) def _system_default_label(datastore): diff --git a/changedetectionio/tests/test_browser_config.py b/changedetectionio/tests/test_browser_config.py index b85e8e145..402456458 100644 --- a/changedetectionio/tests/test_browser_config.py +++ b/changedetectionio/tests/test_browser_config.py @@ -1008,3 +1008,33 @@ def test_update_36_survives_an_unusable_legacy_endpoint(client, live_server, mea assert datastore.browser_config_store.get('extra_browser_Broken') is None assert datastore.browser_config_store.get('extra_browser_Good')['browser_config']['connection_url'] \ == 'wss://good.example:9222' + + +def test_browser_picker_lists_each_browser_once(client, live_server, measure_memory_usage, datastore_path): + """A saved config that shares a built-in engine's id must not appear twice in the picker. + + update_35 migrates the old per-engine request timeout / User-Agent into configs keyed + 'html_requests' / 'html_webdriver', so on any upgraded install those ids exist in BOTH the + built-in engine list and browsers.json - which rendered the same browser as two radios on the + watch edit page (and in the group override select). + """ + from changedetectionio.model.browser_config import list_watch_browser_choices + datastore = client.application.config.get('DATASTORE') + + # Exactly what an upgraded install looks like after update_35 + datastore.browser_config_store.upsert('html_requests', label='Basic fast Plaintext/HTTP Client', + base_fetcher='html_requests', browser_config={'timeout': 33}) + + values = [value for value, _label in list_watch_browser_choices(datastore)] + assert len(values) == len(set(values)), f"each browser must be offered once: {values}" + assert values.count('html_requests') == 1 + + # ...and the watch edit page renders one radio per browser + uuid = datastore.add_watch(url="https://example.com") + res = client.get(url_for("ui.ui_edit.edit_page", uuid=uuid)) + assert res.status_code == 200 + assert res.data.count(b'name="fetch_backend" type="radio" value="html_requests"') == 1 + + # A renamed config wins the label, since it is the same browser under a name the user chose + datastore.browser_config_store.update('html_requests', label='Fast plain client') + assert dict(list_watch_browser_choices(datastore))['html_requests'] == 'Fast plain client'