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

Visual panels mode sometimes doesn't restore terminal modes correctly #2216

Closed
segevfiner opened this issue Jan 17, 2022 · 11 comments · Fixed by #2251
Closed

Visual panels mode sometimes doesn't restore terminal modes correctly #2216

segevfiner opened this issue Jan 17, 2022 · 11 comments · Fixed by #2251

Comments

@segevfiner
Copy link
Contributor

segevfiner commented Jan 17, 2022

Work environment

Questions Answers
OS/arch/bits (mandatory) Windows 10.0.19044.0 x64
File format of the file you reverse (mandatory) PE
Architecture/bits of the file (mandatory) x64
rizin -v full output, not truncated (mandatory) rizin 0.3.4 @ windows-x86-64 commit: 2373c53, build: Fri 01/14/2022__18:38:44.79

Expected behavior

On exiting and entering visual panels mode (v), the terminal modes re restored correctly, so scrollback, mouse wheel interaction and such work correctly.

Actual behavior

On the first launch of a terminal (Windows Terminal running pwsh 7.2.1) and entering (v) and exiting (q) visual panels mode, the terminal modes are not restored correctly and scroll back doesn't work and the mouse wheel enters escape sequences into the input line instead of being interpreted.

Steps to reproduce the behavior

  1. Open a new Windows Terminal (1.11.3471.0 and pwsh 7.2.1).
  2. Start rizin on any binary of your choice.
  3. Enter visual panels mode (v).
  4. Exit visual panels mode (q).
  5. Try and use the moues scroll wheel, scroll the terminal, etc.

Workaround: Entering visual mode (V) and exiting seems to fix this.

Additional Logs, screenshots, source code, configuration dump, ...

rizin-bug

@ret2libc
Copy link
Member

Thanks for the well done report. BTW, how did you create those gif? :)

@segevfiner
Copy link
Contributor Author

Thanks for the well done report. BTW, how did you create those gif? :)

https://www.cockos.com/licecap/

@segevfiner
Copy link
Contributor Author

segevfiner commented Jan 24, 2022

This is most likely caused by improper saving/restoring of the terminal mouse mode by the panels mode using calls like

rz_cons_enable_mouse(false);

visual mode also messes with this setting which is likely why opening and closing it fixes this.

@ret2libc
Copy link
Member

Thanks for digging a bit more into this! I didn't have time for this yet. If you feel like it, we of course appreciate PR as well; :D

@segevfiner
Copy link
Contributor Author

Thanks for digging a bit more into this! I didn't have time for this yet. If you feel like it, we of course appreciate PR as well; :D

I will have to dig further in to figure out what the correct mouse mode setting should be first. Don't want to just blindly change stuff 😝

@segevfiner
Copy link
Contributor Author

segevfiner commented Jan 24, 2022

Looking into rz_cons_enable_mouse, it seems the escape sequences used are fscked up.

Documented here: https://invisible-island.net/xterm/ctlseqs/ctlseqs.html.

\x1b[?1000;1006;1015h Sets modes 1000, 1006, and 1015 (DECSET)
"\x1b[?1001r" Restore saved mode 1001 (XTRESTORE), but it was never saved anywhere to begin with? This doesn't seem to make any sense.
"\x1b[?1000l" Reset mode 1000 (DECRST). But what about 1006 and 1015?

And the commented out code there isn't really boding that well... Maybe something that got messed up during a merge?

Combinations that do make sense:
"\x1b[?1000;1006;1015h" "\x1b[?1001s" & "\x1b[?1000;1006;1015l" "\x1b[?1001r"
"\x1b[?1000;1001;1006;1015h" & "\x1b[?1000;1001;1006;1015l"
Or just:
"\x1b[?1000;1006;1015h" & "\x1b[?1000;1006;1015l"

(Not sure if actually needed to save and restore mode 1001 or just set/rst it, terminal escapes are kind of arcane)

I tried changing to "\x1b[?1000;1006;1015h" & "\x1b[?1000;1006;1015l", and it seems to have fixed the problem, I'm not sure about mode 1001 (SET_VT200_HIGHLIGHT_MOUSE, highlight tracking), and note that setting modes wrongly could mess up input as some modes requires the program to cooperate, that is, send some escape sequences in response to the events or the terminal will stop working properly.

@segevfiner
Copy link
Contributor Author

segevfiner commented Jan 25, 2022

@ret2libc I'm not seeing rizin sending \x1b[...T (CSI Ps ; Ps ; Ps ; Ps ; Ps T) which is used by highlight tracking (It basically allows a TUI/curses style program to control mouse highlighting so you can select stuff with the mouse inside widgets and the like), so it is likely the correct pair is "\x1b[?1000;1006;1015h" & "\x1b[?1000;1006;1015l". Should I make a PR with this change?

@wargio
Copy link
Member

wargio commented Jan 26, 2022

can you verify that these changes does not affect also the prompt and powershell terminals?

@segevfiner
Copy link
Contributor Author

@wargio In what way?

@wargio
Copy link
Member

wargio commented Jan 26, 2022

just do the same test on the other 2 terminals.

segevfiner added a commit to segevfiner/rizin that referenced this issue Jan 26, 2022
@segevfiner
Copy link
Contributor Author

Manually tested on Widows pwsh 7.2.1 and cmd running on both wt.exe (Windows Terminal) and classic conhost on Windows 10.0.19044.0 and seems to work fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants