Skip to content

Commit 30c9c62

Browse files
committed
fix(mobile): don't open the sidebar drawer while selecting message text
On mobile, dragging to extend a text selection inside a chat message opened the left sidebar drawer. The swipe-to-open gesture in _initChatSwipeToOpenSidebar (static/js/sidebar-layout.js) armed for any horizontal drag that started anywhere over the chat content, so growing a selection to copy it was captured as an edge-swipe and popped the drawer open. Arm the gesture only from a narrow 24px strip at the screen's left/right edge, and bail whenever a text selection is active at touchstart or appears/grows during the drag. A drag that starts over a message body is now left entirely to native text selection, while a genuine edge-swipe still opens the drawer on either side and the hamburger and backdrop-close paths are unchanged. Adds tests/test_sidebar_swipe_js.py, which loads the real module under Node, dispatches synthetic touch events, and asserts a mid-content selection drag keeps the drawer closed while a true edge-swipe still opens it.
1 parent 28d27ee commit 30c9c62

2 files changed

Lines changed: 211 additions & 1 deletion

File tree

static/js/sidebar-layout.js

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,19 @@ function _initChatSwipeToOpenSidebar() {
490490
'input', 'textarea', 'select',
491491
].join(', ');
492492

493+
// Swipe-to-open arms ONLY from a narrow strip at the screen's left/right
494+
// edge. A horizontal drag that STARTS over chat content (e.g. dragging to
495+
// extend a text selection in a message body) must never be captured as an
496+
// edge-swipe — otherwise selecting text to copy yanks the drawer open.
497+
const EDGE_ZONE = 24; // px from the screen edge that arms the gesture
498+
499+
// A held/growing text selection means the user is copying text, not opening
500+
// the drawer — bail so the native selection wins.
501+
const hasTextSelection = () => {
502+
const sel = window.getSelection && window.getSelection();
503+
return !!(sel && !sel.isCollapsed && String(sel).trim());
504+
};
505+
493506
let sx = 0, sy = 0, track = false, decided = false;
494507

