fix(desktop): back/forward via keyboard chords, mouse X1/X2 buttons, and swipe gestures - #3778
Conversation
|
thanks for covering mouse back too. one ask — keep editable targets from eating cmd-left/right so line-start editing still works |
Review ask on block#3778: editable targets must keep receiving ⌘←/⌘→ so line-start/line-end editing works. This was already the behavior — only ⌘[ / ⌘] (and Alt+arrows on Windows/Linux) match, and everything else falls through to the editor untouched — but nothing pinned it. Add an explicit regression test so a future ⌘-arrow binding can't slip in without seeing this contract. Co-authored-by: Matheus Iser <matheusiser@squareup.com> Signed-off-by: Matheus Iser <matheusiser@squareup.com>
@Chessing234 good call, but this PR doesn't mess with those command shortcuts. I added a test and validated in the dev env. |
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 synthesized findings from two independent agent reviews at head 04917b4 (full write-up in our review channel). the keyboard-chord half of this PR is clean — chord matching in a pure helper with modifier-exclusivity tests, the editable-target guard removal is correctly argued, and the e2e regression test reproduces the exact bug. the inline comments below are about the native gesture layer and the unverified Windows path: one blocking gap on the macOS swipe event class, one pre-merge Windows verification ask, and three nits.
- Guard the DOM X1/X2 mouseup listener with isTauri(): in a plain browser context (dev server, e2e) the browser itself navigates on these buttons, so the listener would double-fire navigation. - Emit mouse-nav to the main window (emit_to) instead of broadcasting to all windows, so navigation stays scoped if multi-window lands. - Rescope the NSEventType::Swipe module docs: it covers page-swipe gestures synthesized by mouse drivers (the verified hardware); stock Apple trackpad / Magic Mouse swipes arrive as phased scroll-wheel events and are deferred to a ScrollWheel + trackSwipeEventWithOptions: follow-up. - Document why the matching OtherMouseDown press intentionally passes through while the release is swallowed. Co-authored-by: Matheus Iser <matheusiser@squareup.com> Signed-off-by: Matheus Iser <matheusiser@squareup.com>
Copy of windows-canary.yml adapted to build from this temporary branch on the fork (require-main guard removed, repository guard retargeted). Never intended for block/buzz; branch will be deleted after testing. Co-authored-by: Matheus Iser <matheusiser@squareup.com> Signed-off-by: Matheus Iser <matheusiser@squareup.com>
There was a problem hiding this comment.
🤖 I verified the changes at 08abf603a — the macOS scope is now accurately documented with the ScrollWheel + trackSwipeEventWithOptions: path deferred to a follow-up, the Windows mouseup handler removal checks out (no dangling references to matchBackForwardMouseButton or the listener), both emits are scoped to the main window, and the test counts line up. Also ran the desktop unit suite, the Tauri Rust suite, and typecheck locally at this exact commit — all green.
Approving. Note the full CI workflows haven't run on this head yet, so we'll still need those green before merge.
The global back/forward shortcuts (⌘[ / ⌘] on macOS, Alt+←/→ on Windows/Linux) were guarded by an editable-target check, but the message composer autofocuses on every channel switch (useComposerAutofocus), so in steady state focus almost always lives in a contenteditable and the chords were silently swallowed — the feature read as nonexistent. Match the chord regardless of the event target. Safe because neither chord carries text-editing semantics and the TipTap editor binds no conflicting shortcuts; preventDefault keeps the chord out of the editor. Chord matching is extracted into a pure helper (backForwardChords.ts) with unit tests, and navigation.spec.ts gains the missing regression case: pressing the chord while the composer is focused. Verified the new e2e test fails against the previous implementation. Fixes block#3775 Signed-off-by: npub1yvnq5equak5errqpku8stskushny9wsvt0fc2ywcpwt79yslwaqswe7tse <23260a641ceda9918c01b70f05c2dc85e642ba0c5bd38511d80b97e2921f7741@buzz.block.builderlab.xyz> Co-authored-by: Matheus Iser <matheusiser@squareup.com> Signed-off-by: Matheus Iser <matheusiser@squareup.com>
… gestures Browsers navigate on the mouse back/forward buttons; Buzz did nothing. Two delivery paths are needed: - Windows/Linux: WebView2/WebKitGTK deliver X1/X2 as DOM mouse events, so a window mouseup listener maps button 3/4 to back/forward, sharing the pure-helper pattern of the keyboard chords. - macOS: WKWebView never delivers these inputs to the page (Safari handles them in the app layer), so a native NSEvent local monitor catches them and emits a mouse-nav Tauri event the frontend acts on. Two AppKit event shapes map to navigation: otherMouseUp with button 3/4, and horizontal swipe gestures (the page-swipe convention Safari follows — trackpad two-finger swipes arrive this way, and many mouse drivers synthesize the same gesture for the back/forward buttons instead of button events). The swipe path was verified end to end on hardware whose back/forward buttons produce only swipe gestures, never button-3/4 events. Unit tests cover the DOM button mapping (backForwardChords.test.mjs) and the native mapping helpers (mouse_nav.rs). Refs block#3775 Co-authored-by: Matheus Iser <matheusiser@squareup.com> Signed-off-by: Matheus Iser <matheusiser@squareup.com>
Review ask on block#3778: editable targets must keep receiving ⌘←/⌘→ so line-start/line-end editing works. This was already the behavior — only ⌘[ / ⌘] (and Alt+arrows on Windows/Linux) match, and everything else falls through to the editor untouched — but nothing pinned it. Add an explicit regression test so a future ⌘-arrow binding can't slip in without seeing this contract. Co-authored-by: Matheus Iser <matheusiser@squareup.com> Signed-off-by: Matheus Iser <matheusiser@squareup.com>
- Guard the DOM X1/X2 mouseup listener with isTauri(): in a plain browser context (dev server, e2e) the browser itself navigates on these buttons, so the listener would double-fire navigation. - Emit mouse-nav to the main window (emit_to) instead of broadcasting to all windows, so navigation stays scoped if multi-window lands. - Rescope the NSEventType::Swipe module docs: it covers page-swipe gestures synthesized by mouse drivers (the verified hardware); stock Apple trackpad / Magic Mouse swipes arrive as phased scroll-wheel events and are deferred to a ScrollWheel + trackSwipeEventWithOptions: follow-up. - Document why the matching OtherMouseDown press intentionally passes through while the release is swallowed. Co-authored-by: Matheus Iser <matheusiser@squareup.com> Signed-off-by: Matheus Iser <matheusiser@squareup.com>
WebView2 already handles back and forward mouse buttons natively, so the frontend mouseup handler risks double navigation. Keep the native macOS NSEvent path and leave other webviews to their built-in behavior. Signed-off-by: Matheus Iser <matheusiser@squareup.com>
mouse_nav is macOS-only native I/O, as is tray_menu. Grouping them under tray_menu means lib.rs no longer declares the module or calls its init(), removing the 4 lines that pushed lib.rs past the 1000-line gate. mouse_nav::init is made generic over R: Runtime so tray_menu (itself generic) can call it without a type mismatch. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
08abf60 to
6f10943
Compare
Problem
Two related gaps in global back/forward navigation. Fixes #3775.
Duplicate check: searched open PRs and issues — none found beyond #3775 (filed alongside this fix). #3078 / #3377 are next/previous-channel navigation, a different feature.
Root causes
Keyboard:
useBackForwardControls's keydown handler bailed whenever the event target was editable — butuseComposerAutofocusdeliberately focuses the message composer (a ProseMirror contenteditable) on mount and on every channel switch. In steady state focus almost always lives in the composer, so the chords were silently swallowed. Invisible to CI becausenavigation.spec.tsonly ever clicked theglobal-back/global-forwardbuttons, never pressed the keys.Mouse/swipe: on macOS, WKWebView never delivers X1/X2 button events or swipe gestures to the page (Safari handles them natively in the app layer, not in page JS), and Buzz had no native handler.
Fix
Keyboard chords (web layer)
Match the existing platform chord regardless of the event target and drop the editable-target guard:
⌘[/⌘]have no text-editing semantics in macOS text fields, and the TipTap/StarterKit editor config binds noMod-[/Mod-]shortcuts (checkeduseRichTextEditor.ts— list indentation is Tab/Shift-Tab).preventDefault()keeps the chord out of the editor — asserted in the e2e test.This matches browsers and Slack, where back/forward chords work while a text field is focused. Chord matching is extracted into a pure helper,
app/navigation/backForwardChords.ts, so it can be unit tested; behavior (bindings, modifier exclusivity,code-based matching for non-US layouts) is unchanged.macOS mouse buttons and swipe gestures (native layer)
An NSEvent local monitor in
mouse_nav.rscatches what the webview can't see and emits amouse-navTauri event to the main window (emit_to, so navigation stays scoped if multi-window ever lands) that the frontend acts on. Two AppKit event shapes map to navigation:otherMouseUpwith button 3/4 — mice whose X1/X2 buttons arrive as plain button events. These are swallowed after emitting so nothing downstream double-handles them.swipewith a horizontal delta — AppKit's page-swipe gesture (swipeWithEvent:):deltaX > 0back,deltaX < 0forward. Sent by mouse drivers that synthesize a page-swipe gesture for the back/forward buttons instead of button-3/4 events (the hardware this was verified on). Stock Apple trackpad and Magic Mouse swipes arrive as phased scroll-wheel events instead, which this PR does not handle — that path (ScrollWheel+trackSwipeEventWithOptions:, which also needs scroll-edge detection) is deferred to a follow-up. Swipes are passed through (swallowing mid-gesture events could confuse AppKit gesture tracking).The swipe path was verified end to end on hardware whose back/forward buttons emit only swipe gestures, never button-3/4 events — an instrumented event monitor confirmed the events arrive as
NSEventType::SwipewithdeltaX ±1, and navigation worked after mapping them.Tests
backForwardChords.test.mjs): supported chords, modifier exclusivity,codefallback, and preservation of line-editing shortcuts.mouse_nav.rs): button 3/4 directions, other buttons ignored, swipe delta sign → direction, zero-delta (gesture-begin) ignored.navigation.spec.ts: presses the platform chord while the composer is focused — the missing coverage. Verified it fails against the pre-fix implementation and passes with the fix.cargo test, buzz-desktop): 1888 passed / 0 failed.pnpm typecheck,biome check,pnpm check,cargo fmt --check,cargo clippy: clean (no new warnings).navigation.spec.tsis fully green.Manual test
⌘[— returns to the previous channel;⌘]— forward again. Typing[/]in the composer inserts normally.Update — 2026-07-31
Removed the redundant DOM mouse-button handler after verifying it was unnecessary. The native macOS path remains unchanged and was revalidated manually.