X10 mouse protocol is capped at 127

[jerch]

Currently the X10 mouse coord handling is capped at 127, I guess this is due to UTF8 problems with higher values. Still the protocol officially encodes up to position 223 (charCode 255) in a binary fashion.

To support those non UTF8 values we need to find a way to get the data to the backend (either by letting non valid UTF8 sequences passing through or by wrapping the binary data into some other encoding, e.g. base64).

[jocutajar]

Trying X10 mouse tracking with UTF-8 extended support in VS Code, I get a very confusing result.

Reproduce:

1. Switch on SET_EXT_MODE_MOUSE (1005): \x1b[?1005h.
2. Press and hold ctrl and left button and drag to the top left corner.
3. Capture the mouse tracking sequence.

Actual result:
CSI M (195) (130) (194) (128) ! !

Expected result (as in xterm):
CSI M P ! !

Why is it a problem?
Mouse tracking is broken. Basic mouse events are OK but anything more advanced is an oops... Try also Ctrl + wheel up/down.

Insights:

1. The terminal does not need to switch to UTF-8 as the mouse event parameters can be expressed in plain ASCII
2. Even if UTF-8 switch was in order, the functions expects 3 UTF-8 characters, 4 were given. It makes no sense to me. What are the two utf-8 characters supposed to represent?
3. There are other cases where mouse events are way off the mark. A complete mouse tracking code review would be in order.

[jerch]

@jocutajar We currently have no tests for the different mouse protocols, its likely that the other protocol extensions are broken to some degree as well. I think this went unnoticed due to the fact that only a few apps use the mouse at all and some protocol versions are very uncommon.
Writing some tests based on other terminals would be a good starting point to get a better coverage rolling. Also the current code is hard to read and prolly hard to fix, feel free to come up with a better solution.

[egmontkob]

@jocutajar SET_EXT_MODE_MOUSE (1005) is a badly designed extension (see e.g. https://midnight-commander.org/ticket/2662), every application should be using 1006 instead.

[jerch]

@jocutajar Actually I cannot repro the weird additional bytes in UTF8 mode in the demo (the encoding itself works fine). Did you test this with winpty or conpty under windows by any chance? This pretty much looks like some layer in between did additional faulty encoding on top.
We still have alot to fix in mouse field, see #2316.

@egmontkob Yeah SGR mode seems best (as it even can report which button got released). I still think we gonna keep on the others as well to not break with random apps.

[egmontkob]

I still think we gonna keep on the others as well to not break with random apps.

FYI: VTE / gnome-terminal doesn't support the UTF-8 1005 mode, and we haven't received a report about it. We should also drop the urxvt 1015 mode, at least to get feedback on existing users, if any. Just filed VTE 155.

[egmontkob]

I think the origin of the 1005 mode is the following:

Emacs used to have a bug, similar to the one this issue is about. It did the UTF-8 decoding and mouse decoding in the opposite stacking order than desired, that is, expected UTF-8 encoded Unicode codepoints to represent the coordinates. Then xterm was adjusted to this behavior (subject to the new 1005 switch, and limited to 2-byte values) to please Emacs and overcome the 223 limit. This latter IMO shouldn't have happened, it should have been realized that the protocol is not backwards compatible and thus rejected from xterm, forcing to go straight for a better solution. Anyway, it happened, and we can't change the past.

As far as I know, Emacs has supported the SGR 1006 extension for a long time now. I'm not aware of any other app ever using the 2-byte UTF-8 1005 protocol, but my knowledge is obviously not exhaustive.

[jerch]

Lol true, the 1015 mode is even more harmful due to the ambiguity. Hmm ok, since you never had an issue without 1005, then we should consider to remove it as well.

[jocutajar]

@jocutajar SET_EXT_MODE_MOUSE (1005) is a badly designed extension [..] every application should be using 1006 instead.

I see. Using the SGR mode 1006 I get better results. Still some button and modifier combinations are broken. Btw., I was not able to switch off the 1015 mode once enabled. Restart is needed.

I think no support of 1005 and 1015 would be better than broken support. Or one should be able to switch easily. We can then send something like this and get the best support possible:

/// A sequence of escape codes to enable SOME terminal mouse support.
const ENTER_MOUSE_SEQUENCE: &'static str =
"\x1b[?1000h\x1b[?1002h\x1b[?1003h\x1b[?1004h\x1b[?10015h\x1b[?1005h\x1b[?1006h";

My app is then able to handle all three if they are not broken.

[jocutajar]

Did you test this with winpty or conpty under windows by any chance?

No, I'm on Debian linux, running the VS code editor and the embedded terminal with xterm.js.

[jerch]

I see. Using the SGR mode 1006 I get better results. Still some button and modifier combinations are broken. Btw., I was not able to switch off the 1015 mode once enabled. Restart is needed.

You can try with #2323 (just pushed it), it cleans up the protocol/encoding nightmare we had before. The rework will not be finished before part 2 of that, also its likely we cannot support all combinations of mouse buttons and actions, since some are supressed at browser engine level (have not yet looked into this).

[egmontkob]

/// A sequence of escape codes to enable SOME terminal mouse support.
const ENTER_MOUSE_SEQUENCE: &'static str =
"\x1b[?1000h\x1b[?1002h\x1b[?1003h\x1b[?1004h\x1b[?10015h\x1b[?1005h\x1b[?1006h";

Asking to enable 1005 without knowing for sure that the terminal supports it is potentially outright harmful, see https://midnight-commander.org/ticket/2662#comment:1. Also I doubt there's any terminal that supports 1005 but not 1006 (except for some old versions from those times when these extensions were created). So really, really, really let's forget 1005.

The only terminal I know that supports 1015 but not 1006 is urxvt. Whether you care about one single terminal emulator enough to parse another protocol in your app or not is up to you; I wouldn't bother. It should be fixed in urxvt and not in hundreds of apps out there.

All in all, my recommendation is: Enable 1006 from your app, be able to parse 1006 and the legacy default protocol (9/1000/1002), and stop here.

[jocutajar]

You can try with #2323

@jerch thanks for the quick action, though I'm not familiar with testing bleeding edge xterm.js. If you can, give me some simple steps to follow. Please consider I'm only using xterm.js as a user as part of VS code and testing my app in it as I develop it, beside xterm and lxterminal.

[jerch]

@jocutajar
If you have nodejs and gcc installed you can grab it as follows:

git clone https://github.com/jerch/xterm.js.git
cd xterm.js
git checkout mouse_services
npm install
npm start

then point your browser to localhost:3000. There you have a shell where you can test it with your app.

Testing this with an independent app is very valuable for us, since you might trigger edge cases we still not cover. Note that the issue with coords > 95 in X10 default mode is still not resolved in the PR (poking around mouse stuff is not enough to fix this due to transport encoding problem).

Edit: The pulled data during npm install are self contained, thus you can simply delete the repo folder afterwards to cleanup things.

[jocutajar]

Ok after installing npm from backports and making space on my stuffed disk I tried e5b28d:

• Ctrl+PressLeftButton
  • lxterminal: \e [ M 0 ! !
  • xterm: -menu-
  • xterm.js e5b28d: \e [ < 64 ; 1 ; 1 M
• Ctrl+ReleaseLeftButton
  • lxterminal: \e [ M 3 ! !
  • xterm: -menu-
  • xterm.js e5b28d: \e [ < 64 ; 1 ; 1 m
• Alt+PressLeftButton
  • lxterminal: \e [ M ( ! !
  • xterm: -nothing-
  • xterm.js e5b28d: \e [ < 0 ; 1 ; 1 M
• Alt+ReleaseLeftButton
  • lxterminal: \e [ M + ! !
  • xterm: -nothing-
  • xterm.js e5b28d: \e [ < 0 ; 1 ; 1 m
• Ctrl+WheelUp
  • lxterminal: \e [ M p ! !
  • xterm: \e [ < 80 ; 1 ; 1 M
  • xterm.js e5b28d: \e [ < 128 ; 1 ; 1 M
• Ctrl+WheelDown
  • lxterminal: \e [ M q ! !
  • xterm: \e [ < 81 ; 1 ; 1 M
  • xterm.js e5b28d: \e [ < 129 ; 1 ; 1 M
• mouse movement (\e[?1003h)
  • lxterminal: \e [ M C ! !
  • xterm: \e [ < 35 ; 1 ; 1 M
  • xterm.js e5b28d: -none-
• Ctrl+DragLeftButton (\e[?1002h)
  • lxterminal: \e [ M P ! !
  • xterm: \e [ < 51 ; 1 ; 1 M
  • xterm.js e5b28d: \e [ < 96; 1 ; 1 M

Oh I've just realized this issue is about the cap on coordinates and I'm talking about buttons. This is probably for another issue. With the SGR mode, I can go up to 300 column/row easily.

So if you look at the numbers, xterm.js has the modifier combinations reported incorrectly. It also doesn't seem to support "any" mouse movement (1003).

[egmontkob]

Apparently your lxterminal doesn't switch to the 1006 mode when your other terminals do. This means your lxterminal is compiled against GTK2 and thus the 8 year old and no longer maintained VTE 0.28. This version should no longer be used. If you're referring to it, comparing the behavior, providing it as some kind of reference etc., you should compile your lxterminal against GTK3 and thus a reasonably fresh VTE (0.50-ish or never; 0.56 being the latest stable right now).

[jerch]

Your reported button codes are weird though, I get quite different values here with SGR encoding:

• Ctrl+PressLeftButton: CSI < 16 ; 1 ; 1 M
• Ctrl+ReleaseLeftButton: CSI < 16 ; 1 ; 1 m
• Alt+PressLeftButton: cannot be tested (catched by different action)
• Alt+ReleaseLeftButton: CSI < 8 ; 1 ; 1 m
• Ctrl+WheelUp: CSI < 80 ; 1 ; 1 M
• Ctrl+WheelDown: CSI < 81 ; 1 ; 1 M
• mouse movement (\e[?1003h): CSI < 35 ; 1 ; 1 M
• Ctrl+DragLeftButton (\e[?1002h): CSI < 48 ; 1 ; 1 M

Are you sure you tested against the last commit? Your values look more like current master. You have to checkout branch mouse_services, then recompile the sources (for simplicity just remove folder out and call npm run watch).

[jerch]

To finally support this:

• establish an onBinary data event to report non UTF8 conformant data as raw bytes (bytestring)
• integrate in attach addon