Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mouse modes #2316

Merged
merged 4 commits into from
Jul 16, 2019
Merged

Mouse modes #2316

merged 4 commits into from
Jul 16, 2019

Conversation

jerch
Copy link
Member

@jerch jerch commented Jul 14, 2019

This PR tries to deal with mouse report issues. We already had several issues/discussions about mouse reports, still this field was never covered with proper testing. Our current mouse report code is quite unreadable / unmaintainable. Furthermore we have known issues on protocol handling as described in #1962.

TODO:

  • establish proper eval / test base to compare with other emulators
  • identify wanted/unwanted mouse modes in xterm.js (as close as possible to xterm)
  • fix/implement & test (prolly with separate issues/PRs) --> Restructure mouse handling #2319

@jerch jerch added the work-in-progress Do not merge label Jul 14, 2019
@jerch jerch self-assigned this Jul 14, 2019
@jerch
Copy link
Member Author

jerch commented Jul 15, 2019

Did several tests with the test_modes.js script across emulators with following results:

  • xterm: works as described in the docs, wheel events are not reported in DECSET 9, several modifiers cannot be tested (as noted in the docs), X10 default coordinates drop to 0 after 223, move events are debounced at grid level (reported only for cell changes, not for movements within a cell), pressing multiple buttons reports the lowest one pressed in move
  • gnome-terminal: some differences to xterm - wheel works in all protocols, all modifiers work (distinguishs whether mouse or modifier was pressed first), move events are not debounced
  • urxvt: wheel works in all protocols, modifiers do not work (prolly rebound), no support for SGR reports, pressing multiple buttons reports the last one pressed in move
  • konsole: no DECSET 9, suppresses reports beyond 223 in default report mode, move events only on press, modifiers do not work (maybe KDE only, tested under xfce), pressing multiple buttons reports the highest one pressed in move
  • mlterm: no DECSET 9, modifiers do not work
  • xterm.js: several things are not in line with most of the others:
    • broken X10 default mode: We currently cannot report sizes beyond 95 due the UTF8 restriction on our transport. This is serious as its likely to affect many apps at bigger terminals.
    • offset in URXVT mode: We have a +1 offset in row/col reports in this mode. Imho easy to fix.
    • release event gets fired immediately after press in DECSET 1000 (does not wait for mouseup?), works correctly in DECSET 1002/1003
    • no move events in DECSET 1003 if no button is pressed
    • multiple buttons pressed always reports the last one on move
    • move events are not debounced
    • no modifiers are reported
    • bug: we dont reset mouse tracking on RIS
    • bug: normal mouse actions are not cancelled properly (middle click still inserts data from clipboard under X11)

xterm.js is not as bad as I though in the beginning, still we are the ugly kid here. Furthermore the other emulators all differ slightly here and there, thus we should identify which things are worth to keep/adopt. In general we should stay close to the others to not break with assumptions on application side.

Proposal:

  • fix X10 default mode being able to address row/col up to 223, drop to 0 beyond that (such a fix is a bit more involved since we have to decide how to transmit non UTF8 stuff)
  • fix offset in URXVT report mode
  • fix press/release events for DECSET 1000
  • allow move events in DECSET 1003 if no button was pressed
  • debounce move events on grid level
  • stick with "last button pressed" reports on move events (seems more logical to me)
  • include modifiers in mouse reports (we eval them anyway)
  • fix bugs like reset and event cancellation
  • consider removing 1005 (see X10 mouse protocol is capped at 127 #1962 (comment))
  • dont bother with DEC locator (see below)

Also tested the focusIn/focusOut event (DECSET 1004). This is not strictly bound to mouse reports (works also when mouse tracking is deactivated, no clue why xterm lists it there) with these results:

  • xterm: no report on activation, after that proper events for focus changes
  • gnome-terminal: initial report on activation, after that proper events for focus changes
  • urxvt: no support
  • konsole: no report on activation, after that proper events for focus changes
  • mlterm: no report on activation, after that proper events for focus changes
  • xterm.js: no report on activation, after that proper events for focus changes on the terminal widget

Imho only gnome-terminal does the right thing here with an initial report so applications know right from the start whether the emulator has focus or not.

Next tests will deal with DEC locator modes.

@jerch jerch added area/mouse-support type/bug Something is misbehaving type/enhancement Features or improvements to existing features labels Jul 15, 2019
@jerch
Copy link
Member Author

jerch commented Jul 16, 2019

Did some tests with DEC locator modes and well, beside xterm no other terminal seems to support those atm (mlterm does, but does it wrong). Furthermore the protocol is really awkward, only press&release can be set to autotracking, some states are reported doubled in different flags, everything else has to be done by polling. Maybe I have overlooked something here. The interesting bit of the locator is the fact that it can report pixel metrics.

Imho we should not mess with DEC locator at this point and remove the stub code we have (it was never activated by DECELR or attached to DECRQLP).

If ppl are interested in a pixel oriented mouse tracking I think this needs more thinking and some iterations over terminal-wg. Maybe @egmontkob has some thoughts on this?

@egmontkob
Copy link

egmontkob commented Jul 16, 2019

I have no idea what "DEC locator mode" is :)

