Merge PR #6746 from happy5318/fix/mobile-enter-newline
This commit is contained in:
nesquena-hermes committed
commit
10faa69155
6 files changed
+877
-13
No files matched your search
@@ -0,0 +1,566 @@
|
||||
"""Regression: mobile soft-keyboard Enter must insert a newline, not send.
|
||||
|
||||
This module covers the three production changes in this PR, each with a test
|
||||
that provably fails when only that change is reverted:
|
||||
|
||||
A. ``_isTouchOnlyDevice()`` supplements media-query touch detection for phones.
|
||||
Several iOS Safari builds report ``(any-pointer:fine)`` as **true** on a
|
||||
plain iPhone (Apple Pencil / pointer-emulation heuristics), so the upstream
|
||||
``matchMedia('(pointer:coarse)') && !_hasFinePointerCoexisting()`` test
|
||||
evaluates false on exactly the devices it is meant to detect, and plain
|
||||
Enter sends instead of inserting a newline. Phone UAs bypass that unreliable
|
||||
signal, while tablets still honor it so attached hardware keyboards retain
|
||||
desktop Enter-to-send behavior.
|
||||
|
||||
B. ``window._sendKey`` is initialised synchronously from ``localStorage``
|
||||
before the composer handler is registered. Without it, on a slow network the
|
||||
handler runs while ``window._sendKey`` is still undefined, so a user who
|
||||
configured ``shift+enter`` has plain Enter send the message.
|
||||
|
||||
C. The composer textarea carries ``enterkeyhint="enter"`` so the iOS software
|
||||
keyboard renders a return key rather than a "Go"/"Send" key.
|
||||
|
||||
The behavioural tests execute the **real** helpers and composer handler
|
||||
extracted verbatim from ``static/boot.js`` — reverting the production code
|
||||
changes what these tests run.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
try:
|
||||
from playwright.sync_api import sync_playwright
|
||||
except Exception: # pragma: no cover - playwright optional in some envs
|
||||
sync_playwright = None
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
BOOT_JS = (REPO / "static" / "boot.js").read_text(encoding="utf-8")
|
||||
INDEX_HTML = (REPO / "static" / "index.html").read_text(encoding="utf-8")
|
||||
|
||||
IPHONE_UA = (
|
||||
"Mozilla/5.0 (iPhone; CPU iPhone OS 17_5 like Mac OS X) "
|
||||
"AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.5 Mobile/15E148 Safari/604.1"
|
||||
)
|
||||
IPAD_UA = (
|
||||
"Mozilla/5.0 (iPad; CPU OS 17_5 like Mac OS X) "
|
||||
"AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.5 Mobile/15E148 Safari/604.1"
|
||||
)
|
||||
ANDROID_TABLET_UA = (
|
||||
"Mozilla/5.0 (Linux; Android 14; SM-X910) AppleWebKit/537.36 "
|
||||
"(KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36"
|
||||
)
|
||||
TOUCH_MAC_UA = (
|
||||
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) "
|
||||
"AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.5 Safari/605.1.15"
|
||||
)
|
||||
|
||||
|
||||
def _require_playwright():
|
||||
if sync_playwright is None:
|
||||
pytest.skip("playwright is unavailable; run `playwright install chromium`")
|
||||
return sync_playwright
|
||||
|
||||
|
||||
# ─── Source extraction ────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def _extract_function(name: str, source: str) -> str:
|
||||
"""Extract a top-level ``function name(...){...}`` with brace matching."""
|
||||
start = source.index(f"function {name}(")
|
||||
brace_start = source.index("{", start)
|
||||
depth = 0
|
||||
for i in range(brace_start, len(source)):
|
||||
c = source[i]
|
||||
if c == "{":
|
||||
depth += 1
|
||||
elif c == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return source[start : i + 1]
|
||||
raise AssertionError(f"could not extract function {name}")
|
||||
|
||||
|
||||
def _extract_composer_handler() -> str:
|
||||
"""Extract the real ``$('msg').addEventListener('keydown', …)`` call."""
|
||||
signature = "$('msg').addEventListener('keydown',e=>"
|
||||
start = BOOT_JS.index(signature)
|
||||
brace = BOOT_JS.index("{", start)
|
||||
depth = 0
|
||||
for idx in range(brace, len(BOOT_JS)):
|
||||
char = BOOT_JS[idx]
|
||||
if char == "{":
|
||||
depth += 1
|
||||
elif char == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
end = BOOT_JS.index(");", idx) + 2
|
||||
return BOOT_JS[start:end]
|
||||
raise AssertionError("could not extract composer keydown listener")
|
||||
|
||||
|
||||
def _extract_send_key_init() -> str:
|
||||
"""Extract the synchronous ``window._sendKey`` bootstrap from boot.js.
|
||||
|
||||
Returns an empty string when the line is absent, so reverting change B
|
||||
removes it from the harness too (which is what makes the test bite).
|
||||
"""
|
||||
match = re.search(
|
||||
r"^try\{\s*window\._sendKey=localStorage\.getItem\([^\n]*$",
|
||||
BOOT_JS,
|
||||
re.MULTILINE,
|
||||
)
|
||||
return match.group(0) if match else ""
|
||||
|
||||
|
||||
def _available_helpers() -> str:
|
||||
"""Extract the touch/IME/numpad helpers the composer handler may call.
|
||||
|
||||
``_isTouchOnlyDevice`` is this PR's addition and ``_hasFinePointerCoexisting``
|
||||
is the upstream helper; include whichever exist so the harness stays valid
|
||||
with the production change reverted.
|
||||
"""
|
||||
names = (
|
||||
"_isImeEnter",
|
||||
"_isNumpadEnter",
|
||||
"_hasFinePointerCoexisting",
|
||||
"_isTouchOnlyDevice",
|
||||
)
|
||||
out = []
|
||||
for name in names:
|
||||
if f"function {name}(" in BOOT_JS:
|
||||
out.append(_extract_function(name, BOOT_JS))
|
||||
return "\n".join(out)
|
||||
|
||||
|
||||
HARNESS_HTML = """
|
||||
<!doctype html>
|
||||
<html><head><meta charset="utf-8"></head>
|
||||
<body>
|
||||
<textarea id="msg" rows="1"></textarea>
|
||||
<script>
|
||||
var _imeComposing = false;
|
||||
function $(id) { return document.getElementById(id); }
|
||||
|
||||
// --- Real helpers extracted from boot.js ---
|
||||
__HELPERS__
|
||||
|
||||
// --- Real synchronous _sendKey bootstrap extracted from boot.js ---
|
||||
// Empty when that line has been removed from production code.
|
||||
__SENDKEY_INIT__
|
||||
|
||||
window.__sendCount = 0;
|
||||
window.__defaultPrevented = 0;
|
||||
window.send = function() { window.__sendCount++; };
|
||||
var send = window.send;
|
||||
|
||||
// --- Real composer handler extracted from boot.js ---
|
||||
__HANDLER__
|
||||
|
||||
// Observe whether the handler suppressed the browser's default newline.
|
||||
document.getElementById('msg').addEventListener('keydown', function(e){
|
||||
if (e.defaultPrevented) window.__defaultPrevented++;
|
||||
});
|
||||
</script>
|
||||
</body></html>
|
||||
"""
|
||||
|
||||
|
||||
def _build_harness_html() -> str:
|
||||
return (
|
||||
HARNESS_HTML.replace("__HELPERS__", _available_helpers())
|
||||
.replace("__SENDKEY_INIT__", _extract_send_key_init())
|
||||
.replace("__HANDLER__", _extract_composer_handler())
|
||||
)
|
||||
|
||||
|
||||
def _write_harness() -> Path:
|
||||
tmp = Path(tempfile.mkstemp(suffix=".html")[1])
|
||||
tmp.write_text(_build_harness_html(), encoding="utf-8")
|
||||
return tmp
|
||||
|
||||
|
||||
# ─── Media-query shim ─────────────────────────────────────────────────────
|
||||
|
||||
# Reproduces the iOS Safari quirk: a touch-primary phone that ALSO reports
|
||||
# (any-pointer:fine) as true. Installed before any page script runs.
|
||||
IOS_SAFARI_MATCHMEDIA_QUIRK = """
|
||||
(() => {
|
||||
const real = window.matchMedia.bind(window);
|
||||
window.matchMedia = (q) => {
|
||||
if (q === '(pointer:coarse)') return {matches: true, media: q};
|
||||
if (q === '(any-pointer:fine)') return {matches: true, media: q};
|
||||
return real(q);
|
||||
};
|
||||
})();
|
||||
"""
|
||||
|
||||
# A plain desktop: no coarse pointer, fine pointer present.
|
||||
DESKTOP_MATCHMEDIA = """
|
||||
(() => {
|
||||
const real = window.matchMedia.bind(window);
|
||||
window.matchMedia = (q) => {
|
||||
if (q === '(pointer:coarse)') return {matches: false, media: q};
|
||||
if (q === '(any-pointer:fine)') return {matches: true, media: q};
|
||||
return real(q);
|
||||
};
|
||||
})();
|
||||
"""
|
||||
|
||||
# A tablet / iPadOS-desktop UA with only a software keyboard: a coarse pointer
|
||||
# is present (touch screen) but no fine pointer (no attached hardware
|
||||
# keyboard or trackpad). This is the path that the production change leaves
|
||||
# alone — these devices must keep the mobile "Enter inserts newline" behavior.
|
||||
TABLET_NO_FINE_POINTER_MATCHMEDIA = """
|
||||
(() => {
|
||||
const real = window.matchMedia.bind(window);
|
||||
window.matchMedia = (q) => {
|
||||
if (q === '(pointer:coarse)') return {matches: true, media: q};
|
||||
if (q === '(any-pointer:fine)') return {matches: false, media: q};
|
||||
return real(q);
|
||||
};
|
||||
})();
|
||||
"""
|
||||
|
||||
|
||||
def _press_enter(page, *, shift=False, ctrl=False, meta=False, numpad=False):
|
||||
page.evaluate(
|
||||
"""
|
||||
({shift, ctrl, meta, code, location}) => {
|
||||
const el = document.getElementById('msg');
|
||||
el.focus();
|
||||
el.dispatchEvent(new KeyboardEvent('keydown', {
|
||||
key: 'Enter', code, keyCode: 13, which: 13, location,
|
||||
shiftKey: shift, ctrlKey: ctrl, metaKey: meta,
|
||||
bubbles: true, cancelable: true,
|
||||
}));
|
||||
}
|
||||
""",
|
||||
{
|
||||
"shift": shift,
|
||||
"ctrl": ctrl,
|
||||
"meta": meta,
|
||||
"code": "NumpadEnter" if numpad else "Enter",
|
||||
"location": 3 if numpad else 0,
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
def _send_count(page) -> int:
|
||||
return page.evaluate("__sendCount")
|
||||
|
||||
|
||||
# ─── Change A: UA-first touch detection ───────────────────────────────────
|
||||
|
||||
|
||||
def test_iphone_reporting_fine_pointer_still_treats_enter_as_newline():
|
||||
"""Change A. iPhone UA + ``(any-pointer:fine)`` true → Enter must NOT send.
|
||||
|
||||
Reverting to ``matchMedia('(pointer:coarse)') && !_hasFinePointerCoexisting()``
|
||||
makes ``_mobileDefault`` false here, so plain Enter sends and this fails.
|
||||
"""
|
||||
playwright = _require_playwright()
|
||||
harness = _write_harness()
|
||||
with playwright() as p:
|
||||
browser = p.chromium.launch(args=["--no-sandbox", "--disable-dev-shm-usage"])
|
||||
ctx = browser.new_context(
|
||||
user_agent=IPHONE_UA,
|
||||
viewport={"width": 390, "height": 844},
|
||||
has_touch=True,
|
||||
is_mobile=True,
|
||||
)
|
||||
ctx.add_init_script(IOS_SAFARI_MATCHMEDIA_QUIRK)
|
||||
page = ctx.new_page()
|
||||
page.goto(harness.as_uri())
|
||||
page.wait_for_load_state("domcontentloaded")
|
||||
|
||||
# Sanity: the quirk is actually in place, otherwise the test is vacuous.
|
||||
assert page.evaluate("matchMedia('(pointer:coarse)').matches") is True
|
||||
assert page.evaluate("matchMedia('(any-pointer:fine)').matches") is True
|
||||
|
||||
_press_enter(page)
|
||||
sends = _send_count(page)
|
||||
prevented = page.evaluate("__defaultPrevented")
|
||||
browser.close()
|
||||
|
||||
assert sends == 0, (
|
||||
"plain Enter sent the message on an iPhone that reports (any-pointer:fine); "
|
||||
"the soft keyboard's return key must insert a newline"
|
||||
)
|
||||
assert prevented == 0, "Enter should fall through so the browser inserts the newline"
|
||||
|
||||
|
||||
def test_android_phone_treats_enter_as_newline():
|
||||
"""Change A. Android UA → Enter must NOT send, regardless of media queries."""
|
||||
playwright = _require_playwright()
|
||||
harness = _write_harness()
|
||||
android_ua = (
|
||||
"Mozilla/5.0 (Linux; Android 14; Pixel 8) AppleWebKit/537.36 "
|
||||
"(KHTML, like Gecko) Chrome/126.0.0.0 Mobile Safari/537.36"
|
||||
)
|
||||
with playwright() as p:
|
||||
browser = p.chromium.launch(args=["--no-sandbox", "--disable-dev-shm-usage"])
|
||||
ctx = browser.new_context(
|
||||
user_agent=android_ua,
|
||||
viewport={"width": 412, "height": 915},
|
||||
has_touch=True,
|
||||
is_mobile=True,
|
||||
)
|
||||
ctx.add_init_script(IOS_SAFARI_MATCHMEDIA_QUIRK)
|
||||
page = ctx.new_page()
|
||||
page.goto(harness.as_uri())
|
||||
page.wait_for_load_state("domcontentloaded")
|
||||
_press_enter(page)
|
||||
sends = _send_count(page)
|
||||
browser.close()
|
||||
assert sends == 0, "plain Enter must insert a newline on an Android phone"
|
||||
|
||||
|
||||
def _run_enter_case(
|
||||
playwright,
|
||||
harness: Path,
|
||||
*,
|
||||
user_agent: str,
|
||||
max_touch_points: int = 0,
|
||||
matchmedia_shim: str = IOS_SAFARI_MATCHMEDIA_QUIRK,
|
||||
expect_coarse: bool = True,
|
||||
expect_fine: bool = True,
|
||||
) -> tuple[int, int]:
|
||||
with playwright() as p:
|
||||
browser = p.chromium.launch(args=["--no-sandbox", "--disable-dev-shm-usage"])
|
||||
ctx = browser.new_context(
|
||||
user_agent=user_agent,
|
||||
viewport={"width": 1024, "height": 768},
|
||||
has_touch=True,
|
||||
)
|
||||
ctx.add_init_script(
|
||||
f"Object.defineProperty(navigator, 'maxTouchPoints', {{value: {max_touch_points}}});"
|
||||
)
|
||||
ctx.add_init_script(matchmedia_shim)
|
||||
page = ctx.new_page()
|
||||
page.goto(harness.as_uri())
|
||||
page.wait_for_load_state("domcontentloaded")
|
||||
# Sanity: the shim is actually in place, otherwise the test would be vacuous.
|
||||
assert page.evaluate("matchMedia('(pointer:coarse)').matches") is expect_coarse
|
||||
assert page.evaluate("matchMedia('(any-pointer:fine)').matches") is expect_fine
|
||||
_press_enter(page)
|
||||
sends = _send_count(page)
|
||||
prevented = page.evaluate("__defaultPrevented")
|
||||
browser.close()
|
||||
return sends, prevented
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("user_agent", "max_touch_points", "device"),
|
||||
[
|
||||
(IPAD_UA, 5, "iPad with Magic Keyboard"),
|
||||
(ANDROID_TABLET_UA, 5, "Android tablet with hardware keyboard"),
|
||||
(TOUCH_MAC_UA, 5, "touch Macintosh/iPadOS desktop UA"),
|
||||
],
|
||||
)
|
||||
def test_tablet_with_fine_pointer_keeps_enter_to_send(
|
||||
user_agent: str,
|
||||
max_touch_points: int,
|
||||
device: str,
|
||||
):
|
||||
"""Tablet/touch-desktop UA + fine pointer must retain desktop Enter=send."""
|
||||
playwright = _require_playwright()
|
||||
harness = _write_harness()
|
||||
sends, prevented = _run_enter_case(
|
||||
playwright,
|
||||
harness,
|
||||
user_agent=user_agent,
|
||||
max_touch_points=max_touch_points,
|
||||
)
|
||||
|
||||
assert sends == 1, f"plain Enter must send on {device}"
|
||||
assert prevented == 1, f"plain Enter must be consumed after sending on {device}"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("user_agent", "max_touch_points", "device"),
|
||||
[
|
||||
(IPAD_UA, 5, "iPad with software keyboard only"),
|
||||
(ANDROID_TABLET_UA, 5, "Android tablet with software keyboard only"),
|
||||
(TOUCH_MAC_UA, 5, "touch Macintosh/iPadOS desktop with software keyboard only"),
|
||||
],
|
||||
)
|
||||
def test_tablet_without_fine_pointer_inserts_newline(
|
||||
user_agent: str,
|
||||
max_touch_points: int,
|
||||
device: str,
|
||||
):
|
||||
"""Control for the tablet-with-fine-pointer test.
|
||||
|
||||
A tablet / iPadOS-desktop UA reporting only a coarse pointer (no fine
|
||||
pointer — i.e. no attached hardware keyboard) must keep the mobile
|
||||
"plain Enter inserts a newline" behavior, because the production change
|
||||
only short-circuits phone UAs and otherwise still falls back to the
|
||||
media-query test ``(pointer:coarse) && !(any-pointer:fine)``.
|
||||
|
||||
Reverting the change to a phone-only match would break this test, and a
|
||||
tablet shortcut that always returns true would invert
|
||||
``test_tablet_with_fine_pointer_keeps_enter_to_send``.
|
||||
"""
|
||||
playwright = _require_playwright()
|
||||
harness = _write_harness()
|
||||
sends, prevented = _run_enter_case(
|
||||
playwright,
|
||||
harness,
|
||||
user_agent=user_agent,
|
||||
max_touch_points=max_touch_points,
|
||||
matchmedia_shim=TABLET_NO_FINE_POINTER_MATCHMEDIA,
|
||||
expect_fine=False,
|
||||
)
|
||||
|
||||
assert sends == 0, (
|
||||
f"plain Enter must insert a newline on {device} (no fine pointer "
|
||||
"is exposed, so the media-query path should still classify it as touch-only)"
|
||||
)
|
||||
assert prevented == 0, (
|
||||
f"Enter must fall through so the browser inserts the newline on {device}"
|
||||
)
|
||||
|
||||
|
||||
def test_mobile_ctrl_enter_still_sends():
|
||||
"""Change A must not cost mobile users a way to send from the keyboard."""
|
||||
playwright = _require_playwright()
|
||||
harness = _write_harness()
|
||||
with playwright() as p:
|
||||
browser = p.chromium.launch(args=["--no-sandbox", "--disable-dev-shm-usage"])
|
||||
ctx = browser.new_context(
|
||||
user_agent=IPHONE_UA,
|
||||
viewport={"width": 390, "height": 844},
|
||||
has_touch=True,
|
||||
is_mobile=True,
|
||||
)
|
||||
ctx.add_init_script(IOS_SAFARI_MATCHMEDIA_QUIRK)
|
||||
page = ctx.new_page()
|
||||
page.goto(harness.as_uri())
|
||||
page.wait_for_load_state("domcontentloaded")
|
||||
|
||||
_press_enter(page, ctrl=True)
|
||||
ctrl_sends = _send_count(page)
|
||||
_press_enter(page, numpad=True)
|
||||
total_sends = _send_count(page)
|
||||
browser.close()
|
||||
|
||||
assert ctrl_sends == 1, "Ctrl+Enter must still send on mobile"
|
||||
assert total_sends == 2, "Numpad Enter must still send on mobile"
|
||||
|
||||
|
||||
def test_desktop_enter_still_sends():
|
||||
"""Change A must not alter desktop behaviour."""
|
||||
playwright = _require_playwright()
|
||||
harness = _write_harness()
|
||||
with playwright() as p:
|
||||
browser = p.chromium.launch(args=["--no-sandbox", "--disable-dev-shm-usage"])
|
||||
ctx = browser.new_context()
|
||||
ctx.add_init_script(DESKTOP_MATCHMEDIA)
|
||||
page = ctx.new_page()
|
||||
page.goto(harness.as_uri())
|
||||
page.wait_for_load_state("domcontentloaded")
|
||||
_press_enter(page)
|
||||
sends = _send_count(page)
|
||||
browser.close()
|
||||
assert sends == 1, "plain Enter must still send on a desktop browser"
|
||||
|
||||
|
||||
# ─── Change B: synchronous _sendKey bootstrap ─────────────────────────────
|
||||
|
||||
|
||||
def test_stored_shift_enter_preference_applies_before_settings_fetch():
|
||||
"""Change B. A stored ``shift+enter`` must hold on the very first keypress.
|
||||
|
||||
Reverting the synchronous ``window._sendKey`` bootstrap leaves the value
|
||||
undefined while the async ``/api/settings`` fetch is still in flight, so the
|
||||
handler falls into the default branch and plain Enter sends.
|
||||
"""
|
||||
playwright = _require_playwright()
|
||||
harness = _write_harness()
|
||||
with playwright() as p:
|
||||
browser = p.chromium.launch(args=["--no-sandbox", "--disable-dev-shm-usage"])
|
||||
ctx = browser.new_context()
|
||||
ctx.add_init_script(DESKTOP_MATCHMEDIA)
|
||||
# Seed the preference the way a returning user's browser would have it,
|
||||
# before any page script runs.
|
||||
ctx.add_init_script(
|
||||
"try{localStorage.setItem('hermes-pref-send_key','shift+enter');}catch(e){}"
|
||||
)
|
||||
page = ctx.new_page()
|
||||
page.goto(harness.as_uri())
|
||||
page.wait_for_load_state("domcontentloaded")
|
||||
|
||||
assert (
|
||||
page.evaluate("localStorage.getItem('hermes-pref-send_key')")
|
||||
== "shift+enter"
|
||||
), "harness failed to seed the preference; test would be vacuous"
|
||||
|
||||
_press_enter(page)
|
||||
plain_sends = _send_count(page)
|
||||
_press_enter(page, shift=True)
|
||||
after_shift = _send_count(page)
|
||||
browser.close()
|
||||
|
||||
assert plain_sends == 0, (
|
||||
"plain Enter sent the message even though the stored preference is "
|
||||
"shift+enter; window._sendKey was not initialised before the handler ran"
|
||||
)
|
||||
assert after_shift == 1, "Shift+Enter must send when the preference is shift+enter"
|
||||
|
||||
|
||||
def test_send_key_bootstrap_precedes_composer_handler():
|
||||
"""Change B, structural. The bootstrap must sit ABOVE the handler in boot.js.
|
||||
|
||||
Ordering is what closes the race; a bootstrap placed after the
|
||||
``addEventListener`` call would still leave the first keypress undefined.
|
||||
"""
|
||||
init = _extract_send_key_init()
|
||||
assert init, "synchronous window._sendKey bootstrap is missing from boot.js"
|
||||
handler_idx = BOOT_JS.index("$('msg').addEventListener('keydown',e=>")
|
||||
assert BOOT_JS.index(init) < handler_idx, (
|
||||
"window._sendKey bootstrap must appear before the composer keydown "
|
||||
"listener is registered"
|
||||
)
|
||||
|
||||
|
||||
# ─── Change C: enterkeyhint on the composer ───────────────────────────────
|
||||
|
||||
|
||||
def test_composer_textarea_declares_enterkeyhint():
|
||||
"""Change C, structural. iOS renders the return key from ``enterkeyhint``.
|
||||
|
||||
The rendered keycap is a native-keyboard behaviour with no DOM-observable
|
||||
effect in a headless browser, so this asserts the attribute is present and
|
||||
parsed rather than claiming to verify the keyboard itself.
|
||||
"""
|
||||
match = re.search(r"<textarea id=\"msg\"[^>]*>", INDEX_HTML)
|
||||
assert match, "composer textarea not found in index.html"
|
||||
assert 'enterkeyhint="enter"' in match.group(0), (
|
||||
"composer textarea must declare enterkeyhint=\"enter\" so the iOS "
|
||||
"software keyboard shows a return key instead of Go/Send"
|
||||
)
|
||||
|
||||
|
||||
def test_enterkeyhint_reaches_the_dom():
|
||||
"""Change C. The attribute must survive parsing as the DOM property."""
|
||||
playwright = _require_playwright()
|
||||
match = re.search(r"<textarea id=\"msg\"[^>]*>", INDEX_HTML)
|
||||
assert match, "composer textarea not found in index.html"
|
||||
snippet = (
|
||||
"<!doctype html><html><head><meta charset='utf-8'></head><body>"
|
||||
+ match.group(0)
|
||||
+ "</textarea></body></html>"
|
||||
)
|
||||
tmp = Path(tempfile.mkstemp(suffix=".html")[1])
|
||||
tmp.write_text(snippet, encoding="utf-8")
|
||||
with playwright() as p:
|
||||
browser = p.chromium.launch(args=["--no-sandbox", "--disable-dev-shm-usage"])
|
||||
page = browser.new_page()
|
||||
page.goto(tmp.as_uri())
|
||||
hint = page.evaluate("document.getElementById('msg').enterKeyHint")
|
||||
browser.close()
|
||||
assert hint == "enter", f"enterKeyHint resolved to {hint!r}, expected 'enter'"
|
||||
@@ -1697,15 +1697,6 @@ def test_mobile_enter_newline_does_not_depend_on_viewport_heuristic():
|
||||
"the viewport height-delta probe must no longer gate the mobile Enter override"
|
||||
|
||||
|
||||
def test_mobile_enter_newline_respects_hardware_keyboard_on_touch_devices():
|
||||
"""Touch devices with a co-existing fine pointer (hardware keyboard) keep desktop Enter=send."""
|
||||
boot_js = (REPO / "static" / "boot.js").read_text(encoding="utf-8")
|
||||
assert "any-pointer:fine" in boot_js, \
|
||||
"boot.js must use any-pointer:fine to detect a co-existing hardware keyboard/trackpad"
|
||||
assert "!_hasFinePointerCoexisting()" in boot_js, \
|
||||
"mobile Enter newline override must skip touch devices that also expose a fine pointer"
|
||||
|
||||
|
||||
def test_mobile_enter_newline_only_overrides_enter_default():
|
||||
"""Mobile newline override must only apply when _sendKey is the default 'enter'."""
|
||||
boot_js = (REPO / "static" / "boot.js").read_text(encoding="utf-8")
|
||||
|
||||
@@ -0,0 +1,287 @@
|
||||
"""Regression: send-key preference changes take effect without page reload.
|
||||
|
||||
This test catches the bug where an inline capture-phase interceptor in
|
||||
index.html snapshotted ``_sendKey`` into a closure at page-parse time.
|
||||
When the user later changed the setting (which updates ``window._sendKey``
|
||||
via panels.js), the stale closure still called ``stopImmediatePropagation()``
|
||||
and blocked the newly configured shortcut from reaching the boot.js
|
||||
composer handler.
|
||||
|
||||
The fix removes the inline interceptor entirely — boot.js already
|
||||
initialises ``window._sendKey`` synchronously and its composer handler
|
||||
reads the live value on every event.
|
||||
|
||||
This Playwright test loads a minimal HTML page that includes the real
|
||||
boot.js composer keydown handler (extracted verbatim), simulates a
|
||||
preference change at runtime, and verifies the correct behaviour for
|
||||
each send-key mode — including the transition from one mode to another
|
||||
without a page reload.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
try:
|
||||
from playwright.sync_api import sync_playwright
|
||||
except Exception:
|
||||
sync_playwright = None
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
BOOT_JS = (REPO / "static" / "boot.js").read_text(encoding="utf-8")
|
||||
INDEX_HTML = (REPO / "static" / "index.html").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _require_playwright():
|
||||
if sync_playwright is None:
|
||||
pytest.skip("playwright is unavailable; run `playwright install chromium`")
|
||||
return sync_playwright
|
||||
|
||||
|
||||
def _extract_composer_handler() -> str:
|
||||
"""Extract the real ``$('msg').addEventListener('keydown', …)`` handler
|
||||
from boot.js, including its full body and the closing ``);``."""
|
||||
signature = "$('msg').addEventListener('keydown',e=>"
|
||||
start = BOOT_JS.index(signature)
|
||||
brace = BOOT_JS.index("{", start)
|
||||
depth = 0
|
||||
for idx in range(brace, len(BOOT_JS)):
|
||||
char = BOOT_JS[idx]
|
||||
if char == "{":
|
||||
depth += 1
|
||||
elif char == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
# Include the closing ``);`` that completes the addEventListener call.
|
||||
end = BOOT_JS.index(");", idx) + 2
|
||||
return BOOT_JS[start:end]
|
||||
raise AssertionError("could not extract composer keydown listener")
|
||||
|
||||
|
||||
def _extract_function(name: str, source: str) -> str:
|
||||
"""Extract a function by name, matching braces properly."""
|
||||
pattern = f"function {name}("
|
||||
start = source.index(pattern)
|
||||
brace_start = source.index("{", start)
|
||||
depth = 0
|
||||
for i in range(brace_start, len(source)):
|
||||
c = source[i]
|
||||
if c == "{":
|
||||
depth += 1
|
||||
elif c == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return source[start : i + 1]
|
||||
raise ValueError(f"Could not extract function {name}")
|
||||
|
||||
|
||||
def _extract_helper_functions() -> str:
|
||||
"""Extract the helper functions the composer handler depends on."""
|
||||
names = ("_isImeEnter", "_isNumpadEnter", "_isTouchOnlyDevice")
|
||||
return "\n".join(_extract_function(n, BOOT_JS) for n in names)
|
||||
|
||||
|
||||
# Minimal HTML that loads the real handler logic.
|
||||
HARNESS_HTML = """
|
||||
<!doctype html>
|
||||
<html><head><meta charset="utf-8"></head>
|
||||
<body>
|
||||
<textarea id="msg" rows="1"></textarea>
|
||||
<script>
|
||||
// _imeComposing is a module-level flag in boot.js that _isImeEnter references.
|
||||
var _imeComposing = false;
|
||||
// Minimal $ shim (boot.js uses $('msg') === document.getElementById('msg'))
|
||||
function $(id) { return document.getElementById(id); }
|
||||
|
||||
// --- Real helper functions extracted from boot.js ---
|
||||
__HELPERS__
|
||||
|
||||
// --- Synchronous _sendKey init (from boot.js line ~2362) ---
|
||||
try { window._sendKey = localStorage.getItem('hermes-pref-send_key') || 'enter'; } catch(_) { window._sendKey = 'enter'; }
|
||||
|
||||
// --- Real composer handler extracted from boot.js ---
|
||||
// send() stub: count invocations instead of doing real work.
|
||||
window.__sendCount = 0;
|
||||
window.send = function() { window.__sendCount++; };
|
||||
var send = window.send; // local alias so extracted handler's send() resolves
|
||||
|
||||
__HANDLER__
|
||||
</script>
|
||||
</body></html>
|
||||
"""
|
||||
|
||||
|
||||
def _build_harness_html() -> str:
|
||||
return HARNESS_HTML.replace("__HELPERS__", _extract_helper_functions()).replace(
|
||||
"__HANDLER__", _extract_composer_handler()
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def page():
|
||||
_require_playwright()
|
||||
with sync_playwright() as p:
|
||||
browser = p.chromium.launch(args=["--no-sandbox", "--disable-dev-shm-usage"])
|
||||
context = browser.new_context()
|
||||
pg = context.new_page()
|
||||
html = _build_harness_html()
|
||||
import tempfile
|
||||
|
||||
tmp = Path(tempfile.mkstemp(suffix=".html")[1])
|
||||
tmp.write_text(html, encoding="utf-8")
|
||||
pg.goto(tmp.as_uri())
|
||||
pg.wait_for_load_state("domcontentloaded")
|
||||
yield pg
|
||||
browser.close()
|
||||
|
||||
|
||||
def _set_send_key(page, mode: str):
|
||||
"""Simulate panels.js applying a saved settings preference at runtime."""
|
||||
page.evaluate(
|
||||
f"""() => {{
|
||||
localStorage.setItem('hermes-pref-send_key', '{mode}');
|
||||
window._sendKey = '{mode}';
|
||||
}}"""
|
||||
)
|
||||
|
||||
|
||||
def _press_enter(page, *, shift=False, ctrl=False, meta=False, numpad=False):
|
||||
"""Dispatch a real Enter keydown on #msg with optional modifiers."""
|
||||
code = "NumpadEnter" if numpad else "Enter"
|
||||
location = 3 if numpad else 0
|
||||
page.evaluate(
|
||||
"""
|
||||
({shift, ctrl, meta, code, location}) => {
|
||||
const el = document.getElementById('msg');
|
||||
el.focus();
|
||||
el.dispatchEvent(new KeyboardEvent('keydown', {
|
||||
key: 'Enter',
|
||||
code: code,
|
||||
keyCode: 13,
|
||||
which: 13,
|
||||
location: location,
|
||||
shiftKey: shift,
|
||||
ctrlKey: ctrl,
|
||||
metaKey: meta,
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
}));
|
||||
}
|
||||
""",
|
||||
{"shift": shift, "ctrl": ctrl, "meta": meta, "code": code, "location": location},
|
||||
)
|
||||
|
||||
|
||||
def _send_count(page) -> int:
|
||||
return page.evaluate("__sendCount")
|
||||
|
||||
|
||||
# ─── Static guard ─────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_no_inline_interceptor_remains():
|
||||
"""The stale-snapshot interceptor must be gone from index.html."""
|
||||
assert "stopImmediatePropagation" not in INDEX_HTML, (
|
||||
"index.html still contains a capture-phase interceptor with "
|
||||
"stopImmediatePropagation — this reintroduces the stale _sendKey bug"
|
||||
)
|
||||
|
||||
|
||||
# ─── Behavioural tests ────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_enter_mode_plain_enter_sends(page):
|
||||
_set_send_key(page, "enter")
|
||||
_press_enter(page)
|
||||
assert _send_count(page) == 1
|
||||
|
||||
|
||||
def test_enter_mode_shift_enter_does_not_send(page):
|
||||
_set_send_key(page, "enter")
|
||||
_press_enter(page, shift=True)
|
||||
assert _send_count(page) == 0
|
||||
|
||||
|
||||
def test_shift_enter_mode_plain_enter_does_not_send(page):
|
||||
_set_send_key(page, "shift+enter")
|
||||
_press_enter(page)
|
||||
assert _send_count(page) == 0
|
||||
|
||||
|
||||
def test_shift_enter_mode_shift_enter_sends(page):
|
||||
_set_send_key(page, "shift+enter")
|
||||
_press_enter(page, shift=True)
|
||||
assert _send_count(page) == 1
|
||||
|
||||
|
||||
def test_ctrl_enter_mode_plain_enter_does_not_send(page):
|
||||
_set_send_key(page, "ctrl+enter")
|
||||
_press_enter(page)
|
||||
assert _send_count(page) == 0
|
||||
|
||||
|
||||
def test_ctrl_enter_mode_ctrl_enter_sends(page):
|
||||
_set_send_key(page, "ctrl+enter")
|
||||
_press_enter(page, ctrl=True)
|
||||
assert _send_count(page) == 1
|
||||
|
||||
|
||||
def test_ctrl_enter_mode_cmd_enter_sends(page):
|
||||
_set_send_key(page, "ctrl+enter")
|
||||
_press_enter(page, meta=True)
|
||||
assert _send_count(page) == 1
|
||||
|
||||
|
||||
# ─── Core regression: live preference change ─────────────────────────────
|
||||
|
||||
|
||||
def test_preference_change_enter_to_shift_enter_without_reload(page):
|
||||
"""The bug: interceptor snapshotted 'enter' at load time. Changing to
|
||||
'shift+enter' at runtime didn't update the closure, so Shift+Enter was
|
||||
still blocked. After the fix, the live window._sendKey is read on every
|
||||
event, so the transition takes effect immediately."""
|
||||
# Start in 'enter' mode — plain Enter sends.
|
||||
_set_send_key(page, "enter")
|
||||
_press_enter(page)
|
||||
assert _send_count(page) == 1
|
||||
|
||||
# Switch to 'shift+enter' without reloading.
|
||||
_set_send_key(page, "shift+enter")
|
||||
|
||||
# Plain Enter should no longer send.
|
||||
_press_enter(page)
|
||||
assert _send_count(page) == 1 # unchanged
|
||||
|
||||
# Shift+Enter should now send.
|
||||
_press_enter(page, shift=True)
|
||||
assert _send_count(page) == 2
|
||||
|
||||
|
||||
def test_preference_change_shift_enter_to_enter_without_reload(page):
|
||||
"""Reverse transition: 'shift+enter' → 'enter'."""
|
||||
_set_send_key(page, "shift+enter")
|
||||
_press_enter(page) # no send
|
||||
_press_enter(page, shift=True) # sends
|
||||
assert _send_count(page) == 1
|
||||
|
||||
_set_send_key(page, "enter")
|
||||
_press_enter(page) # now sends
|
||||
assert _send_count(page) == 2
|
||||
|
||||
_press_enter(page, shift=True) # no longer sends
|
||||
assert _send_count(page) == 2
|
||||
|
||||
|
||||
def test_preference_change_enter_to_ctrl_enter_without_reload(page):
|
||||
"""Transition: 'enter' → 'ctrl+enter'."""
|
||||
_set_send_key(page, "enter")
|
||||
_press_enter(page) # sends
|
||||
assert _send_count(page) == 1
|
||||
|
||||
_set_send_key(page, "ctrl+enter")
|
||||
_press_enter(page) # no longer sends
|
||||
assert _send_count(page) == 1
|
||||
|
||||
_press_enter(page, ctrl=True) # sends
|
||||
assert _send_count(page) == 2
|
||||
Reference in new issue
Block a user