fix: correct SGR mouse-wheel button codes (64/65/66/67, not 68/69/70/71) - #238
fix: correct SGR mouse-wheel button codes (64/65/66/67, not 68/69/70/71)#238smohekey wants to merge 1 commit into
Conversation
The wheel buttons were encoded as `64 + <button number>` (4/5/6/7), producing 68/69/70/71. In the X10/SGR mouse protocol the four wheel buttons are 64/65/66/67 — bit 6 set with the button number in the low two bits — so the surplus bits read as a spurious Shift modifier (`64 + 5` = 69 = wheel-down + shift). Applications that validate the button field reject the modified report as an invalid wheel event and don't scroll; lenient ones mask the modifier and happen to still work. Encode the wheel buttons as `64 + (0/1/2/3)` to match the spec and xterm, so both SGR (`ESC[<…M`) and normal-encoding (`ESC[M`) reports carry the correct code.
#15817) * fix(terminal): send SGR mouse wheel reports with the button codes apps expect xterm.dart 4.0.0 encodes the wheel buttons as 64+4..64+7 rather than 64+0..64+3, so the low bits land on the modifier field and every wheel report the terminal emits reads as wheel-with-Shift. Strict full-screen applications reject the modified event, which is why neither the mouse wheel nor the trackpad scrolls anything once the peer application takes over the alternate screen. Install a mouse handler that keeps every upstream reporting decision and only re-encodes the wheel buttons as 64..67. Non-wheel reports pass through untouched, and the emitted bytes stay identical once upstream ships the same fix, so this can be dropped without a behavior change. Upstream: TerminalStudio/xterm.dart#238 Co-Authored-By: Claude Fable 5 <[email protected]> * fix(terminal): correct the wheel report row, drop the wasted report build Address review feedback on the wheel button fix: - The X10/utf row was encoded as `32 + y + 1` while y is already 1-based, so every normal-mode report pointed one row too low and the `y > limit` guard disagreed with what it emitted. - Gate the wheel path on `mouseMode.reportScroll` and the button state instead of building and discarding a full report string from `defaultMouseHandler` on every scroll tick. This also makes the hardcoded SGR 'M' provably right, since a wheel release now returns before the report is built. - Derive the wire code as `id - 4` and drop `_wheelButtonId`, whose `default` branch was unreachable and defeated enum exhaustiveness. - Assign `mouseHandler` after construction so the `Terminal(...)` line stays untouched. Cover the utf, urxvt, null-byte overflow and click-only branches, and assert that TerminalModel actually installs the handler. Co-Authored-By: Claude Fable 5 <[email protected]> --------- Co-authored-by: Claude Fable 5 <[email protected]>
|
Independent confirmation of this PR, with measurements — I hit this in What happens
Verified on Decoding 68 the way a receiving application does: So the application is told Shift+wheel-up, not wheel-up. The legacy What should happenImpact seen in the fieldWe ship xterm.dart in a Flutter terminal client. On a full-screen TUI the wheel The application received them as Shift+wheel, which it does not bind, and never Why the current values are wrong — re: the comment on line 19The existing comment is the origin of the bug, so it is worth answering directly:
The premise is right and the arithmetic contradicts it. "4 and 5" are the X11
Buttons 1 and 2 have event codes 0 and 1. Add 64 → 64 and 65. By the same Meanwhile bit 6 (value 64) is already the flag that says "this is a wheel event",
So Normative cross-check: xterm.jsxterm.dart is a port of xterm.js, and xterm.js gets this right. From const enum Modifiers { SHIFT = 4, ALT = 8, CTRL = 16 }
function eventCode(e: ICoreMouseEvent, isSGR: boolean): number {
let code = (e.ctrl ? Modifiers.CTRL : 0) | (e.shift ? Modifiers.SHIFT : 0) | (e.alt ? Modifiers.ALT : 0);
if (e.button === CoreMouseButton.WHEEL) {
code |= 64;
code |= e.action; // <-- the direction goes in the LOW bits
} else {
...with ( export const enum CoreMouseAction {
UP = 0, // buttons, wheel
DOWN = 1, // buttons, wheel
LEFT = 2, // wheel only
RIGHT = 3, // wheel only
MOVE = 32 // buttons only
}
Minimal reproducible examplePure Dart — no widgets, no Flutter engine.
name: wheelrepro
environment:
sdk: '>=3.0.0 <4.0.0'
dependencies:
xterm: 4.0.0
import 'package:xterm/core.dart';
String vis(String s) => s.replaceAll('\x1b', 'ESC ');
void main() {
final out = <String>[];
final term = Terminal()..onOutput = out.add;
// What any full-screen TUI does: mouse tracking (DECSET 1000)
// + SGR extended coordinates (DECSET 1006).
term.write('\x1b[?1000h\x1b[?1006h');
const pos = CellOffset(30, 12); // reported 1-based as 31;13
for (final b in [
TerminalMouseButton.wheelUp,
TerminalMouseButton.wheelDown,
TerminalMouseButton.wheelLeft,
TerminalMouseButton.wheelRight,
]) {
out.clear();
term.mouseInput(b, TerminalMouseButtonState.down, pos);
print('${b.name.padRight(11)} id=${b.id.toString().padLeft(3)} -> ${vis(out.join())}');
}
final id = TerminalMouseButton.wheelUp.id;
print('\ndecode of $id: wheel(64)=${id & 64 != 0} SHIFT(4)=${id & 4 != 0} '
'META(8)=${id & 8 != 0} CTRL(16)=${id & 16 != 0} low2=${id & 3}');
}Actual (xterm 4.0.0, and master Expected, and what this PR produces — I applied the diff to a local copy of Swap The patchOne value per line, exactly as this PR has it: --- a/lib/src/core/mouse/button.dart
+++ b/lib/src/core/mouse/button.dart
@@
- wheelUp(id: 64 + 4, isWheel: true),
+ wheelUp(id: 64 + 0, isWheel: true),
- wheelDown(id: 64 + 5, isWheel: true),
+ wheelDown(id: 64 + 1, isWheel: true),
- wheelLeft(id: 64 + 6, isWheel: true),
+ wheelLeft(id: 64 + 2, isWheel: true),
- wheelRight(id: 64 + 7, isWheel: true),
+ wheelRight(id: 64 + 3, isWheel: true),Because Regression checkThe only other place
Environment
Happy to open a companion issue, add a regression test under |
Problem
TerminalMouseButtonencodes the wheel buttons with the wrong values:In the X10/SGR mouse protocol the four wheel buttons are 64/65/66/67: bit 6 set (the high-button/"wheel" bit, +64) with the button number carried in the low two bits. The enum adds the raw button number (4/5/6/7) on top of 64 instead of transposing it into the low bits, giving 68/69/70/71.
Those extra bits are the modifier bits of the encoding:
68 = 64 + 4is wheel-up with Shift,69is wheel-down with Shift, and so on. So every wheel report the terminal emits carries a spurious Shift modifier.Impact
Applications that check the button field reject the report as a modified/invalid wheel event and don't scroll. (More lenient apps mask the modifier bit and still scroll, which is why this often goes unnoticed.) On the alternate screen,
TerminalScrollGestureHandlerforwards wheel events throughmouseInput, so a strict full-screen TUI never scrolls from the wheel.Fix
Encode the wheel buttons as
64 + (0/1/2/3)— bit 6 plus the button number in the low two bits — matching the SGR spec and xterm.Verified against an app that enables SGR mouse mode (
?1006) with any-event tracking (?1003): a wheel-down previously emittedESC[<69;…Mand did nothing; it now emitsESC[<65;…Mand the app scrolls.ESC[M(normal-encoding) reports are corrected the same way.