From aaf608603c9bb24c3cacc41b51e7407a3dff3a96 Mon Sep 17 00:00:00 2001 From: Aaron Iba Date: Thu, 30 Jul 2026 10:47:48 -0400 Subject: [PATCH 1/2] fill_input: clear via element.select(), not a synthetic Cmd/Ctrl+A MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The select-all shortcut never fires over CDP. Measured on Brave 150: selectionEnd - selectionStart == 0 on an 11-char field, in every tab state (activated, background, background with focus emulation). Backspace then deletes at the caret rather than the selection, and the new text lands beside the old value instead of replacing it. The corruption is silent and caret-dependent, so it is not even consistent — two runs of the same call produced 'REPLACEDpreexisting' and 'preexistinREPLACED' where both should have been 'REPLACED'. Select with element.select() (selectAllChildren for contenteditable) instead. Skip the Backspace when the field is already empty: select() on an empty field leaves it in a state where subsequent characters don't insert at all. For clear_first=False, park the caret at the end — focus() puts it at 0, so appended text would otherwise be prepended. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01G35BbXw4eNsefYAT5Yd45N --- src/browser_harness/helpers.py | 38 ++++++++++++++++------- tests/unit/test_helpers.py | 55 ++++++++++++++++++++++------------ 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/src/browser_harness/helpers.py b/src/browser_harness/helpers.py index f6ec182e..27452407 100644 --- a/src/browser_harness/helpers.py +++ b/src/browser_harness/helpers.py @@ -194,16 +194,34 @@ def fill_input(selector, text, clear_first=True, timeout=0.0): if not focused: raise RuntimeError(f"fill_input: element not found: {selector!r}") if clear_first: - # Dispatch select-all directly — NOT via press_key, which always emits a - # `char` event for single-char keys. With Ctrl/Cmd held, that `char` - # makes Chrome treat the input as a printable "a" instead of firing the - # select-all shortcut, leaving the field uncleared. - mods = 4 if sys.platform == "darwin" else 2 # Cmd on macOS, Ctrl elsewhere - select_all = {"key": "a", "code": "KeyA", "modifiers": mods, - "windowsVirtualKeyCode": 65, "nativeVirtualKeyCode": 65} - cdp("Input.dispatchKeyEvent", type="rawKeyDown", **select_all) - cdp("Input.dispatchKeyEvent", type="keyUp", **select_all) - press_key("Backspace") + # Select via element.select(), not a synthetic Cmd/Ctrl+A: the + # shortcut never fires select-all over CDP. Measured on Brave 150, + # selectionEnd - selectionStart == 0 on an 11-char field, in all three + # tab states (activated, background, background + focus emulation), so + # this is not background-tab specific. Backspace then deletes at the + # caret and the new text lands beside the old value instead of + # replacing it — silently, and the caret position varies, so the same + # call can yield 'REPLACEDpreexisting' or 'preexistinREPLACED'. + # Only select when there IS content: select() on an empty field leaves + # it in a state where subsequent characters don't insert at all. + had_content = js( + f"(()=>{{const e=document.querySelector({json.dumps(selector)});" + f"if(!e)return false;" + f"const v=('value'in e)?e.value:e.textContent;" + f"if(!v)return false;" + f"if(e.select)e.select();else document.getSelection().selectAllChildren(e);" + f"return true;}})()" + ) + if had_content: + press_key("Backspace") + else: + # Append semantics: focus() parks the caret at 0, so typed text would + # land before the existing value. Move it to the end. + js( + f"(()=>{{const e=document.querySelector({json.dumps(selector)});" + f"if(e&&e.setSelectionRange&&'value'in e)" + f"try{{e.setSelectionRange(e.value.length,e.value.length)}}catch(_){{}}}})()" + ) for ch in text: press_key(ch) js( diff --git a/tests/unit/test_helpers.py b/tests/unit/test_helpers.py index 4a45ee07..4d7a1431 100644 --- a/tests/unit/test_helpers.py +++ b/tests/unit/test_helpers.py @@ -110,10 +110,9 @@ def fake_js(expr, **kwargs): helpers.fill_input("#missing", "hello") -def test_fill_input_clear_first_sends_select_all_then_backspace(): - import sys - +def test_fill_input_clear_first_selects_via_js_then_backspace(): key_events = [] + js_calls = [] def fake_cdp(method, **kwargs): if method == "Input.dispatchKeyEvent": @@ -121,31 +120,49 @@ def fake_cdp(method, **kwargs): return {} def fake_js(expr, **kwargs): - return True # element found + js_calls.append(expr) + return True # element found / field has content with patch("browser_harness.helpers.cdp", side_effect=fake_cdp), \ patch("browser_harness.helpers.js", side_effect=fake_js): helpers.fill_input("#inp", "x", clear_first=True) - # The "a" must be dispatched with the platform-correct modifier (Meta=4 on - # macOS, Ctrl=2 elsewhere). Without the modifier, the field would never get - # selected — it would just receive a literal "a". - expected_mod = 4 if sys.platform == "darwin" else 2 - a_events = [e for e in key_events if e.get("key") == "a"] - assert a_events, "expected an 'a' key event for select-all" - assert all(e.get("modifiers") == expected_mod for e in a_events), \ - f"select-all 'a' must carry modifiers={expected_mod}; got {[e.get('modifiers') for e in a_events]}" - - # Crucial: no `char` event for the "a" — emitting one makes Chrome treat - # Cmd/Ctrl+A as a printable letter instead of a shortcut. - assert not any(e.get("type") == "char" and e.get("text") == "a" for e in key_events), \ - "select-all must not emit a 'char' event with text='a' (would cancel the shortcut)" - - # Backspace still fires (via press_key, which uses keyDown). + # Clearing selects via element.select() in JS, not a synthetic Cmd/Ctrl+A: + # on a focus-emulated background tab that shortcut selects nothing, so + # Backspace deletes at the caret and the new text lands beside the old + # value rather than replacing it. + assert any(".select()" in e or "selectAllChildren" in e for e in js_calls), \ + "clear_first must select the field content via JS" + assert not any(e.get("key") == "a" for e in key_events), \ + "clear_first must not dispatch Cmd/Ctrl+A key events" + + # Backspace still fires, to delete the now-selected content. keys_down = [e.get("key") for e in key_events if e.get("type") in ("keyDown", "rawKeyDown")] assert "Backspace" in keys_down +def test_fill_input_clear_first_skips_backspace_on_empty_field(): + key_events = [] + + def fake_cdp(method, **kwargs): + if method == "Input.dispatchKeyEvent": + key_events.append(kwargs) + return {} + + def fake_js(expr, **kwargs): + # focus() lookup finds the element; the select-content check reports an + # empty field. select() on an empty field leaves it in a state where + # subsequent characters don't insert at all. + return False if ".select()" in expr or "selectAllChildren" in expr else True + + with patch("browser_harness.helpers.cdp", side_effect=fake_cdp), \ + patch("browser_harness.helpers.js", side_effect=fake_js): + helpers.fill_input("#inp", "x", clear_first=True) + + keys_down = [e.get("key") for e in key_events if e.get("type") in ("keyDown", "rawKeyDown")] + assert "Backspace" not in keys_down, "empty field must not receive Backspace" + + def test_fill_input_no_clear_skips_ctrl_a(): key_events = [] From babf1f659ec1e07ade3a335caf5c7a08f3bcea00 Mon Sep 17 00:00:00 2001 From: Aaron Iba Date: Thu, 30 Jul 2026 11:52:07 -0400 Subject: [PATCH 2/2] fill_input: move the caret to the end for contenteditable too The clear path handles contenteditable via selectAllChildren, but the clear_first=False path only called setSelectionRange, which contenteditable elements do not have. The caret stayed at the browser default (position 0), so "append" prepended instead:
keep
fill_input("#ce", "-more", clear_first=False) # was '-morekeep', now 'keep-more' Fall back to a Range collapsed to the end of the element's contents. Caught by cubic-dev-ai review on #570 and confirmed against a live page. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01G35BbXw4eNsefYAT5Yd45N --- src/browser_harness/helpers.py | 9 +++++++-- tests/unit/test_helpers.py | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/browser_harness/helpers.py b/src/browser_harness/helpers.py index 27452407..c4ac57bc 100644 --- a/src/browser_harness/helpers.py +++ b/src/browser_harness/helpers.py @@ -219,8 +219,13 @@ def fill_input(selector, text, clear_first=True, timeout=0.0): # land before the existing value. Move it to the end. js( f"(()=>{{const e=document.querySelector({json.dumps(selector)});" - f"if(e&&e.setSelectionRange&&'value'in e)" - f"try{{e.setSelectionRange(e.value.length,e.value.length)}}catch(_){{}}}})()" + f"if(!e)return;" + f"if('value'in e&&e.setSelectionRange)" + f"{{try{{e.setSelectionRange(e.value.length,e.value.length)}}catch(_){{}}return}}" + # contenteditable has no setSelectionRange; collapse a Range to the end + # instead, or the caret stays at 0 and 'append' prepends. + f"const r=document.createRange();r.selectNodeContents(e);r.collapse(false);" + f"const s=document.getSelection();s.removeAllRanges();s.addRange(r);}})()" ) for ch in text: press_key(ch) diff --git a/tests/unit/test_helpers.py b/tests/unit/test_helpers.py index 4d7a1431..b0b8dd55 100644 --- a/tests/unit/test_helpers.py +++ b/tests/unit/test_helpers.py @@ -367,3 +367,22 @@ def fake_send(req): "session filter, the background rWS/lF pair would have updated " "last_activity and prevented the idle window from elapsing." ) + + +def test_fill_input_append_moves_caret_to_end_for_contenteditable(): + js_calls = [] + + def fake_js(expr, **kwargs): + js_calls.append(expr) + return True + + with patch("browser_harness.helpers.cdp", side_effect=lambda *a, **k: {}), \ + patch("browser_harness.helpers.js", side_effect=fake_js): + helpers.fill_input("#ce", "x", clear_first=False) + + # contenteditable has no setSelectionRange; without a Range fallback the + # caret stays at 0 and "append" silently prepends. + caret = [e for e in js_calls if "setSelectionRange" in e or "collapse" in e] + assert caret, "clear_first=False must move the caret to the end" + assert any("collapse(false)" in e and "selectNodeContents" in e for e in caret), \ + "caret move must fall back to a collapsed Range for contenteditable"