fix(helpers): press_key supports keyboard shortcuts (suppress char + canonical code/vk)

src/browser_harness/helpers.py

@@ -223,15 +223,10 @@ 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 now suppresses the printable text/char event when a
+        # shortcut modifier (Alt/Ctrl/Meta) is held, so this dispatches a
+        # real Cmd+A / Ctrl+A instead of typing the letter "a".
+        press_key("a", modifiers=4 if sys.platform == "darwin" else 2)
         press_key("Backspace")
     for ch in text:
         press_key(ch)
@@ -250,15 +245,54 @@ def fill_input(selector, text, clear_first=True, timeout=0.0):
     "Home": (36, "Home", ""), "End": (35, "End", ""),
     "PageUp": (33, "PageUp", ""), "PageDown": (34, "PageDown", ""),
 }
+def _key_metadata(key):
+    """(windowsVirtualKeyCode, code, text) for a key arg to press_key.
+
+    Letters and digits resolve to canonical CDP physical-key codes ("KeyA",
+    "Digit0") and the upper-case-derived virtual key code, so Chrome
+    recognises them as the same physical key a real keyboard would emit.
+    Without this, shortcuts like Cmd+A see code="a" / vk=97 and don't
+    fire — Chrome's shortcut handlers expect code="KeyA" / vk=65.
+    """
+    if key in _KEYS:
+        return _KEYS[key]
+    if len(key) != 1:
+        return (0, key, "")
+    upper = key.upper()
+    if "A" <= upper <= "Z":
+        # vk for letters is the uppercase ASCII codepoint, regardless of case.
+        return (ord(upper), f"Key{upper}", key)
+    if "0" <= key <= "9":
+        return (ord(key), f"Digit{key}", key)
+    # Punctuation / symbols — best-effort; Chrome may still recognise the
+    # shortcut from the printable text and the modifier flags alone.
+    return (ord(key), key, key)
+
+
+# Modifier bitmask: 1=Alt, 2=Ctrl, 4=Meta(Cmd), 8=Shift. Any of the first
+# three implies the keypress is a shortcut, not text input — the `char`
+# event must be suppressed and `text` left out of keyDown so Chrome doesn't
+# insert the printable form alongside firing the shortcut.
+_SHORTCUT_MODIFIERS = 0b0111
+
+
 def press_key(key, modifiers=0):
     """Modifiers bitfield: 1=Alt, 2=Ctrl, 4=Meta(Cmd), 8=Shift.
-    Special keys (Enter, Tab, Arrow*, Backspace, etc.) carry their virtual key codes
-    so listeners checking e.keyCode / e.key all fire."""
-    vk, code, text = _KEYS.get(key, (ord(key[0]) if len(key) == 1 else 0, key, key if len(key) == 1 else ""))
+
+    For shortcut presses (any of Alt/Ctrl/Meta), `text` is omitted from
+    keyDown and no `char` event is dispatched — otherwise Chrome inserts
+    the printable form (e.g. typing "a" instead of triggering Cmd+A).
+    Letter/digit keys carry canonical CDP `code` ("KeyA", "Digit0") and
+    upper-case-derived `windowsVirtualKeyCode` so `e.code`/`e.keyCode`
+    match what a real keyboard emits.
+    """
+    vk, code, text = _key_metadata(key)
+    is_shortcut = bool(modifiers & _SHORTCUT_MODIFIERS)
     base = {"key": key, "code": code, "modifiers": modifiers, "windowsVirtualKeyCode": vk, "nativeVirtualKeyCode": vk}
-    cdp("Input.dispatchKeyEvent", type="keyDown", **base, **({"text": text} if text else {}))
-    if text and len(text) == 1:
-        cdp("Input.dispatchKeyEvent", type="char", text=text, **{k: v for k, v in base.items() if k != "text"})
+    keydown_extra = {"text": text} if text and not is_shortcut else {}
+    cdp("Input.dispatchKeyEvent", type="keyDown", **base, **keydown_extra)
+    if text and len(text) == 1 and not is_shortcut:
+        cdp("Input.dispatchKeyEvent", type="char", text=text, **base)
     cdp("Input.dispatchKeyEvent", type="keyUp", **base)
 
 def scroll(x, y, dy=-300, dx=0):

tests/unit/test_helpers.py

@@ -350,3 +350,130 @@ def fake_send(req):
         "session filter, the background rWS/lF pair would have updated "
         "last_activity and prevented the idle window from elapsing."
     )
+
+
+# --- press_key: shortcut and key-metadata behavior ---
+
+def _capture_press_key(key, modifiers=0):
+    """Run press_key with a fake cdp that records every dispatchKeyEvent."""
+    events = []
+
+    def fake_cdp(method, **kwargs):
+        if method == "Input.dispatchKeyEvent":
+            events.append(kwargs)
+        return {}
+
+    with patch("browser_harness.helpers.cdp", side_effect=fake_cdp):
+        helpers.press_key(key, modifiers=modifiers)
+    return events
+
+
+def test_press_key_letter_uses_canonical_code_and_vk():
+    """Letters must resolve to CDP physical-key codes (KeyA) and the
+    upper-case ASCII codepoint as vk (65). Without that, Chrome's shortcut
+    handlers don't recognise the press as the same physical key a real
+    keyboard would emit."""
+    events = _capture_press_key("a")
+    keydown = next(e for e in events if e["type"] == "keyDown")
+    assert keydown["code"] == "KeyA"
+    assert keydown["windowsVirtualKeyCode"] == 65
+    assert keydown["nativeVirtualKeyCode"] == 65
+    # Uppercase letter still maps to KeyA / 65 — vk is case-insensitive.
+    events = _capture_press_key("Z")
+    keydown = next(e for e in events if e["type"] == "keyDown")
+    assert keydown["code"] == "KeyZ"
+    assert keydown["windowsVirtualKeyCode"] == 90
+
+
+def test_press_key_digit_uses_canonical_code_and_vk():
+    events = _capture_press_key("5")
+    keydown = next(e for e in events if e["type"] == "keyDown")
+    assert keydown["code"] == "Digit5"
+    assert keydown["windowsVirtualKeyCode"] == ord("5")  # 53
+
+
+def test_press_key_special_key_uses_kkeys_table():
+    """Pre-existing behavior preserved: special keys carry their virtual
+    key codes from _KEYS so listeners checking e.keyCode/e.key still fire."""
+    events = _capture_press_key("Enter")
+    keydown = next(e for e in events if e["type"] == "keyDown")
+    assert keydown["code"] == "Enter"
+    assert keydown["windowsVirtualKeyCode"] == 13
+    assert keydown.get("text") == "\r"  # Enter inserts a CR
+
+
+def test_press_key_no_modifiers_emits_text_and_char_event():
+    """For ordinary text input, keyDown carries `text` and a `char` event
+    fires — Chrome inserts the printable character into the focused input."""
+    events = _capture_press_key("a")
+    keydown = next(e for e in events if e["type"] == "keyDown")
+    assert keydown.get("text") == "a"
+    chars = [e for e in events if e["type"] == "char"]
+    assert len(chars) == 1
+    assert chars[0].get("text") == "a"
+
+
+def test_press_key_with_ctrl_suppresses_text_and_char():
+    """Holding Ctrl makes the press a shortcut, not text input. The keyDown
+    must omit `text` and no `char` event must fire — otherwise Chrome
+    inserts the printable letter alongside firing the shortcut handler."""
+    events = _capture_press_key("a", modifiers=2)  # Ctrl
+    keydown = next(e for e in events if e["type"] == "keyDown")
+    assert "text" not in keydown, (
+        f"keyDown must NOT carry text when Ctrl is held — Chrome would "
+        f"insert the letter. Got: {keydown}"
+    )
+    assert keydown["modifiers"] == 2
+    chars = [e for e in events if e["type"] == "char"]
+    assert chars == [], f"expected zero char events with Ctrl held, got: {chars}"
+
+
+def test_press_key_with_meta_suppresses_text_and_char():
+    """Same as Ctrl, but for Cmd on macOS (modifier 4 = Meta)."""
+    events = _capture_press_key("s", modifiers=4)  # Cmd
+    keydown = next(e for e in events if e["type"] == "keyDown")
+    assert "text" not in keydown
+    assert keydown["modifiers"] == 4
+    assert keydown["code"] == "KeyS"
+    assert keydown["windowsVirtualKeyCode"] == ord("S")
+    assert not any(e["type"] == "char" for e in events)
+
+
+def test_press_key_with_alt_suppresses_text_and_char():
+    """Alt-shortcuts also should not insert text (e.g. browser/menu shortcuts
+    on Linux/Windows)."""
+    events = _capture_press_key("f", modifiers=1)  # Alt
+    keydown = next(e for e in events if e["type"] == "keyDown")
+    assert "text" not in keydown
+    assert not any(e["type"] == "char" for e in events)
+
+
+def test_press_key_shift_alone_still_emits_text_and_char():
+    """Shift alone is text input (Shift+A still types whatever the caller
+    asked for). Only Alt/Ctrl/Meta turn the press into a shortcut."""
+    events = _capture_press_key("A", modifiers=8)  # Shift
+    keydown = next(e for e in events if e["type"] == "keyDown")
+    assert keydown.get("text") == "A"
+    assert keydown["modifiers"] == 8
+    chars = [e for e in events if e["type"] == "char"]
+    assert len(chars) == 1
+    assert chars[0].get("text") == "A"
+
+
+def test_press_key_ctrl_shift_combo_still_suppresses():
+    """Multi-modifier shortcuts (e.g. Ctrl+Shift+P for command palette) must
+    also suppress text/char — Ctrl wins over Shift's text-input semantics."""
+    events = _capture_press_key("p", modifiers=2 | 8)  # Ctrl+Shift
+    keydown = next(e for e in events if e["type"] == "keyDown")
+    assert "text" not in keydown
+    assert keydown["modifiers"] == 10
+    assert not any(e["type"] == "char" for e in events)
+
+
+def test_press_key_emits_keyup_with_consistent_metadata():
+    """Every press emits a matching keyUp regardless of modifiers."""
+    events = _capture_press_key("a", modifiers=2)
+    keyup = next(e for e in events if e["type"] == "keyUp")
+    assert keyup["code"] == "KeyA"
+    assert keyup["windowsVirtualKeyCode"] == 65
+    assert keyup["modifiers"] == 2