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

SetConsoleWindowInfo doesn't scroll visible region in Terminal #10191

Open
malxau opened this issue May 26, 2021 · 11 comments
Open

SetConsoleWindowInfo doesn't scroll visible region in Terminal #10191

malxau opened this issue May 26, 2021 · 11 comments
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal.
Milestone

Comments

@malxau
Copy link
Contributor

malxau commented May 26, 2021

Windows Terminal version (or Windows build number)

1.9.1445.0

Other Software

Yori (but only as a means to disable quickedit and call ReadConsoleInput, you might have something better)

Steps to reproduce

  1. Ensure quickedit is disabled, so mouse events are processed by the application.
  2. Attempt to scroll the viewport with the mouse wheel.

Expected Behavior

In conhost, using the mouse wheel when quickedit is disabled still moves the viewport transparently to the application.

This means that any TUI application wanting to implement mouse wheel logic itself is essentially forced to use CreateConsoleScreenBuffer/SetConsoleActiveScreenBuffer to ensure that the viewport has nothing to scroll.

Actual Behavior

In Terminal, using the mouse wheel when quickedit is disabled notifies the program and does not move the viewport.

This means that any CLI application is forced to handle mouse wheel notifications and call SetConsoleWindowInfo as needed. I don't even know if this works on Terminal (the documentation suggests it might not.)

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels May 26, 2021
@DHowett
Copy link
Member

DHowett commented May 26, 2021

As I understand it, this is how conhost should work too. ENABLE_MOUSE_INPUT is a default mode; disabling quick edit without disabling mouse mode should result in mouse events being inserted into the input queue.

At the end of the day, we had to settle on a heuristic for determining whether an application actually wanted mouse input or not.

The things you have found with regards to terminal passing mouse input versus handling it locally (i.e. scrolling the viewport) are likely bugs, not the result of a stance the console team is taking. We are certainly not asking a TUI to implement scrolling on its own, unless it has a specific need to doing so.

Just to confirm: Yori disables quick edit but leaves mouse input on? And the traditional console still scrolls? Highly unexpected. Whenever are we supposed to generate MOUSE_WHEELED events?

@malxau
Copy link
Contributor Author

malxau commented May 26, 2021

Yori disables quick edit but leaves mouse input on?

Yes, intentionally. It's trying to handle other forms of mouse input (not wheel events specifically.)

And the traditional console still scrolls? Highly unexpected.

Yes.

Whenever are we supposed to generate MOUSE_WHEELED events?

Those are generated too AFAIK. That's why I'm saying that a TUI application has to use CreateConsoleScreenBuffer/SetConsoleActiveScreenBuffer to ensure that it only gets MOUSE_WHEELED events without also triggering conhost scrolling.

I understand that the traditional conhost behavior is a little counterintuitive. This really feels like a case of "no good answer", because depending on the class of application it either has to explicitly suppress viewport scrolling or explicitly facilitate it. It's almost like ReadConsoleInput needs a DefWindowProc ;)

@zadjii-msft zadjii-msft added Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels May 26, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 26, 2021
@zadjii-msft zadjii-msft added the Priority-2 A description (P2) label May 26, 2021
@DHowett
Copy link
Member

DHowett commented May 26, 2021

And the traditional console still scrolls? Highly unexpected.

Yes.

Whenever are we supposed to generate MOUSE_WHEELED events?

Those are generated too AFAIK.

You know, I actually can't recapitulate this behavior. I have a program that prints an infinite list of mouse events as they're generated, and when I wheel the viewport definitely doesn't move. I may be missing something...

@DHowett
Copy link
Member

DHowett commented May 26, 2021

Ah, perhaps this is the "snap to bottom on new line" behavior confounding.

@DHowett DHowett added this to the Terminal v2.0 milestone May 27, 2021
@DHowett
Copy link
Member

DHowett commented May 27, 2021

There's no x11 mouse mode that matches this behavior, so we're going to have to get very creative somewhat quickly.

@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label May 27, 2021
@malxau
Copy link
Contributor Author

malxau commented May 27, 2021

I don't understand this comment. Do you mean there's no VT description? x11 should have no idea about the command line program, so it sends the events to the terminal, and the terminal decides how to update its own display and/or what to send to the command line program, no?

Is there anything I can do here that would be helpful?

Note that I didn't mean to file this to say "this behavior is horribly broken"; I meant to say "I'm trying out this mouse support and noticed that it diverges from conhost here, please confirm if this is intentional." I doubt many users will be affected by this, because mouse driven command line programs are most likely TUI programs that really don't want the viewport to scroll.

@DHowett
Copy link
Member

DHowett commented May 27, 2021

I wrote up a whole reply explaining the guiding principles behind conpty before I re-read your question, so I've collapsed that reply below.

I said "x11" instead of "xterm", as xterm is the terminal that specified the mouse modes in VT 😄

collapsed reply about ConPTY

Sorry, what I mean is:

Terminal must only communicate with conhost using VT, as it uses ConPTY. Other third-party terminals must do the same. This is unequivocal. This is also why there's no backdoor from WT to the client application such that we could know its console modes and act differently¹². 😄

Therefore, we have to translate a request for ENABLE_MOUSE_MODE on the input handle into a request from a standards-conforming terminal emulator that it enable VT mouse mode.

Now, mouse mode is specified in VT (by xterm) with a number of different modes (i.e. what sorts of events are requested) and encodings. That set of modes doesn't include one that matches this behavior "the terminal sends scrolling events and also scrolls its viewport" 😄

¹If we communicate that state across ConPTY to Terminal, we're also asking 3rd party developers who are working on terminals for Windows to answer our secret handshake.

²It is also the root cause of us steadfastly not supporting a scrollback buffer on the console app side.

@DHowett
Copy link
Member

DHowett commented May 27, 2021

I think this is a behavior difference that we should like to fix, if possible. I want the breaks we have to make from conhost to be explicit decisions, not just the result of realizing we've been caught out without them. 😄

I always appreciate you filing these issues!

@malxau
Copy link
Contributor Author

malxau commented Jul 16, 2021

Okay, I think I'm full of it. It's not that conhost sends mouse events to the app and implemented wheel itself; I wrote code that scrolled the viewport and completely forgot that it existed.

So now I think the issue is that SetConsoleWindowInfo() does scroll the viewport on conhost but not on WT. That's probably intentional but it's more concrete and less bonkers than my earlier description.

Edit: Okay, now I debugged it. My code calls GetConsoleScreenBufferInfo() which indicates that buffer size == viewport size. Given that, it has no way to describe a scroll action to SetConsoleWindowInfo() - it believes the viewport is already at the top of the buffer. So if an application receives mouse input, scrolling becomes impossible - the terminal won't do it, and the program receiving mouse events can't do it.

@malxau malxau changed the title Mouse scroll when quickedit disabled SetConsoleWindowInfo doesn't scroll visible region in Terminal Jul 16, 2021
@chrisant996
Copy link

I've tracked down multiple Clink problems to GetConsoleScreenBufferInfo() reporting inaccurate size information.

With Microsoft Terminal, GetConsoleScreenBufferInfo() essentially tells the console app that there is no scrollback buffer, because srWindow == dwSize. That is very different from normal conhost behavior.

This makes it seemingly impossible for a console program to interact with the scrollback history.
Clink is unable to scroll the window, so none of its scrolling commands or hotkeys work.
Clink is unable to retrieve text or colors from the scrollback history, so none of its "retrieve text/colors" commands work.

I have scripts bound to F7 and F8 in Clink, and they normally scroll through compiler errors in the scrollback buffer. But they don't work in Microsoft Terminal, because it hides the scrollback buffer from console applications.

csbiInfo _CONSOLE_SCREEN_BUFFER_INFO
dwSize _COORD
X 0x0078 short
Y 0x0032 short
dwCursorPosition _COORD
X 0x0000 short
Y 0x0031 short
wAttributes 0x0007 unsigned short
srWindow _SMALL_RECT
Left 0x0000 short
Top 0x0000 short
Right 0x0077 short
Bottom 0x0031 short
dwMaximumWindowSize _COORD
X 0x0078 short
Y 0x0032 short

@zadjii-msft
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

4 participants