495508
const reset = () => { track = false; decided = false; };
@@ -516,7 +529,17 @@ function _initChatSwipeToOpenSidebar() {
516529
if (t && t.closest && t.closest(EXCLUDE)) return;
517530
// The gesture must start within the chat area itself.
518531
if (!(t && t.closest && t.closest('#chat-container'))) return;
519-
sx = e.touches[0].clientX;
532+
const startX = e.touches[0].clientX;
533+
// Arm the swipe ONLY from the narrow edge strip. A drag that starts over a
534+
// message body (mid-screen) is left alone for native text selection — the
535+
// icon rail is hidden on mobile, so #chat-container reaches the edge and a
536+
// genuine edge-swipe still lands here.
537+
const vw = window.innerWidth;
538+
if (startX > EDGE_ZONE && startX < vw - EDGE_ZONE) return;
539+
// Even from the edge strip, never hijack an in-progress selection (e.g. a
540+
// selection handle dragged up against the edge).
541+
if (hasTextSelection()) return;
542+
sx = startX;
520543
sy = e.touches[0].clientY;
521544
track = true;
522545
}, { passive: true, capture: true });
@@ -531,6 +554,9 @@ function _initChatSwipeToOpenSidebar() {
531554
if (!decided) {
532555
if (adx < 10 && ady < 10) return; // not enough travel to judge
533556
if (ady > adx) { track = false; return; } // vertical-dominant → let it scroll
557+
// A selection appeared/grew during the drag: the user is extending a
558+
// text selection, not swiping. Bail before we preventDefault or open.
559+
if (hasTextSelection()) { track = false; return; }
534560
decided = true; // locked into a horizontal swipe
535561
}
536562
// Claim the gesture from the browser so it doesn't scroll/navigate instead.

tests/test_sidebar_swipe_js.py

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
"""Regression coverage for the mobile swipe-to-open-sidebar gesture guard.
2+
3+
The mobile "swipe from the edge to open the sidebar" gesture lives in
4+
``_initChatSwipeToOpenSidebar`` in ``static/js/sidebar-layout.js``. It used to
5+
arm for any horizontal drag that started anywhere over the chat content, so
6+
dragging a finger to extend a text selection in a message (to copy it) was
7+
captured as an edge-swipe and yanked the drawer open.
8+
9+
These tests load the real browser module under Node, stub just enough of the
10+
DOM/BOM for the gesture handlers to run, dispatch synthetic ``touchstart`` /
11+
``touchmove`` events, and assert whether the drawer opened. They follow the same
12+
Node-subprocess pattern as ``test_markdown_rendering_js.py``.
13+
"""
14+
15+
import json
16+
import shutil
17+
import subprocess
18+
import textwrap
19+
from pathlib import Path
20+
21+
import pytest
22+
23+
_REPO = Path(__file__).resolve().parent.parent
24+
_HAS_NODE = shutil.which("node") is not None
25+
26+
27+
@pytest.fixture(scope="module")
28+
def node_available():
29+
if not _HAS_NODE:
30+
pytest.skip("node binary not on PATH")
31+
32+
33+
# One synthetic gesture, driven entirely by a small scenario dict:
34+
# startX - clientX of the touchstart (px from the left edge)
35+
# moveX - clientX of the touchmove
36+
# selectionAtStart - is a text selection already active at touchstart?
37+
# selectionAtMove - does a selection become/stay active by the touchmove?
38+
# The harness reports whether the swipe opened the drawer, which side, whether
39+
# the sidebar stayed hidden, and how much text remained selected.
40+
_HARNESS = r"""
41+
import fs from 'node:fs';
42+
43+
const S = JSON.parse(process.argv[1]);
44+
45+
// Records the listeners the module wires on `document`.
46+
const listeners = {};
47+
48+
function classList(initial) {
49+
const set = new Set(initial || []);
50+
return {
51+
contains: (c) => set.has(c),
52+
add: (c) => set.add(c),
53+
remove: (c) => set.delete(c),
54+
};
55+
}
56+
57+
const sidebarEl = { classList: classList(['hidden']) }; // drawer starts closed
58+
const chatContainerEl = { classList: classList([]) }; // not compare-active
59+
60+
// The selection text the page would report. Mutable so a scenario can make a
61+
// selection appear between touchstart and touchmove.
62+
let selectionText = S.selectionAtStart ? 'The quick brown fox jumps over the lazy dog.' : '';
63+
64+
let opened = false;
65+
let openedSide = null;
66+
67+
globalThis.window = {
68+
innerWidth: 390, // mobile width (< 768 mobile gate)
69+
_chipDragging: false,
70+
getSelection: () => ({
71+
isCollapsed: selectionText.length === 0,
72+
toString: () => selectionText,
73+
}),
74+
_odyOpenSidebar: (side) => {
75+
opened = true;
76+
openedSide = side;
77+
sidebarEl.classList.remove('hidden');
78+
},
79+
};
80+
81+
globalThis.getComputedStyle = () => ({ display: 'block' });
82+
83+
globalThis.document = {
84+
readyState: 'complete', // so the module auto-wires on import
85+
body: { classList: classList([]) },
86+
addEventListener: (type, handler) => {
87+
(listeners[type] = listeners[type] || []).push(handler);
88+
},
89+
getElementById: (id) => {
90+
if (id === 'sidebar') return sidebarEl;
91+
if (id === 'chat-container') return chatContainerEl;
92+
return null;
93+
},
94+
querySelectorAll: () => [], // no open modals
95+
};
96+
97+
// A touch that lands on a message body: inside #chat-container, but not inside
98+
// any of the EXCLUDE regions the gesture already ignores.
99+
const messageBody = {
100+
closest: (sel) => (sel === '#chat-container' ? chatContainerEl : null),
101+
};
102+
103+
const source = fs.readFileSync('./static/js/sidebar-layout.js', 'utf8');
104+
const moduleUrl = 'data:text/javascript;base64,' + Buffer.from(source).toString('base64');
105+
await import(moduleUrl);
106+
107+
const fire = (type, ev) => (listeners[type] || []).forEach((h) => h(ev));
108+
109+
fire('touchstart', {
110+
target: messageBody,
111+
touches: [{ clientX: S.startX, clientY: 300 }],
112+
});
113+
114+
if (S.selectionAtMove) {
115+
selectionText = 'The quick brown fox jumps over the lazy dog.';
116+
}
117+
118+
fire('touchmove', {
119+
cancelable: true,
120+
preventDefault() {},
121+
touches: [{ clientX: S.moveX, clientY: 300 }],
122+
});
123+
124+
console.log(JSON.stringify({
125+
opened,
126+
openedSide,
127+
drawerHidden: sidebarEl.classList.contains('hidden'),
128+
selectionLen: (window.getSelection().toString() || '').length,
129+
}));
130+
"""
131+
132+
133+
def _run_gesture(**scenario):
134+
result = subprocess.run(
135+
["node", "--input-type=module", "-e", textwrap.dedent(_HARNESS), json.dumps(scenario)],
136+
cwd=_REPO,
137+
capture_output=True,
138+
timeout=15,
139+
text=True,
140+
)
141+
if result.returncode != 0:
142+
raise AssertionError(f"node failed:\nSTDERR:\n{result.stderr}\nSTDOUT:\n{result.stdout}")
143+
return json.loads(result.stdout.splitlines()[-1])
144+
145+
146+
def test_drag_to_select_in_message_does_not_open_drawer(node_available):
147+
# The core bug: a horizontal drag that STARTS over a message body (to grow a
148+
# text selection) must not be captured as an edge-swipe.
149+
r = _run_gesture(startX=195, moveX=335, selectionAtStart=True, selectionAtMove=True)
150+
assert r["opened"] is False
151+
assert r["drawerHidden"] is True
152+
# The native selection is left untouched (the gesture never fought it).
153+
assert r["selectionLen"] > 0
154+
155+
156+
def test_active_selection_blocks_even_an_edge_start_swipe(node_available):
157+
# Starting right at the left edge would normally arm the gesture, but a live
158+
# selection means the user is copying text — the selection guard must refuse.
159+
r = _run_gesture(startX=8, moveX=148, selectionAtStart=True, selectionAtMove=True)
160+
assert r["opened"] is False
161+
assert r["drawerHidden"] is True
162+
163+
164+
def test_selection_growing_mid_drag_bails_out(node_available):
165+
# No selection at touchstart (so the swipe arms), but a selection appears
166+
# during the drag: the touchmove guard must bail before opening.
167+
r = _run_gesture(startX=8, moveX=148, selectionAtStart=False, selectionAtMove=True)
168+
assert r["opened"] is False
169+
assert r["drawerHidden"] is True
170+
171+
172+
def test_genuine_left_edge_swipe_still_opens_drawer(node_available):
173+
# A real edge-swipe with no selection active must still open the drawer.
174+
r = _run_gesture(startX=8, moveX=160, selectionAtStart=False, selectionAtMove=False)
175+
assert r["opened"] is True
176+
assert r["drawerHidden"] is False
177+
assert r["openedSide"] == "left"
178+
179+
180+
def test_genuine_right_edge_swipe_still_opens_drawer(node_available):
181+
# The mirror gesture from the right edge is preserved too (both sides arm).
182+
r = _run_gesture(startX=382, moveX=230, selectionAtStart=False, selectionAtMove=False)
183+
assert r["opened"] is True
184+
assert r["openedSide"] == "right"

0 commit comments

Comments
 (0)