A couple of mouse protocol enhancement ideas were collected at VTE 755183, out of which half cell reporting is my personal favorite.

As per the spirit of my comments at Terminal-wg: simple image display, I'm basically against a protocol that introduces the concept of "pixel". However, fractional cells are okay, and given that ANSI-like escape sequences don't use decimal fractional digits, one way to encode them is to use a fixed denominator (e.g. 1000 or 1024 or lcm([1..n]) for some n), while another way is sure to report the exact numerator and denominator, the latter one then actually leaking the pixel size. (The first approach would also leak lot of information about the possible pixel sizes.) Therefore it's not that bad, after all.

I'm curious whether there are nice use cases where more fine-grained reporting rather than just "half cells" is desired. Maybe eighth cells for dragging a character-based scrollbar which is potentially painted using the U+258x "eighth" glyphs.

As usual, the hard part would be to agree on a reasonable subset of feature improvements (to satisfy real-life demands), rather than overengineering and including even the kitchen sink.

@egmontkob
Copy link

Imho only gnome-terminal does the right thing here with an initial report so applications know right from the start whether the emulator has focus or not.

Yes this was intentional, see in the commit log message.

@egmontkob
Copy link

For existing mouse protocols and implementation differences, @textshell could provide more information. See also VTE 93.

@jerch
Copy link
Member Author

jerch commented Jul 16, 2019

@egmontkob Ah thx for the pointers.

With DEC locator I mean what xterm calls those locator modes:

  • activated by DECELR CSI Ps ; Pu ' z
  • modified by DECSLE CSI Pm ' {
  • polled by DECRQLP CSI Ps ' |, which answers with DECLRP CSI Pe ; Pb ; Pr ; Pc ; Pp & w

Really dont want to bother with those atm (even had to recompile xterm to get it, seems Ubuntu does not bother either).

I'm curious whether there are nice use cases where more fine-grained reporting rather than just "half cells" is desired.

Yes I second your stance against pixels in terminals, still ppl might cry for this once image support lands in bigger scale (yes I see the link to that image thread there 😈). And tbh the current mouse protocols are all flawed, the button encoding is crap in all of them. Dont want to discuss a new protocol here, but I think something like SGR with better button encoding would come handy. And such a new thingy could easily change on the reported metrics by some additional flag.

Yes this was intentional, see in the commit log message.

Thats what I thought 👍, guess we will adopt that behavior.

For existing mouse protocols and implementation differences, @textshell could provide more information. See also VTE 93.

Thx for the pointer, will take this into account. On VTE93 btw - our code intends to only report modifiers for 1000+ modes (but does not work for some other reason). Guess time to look into xterm sources for some comments about the intent here. Or maybe report them always, no clue yet.

@jerch jerch removed the work-in-progress Do not merge label Jul 16, 2019
@jerch jerch merged commit d55fb1e into xtermjs:master Jul 16, 2019
@Tyriar Tyriar added this to the 4.0.0 milestone Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/mouse-support type/bug Something is misbehaving type/enhancement Features or improvements to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants