[O erroneously inputted after exiting from Neovim 0.7.0 in latest Terminal Preview #17652

@MatejKafka commented on GitHub (Jun 7, 2022):

Just to add one datapoint, latest Alacritty handles it correctly, not sure if it supports Focus events.

@MatejKafka commented on GitHub (Jun 7, 2022): Just to add one datapoint, latest Alacritty handles it correctly, not sure if it supports Focus events.

claunia commented

@j4james commented on GitHub (Jun 7, 2022):

FYI, I had a brief look at this, and Neovim does turn off focus events on exit, but prior to that happening, it appears that someone is calling WriteConsoleInput with a FOCUS_EVENT record, and that's what triggers that final report. It's not the result of a WM_SETFOCUS or WM_KILLFOCUS event.

@j4james commented on GitHub (Jun 7, 2022): FYI, I had a brief look at this, and Neovim does turn off focus events on exit, but prior to that happening, it appears that someone is calling `WriteConsoleInput` with a `FOCUS_EVENT` record, and that's what triggers that final report. It's not the result of a `WM_SETFOCUS` or `WM_KILLFOCUS` event.

claunia commented

@nathpete-msft commented on GitHub (Jun 8, 2022):

It looks like there was an issue open on Neovim for something similar: neovim/neovim#17985
That says a fix has been merged in but trying the latest Neovim nightly still has the issue for me in Terminal, so it may not be related.

@nathpete-msft commented on GitHub (Jun 8, 2022): It looks like there was an issue open on Neovim for something similar: neovim/neovim#17985 That says a fix has been merged in but trying the latest Neovim nightly still has the issue for me in Terminal, so it may not be related.

claunia commented

@zadjii-msft commented on GitHub (Jun 8, 2022):

will continue adding notes here...

FYI, I had a brief look at this, and Neovim does turn off focus events on exit, but prior to that happening, it appears that someone is calling WriteConsoleInput with a FOCUS_EVENT record, and that's what triggers that final report. It's not the result of a WM_SETFOCUS or WM_KILLFOCUS event.

Thanks for that! That really made this investigation WAY easier.

>	OpenConsole.exe!_WriteConsoleInputWImplHelper(InputBuffer & context, std::deque<std::unique_ptr<IInputEvent,std::default_delete<IInputEvent>>,std::allocator<std::unique_ptr<IInputEvent,std::default_delete<IInputEvent>>>> & events, unsigned __int64 & written, const bool append) Line 429	C++
 	OpenConsole.exe!ApiRoutines::WriteConsoleInputWImpl(InputBuffer & context, const gsl::span<_INPUT_RECORD const ,-1> buffer, unsigned __int64 & written, const bool append) Line 514	C++
 	OpenConsole.exe!ApiDispatchers::ServerWriteConsoleInput(_CONSOLE_API_MSG * const m, int * const __formal) Line 894	C++
 	OpenConsole.exe!ApiSorter::ConsoleDispatchRequest(_CONSOLE_API_MSG * Message) Line 178	C++
 	OpenConsole.exe!IoDispatchers::ConsoleDispatchRequest(_CONSOLE_API_MSG * pMessage) Line 577	C++
 	OpenConsole.exe!IoSorter::ServiceIoOperation(_CONSOLE_API_MSG * const pMsg, _CONSOLE_API_MSG * * ReplyMsg) Line 33	C++
 	OpenConsole.exe!ConsoleIoThread(void * lpParameter) Line 978	C++

m.Descriptor.Process is a ConsoleProcessHandle* in this console.

Microsoft::Console::Interactivity::ServiceLocator::s_globals.ciConsoleInformation.ProcessHandleList has the processes.

The 0th process in the list is the one that sent it

Looks like the message is coming from neovim.

	Name	Value	Type
	context.InputMode	0x00000208	unsigned long
◢	context._termInput._inputMode	{_data=0x0000000000000201 }	til::enumset<enum Microsoft::Console::VirtualTerminal::TerminalInput::Mode,unsigned __int64>

0x0201 is Mode::FocusEvent | Mode::Ansi, sure.

0x208 is ENABLE_VIRTUAL_TERMINAL_INPUT | ENABLE_WINDOW_INPUT

So, the console is in VT input mode, and is expecting the client can accept focus events. So this must be happening before neovim disables focus events mode...

Yep. Sticking a breakpoint in AdaptDispatch::_SetInputMode, when mode == TerminalInput::Mode::FocusEvent will get hit after the WriteConsoleInput one is resumed. So it definitely seems like the FOCUS_EVENT is getting written before they disable the focus event mode. Maybe they just need to drain all the pending console input?

@zadjii-msft commented on GitHub (Jun 8, 2022): _will continue adding notes here..._ > FYI, I had a brief look at this, and Neovim does turn off focus events on exit, but prior to that happening, it appears that someone is calling `WriteConsoleInput` with a `FOCUS_EVENT` record, and that's what triggers that final report. It's not the result of a `WM_SETFOCUS` or `WM_KILLFOCUS` event. Thanks for that! That really made this investigation WAY easier. ``` > OpenConsole.exe!_WriteConsoleInputWImplHelper(InputBuffer & context, std::deque<std::unique_ptr<IInputEvent,std::default_delete<IInputEvent>>,std::allocator<std::unique_ptr<IInputEvent,std::default_delete<IInputEvent>>>> & events, unsigned __int64 & written, const bool append) Line 429 C++ OpenConsole.exe!ApiRoutines::WriteConsoleInputWImpl(InputBuffer & context, const gsl::span<_INPUT_RECORD const ,-1> buffer, unsigned __int64 & written, const bool append) Line 514 C++ OpenConsole.exe!ApiDispatchers::ServerWriteConsoleInput(_CONSOLE_API_MSG * const m, int * const __formal) Line 894 C++ OpenConsole.exe!ApiSorter::ConsoleDispatchRequest(_CONSOLE_API_MSG * Message) Line 178 C++ OpenConsole.exe!IoDispatchers::ConsoleDispatchRequest(_CONSOLE_API_MSG * pMessage) Line 577 C++ OpenConsole.exe!IoSorter::ServiceIoOperation(_CONSOLE_API_MSG * const pMsg, _CONSOLE_API_MSG * * ReplyMsg) Line 33 C++ OpenConsole.exe!ConsoleIoThread(void * lpParameter) Line 978 C++ ``` `m.Descriptor.Process` is a `ConsoleProcessHandle*` in this console. `Microsoft::Console::Interactivity::ServiceLocator::s_globals.ciConsoleInformation.ProcessHandleList` has the processes. The 0th process in the list is the one that sent it ![image](https://user-images.githubusercontent.com/18356694/172668174-afc2ade4-c50c-4fc3-a11f-868922c7925c.png) Looks like the message _is_ coming from neovim. | Name | Value | Type -- | -- | -- | -- | context.InputMode | 0x00000208 | unsigned long ◢ | context._termInput._inputMode | {_data=0x0000000000000201 } | til::enumset<enum Microsoft::Console::VirtualTerminal::TerminalInput::Mode,unsigned __int64> `0x0201` is `Mode::FocusEvent | Mode::Ansi`, sure. `0x208` is `ENABLE_VIRTUAL_TERMINAL_INPUT | ENABLE_WINDOW_INPUT` So, the console is in VT input mode, and is expecting the client can accept focus events. So this must be happening before neovim disables focus events mode... Yep. Sticking a breakpoint in `AdaptDispatch::_SetInputMode`, when `mode == TerminalInput::Mode::FocusEvent` will get hit after the `WriteConsoleInput` one is resumed. So it definitely seems like the FOCUS_EVENT is getting written _before_ they disable the focus event mode. Maybe they just need to drain all the pending console input?

claunia commented

@DHowett commented on GitHub (Jun 8, 2022):

neovim is sending a win32 console focus event right as it exits? I can't fathom why they'd do that: a synthetic focus event doesn't even say anything about the state of the world -- the absolute best it can accomplish is making a console app think it's in focus when it actually doesn't have win32 focus...

@DHowett commented on GitHub (Jun 8, 2022): _neovim is sending a win32 console focus event right as it exits?_ I can't fathom why they'd do that: a synthetic focus event doesn't even say anything about the state of the world -- the absolute best it can accomplish is making a console app think it's in focus when it actually doesn't have **win32 focus**...

claunia commented

@zadjii-msft commented on GitHub (Jun 8, 2022):

Yea why are they doing that?

Value	Meaning
FOCUS_EVENT 0x0010	The Event member contains a FOCUS_EVENT_RECORD structure. These events are used internally and should be ignored.

I guess that doesn't explicitly state "don't use this", but why

@zadjii-msft commented on GitHub (Jun 8, 2022): Yea why are they doing that? Value | Meaning -- | -- FOCUS_EVENT 0x0010 | The Event member contains a [FOCUS_EVENT_RECORD](https://docs.microsoft.com/en-us/windows/console/focus-event-record-str) structure. These events are used internally and should be ignored. I guess that doesn't explicitly state "don't use this", but _why_

claunia commented

@zadjii-msft commented on GitHub (Jun 8, 2022):

@justinmk any chances you know how/why neovim would be doing this? I reckon this is something that should get moved over to neovim, but I just want to be sure.

@zadjii-msft commented on GitHub (Jun 8, 2022): @justinmk any chances you know how/why neovim would be doing this? I reckon this is something that should get moved over to neovim, but I just want to be sure.

claunia commented

@justinmk commented on GitHub (Jun 8, 2022):

Probably not intentional.

Maybe they just need to drain all the pending console input?

Maybe.

If someone decides to create an issue at Nvim, please reference this issue.

@justinmk commented on GitHub (Jun 8, 2022): Probably not intentional. > Maybe they just need to drain all the pending console input? Maybe. If someone decides to create an issue at Nvim, please reference this issue.

claunia commented

@zadjii-msft commented on GitHub (Jun 8, 2022):

Filed https://github.com/neovim/neovim/issues/18899 for investigation.

Theoretically, we might be able to route input in such a way that focus events from the API don't hit this path. Ultimately, I am curious why this is being sent in the first place. That might affect the route we want to go in mitigating this.

@zadjii-msft commented on GitHub (Jun 8, 2022): Filed https://github.com/neovim/neovim/issues/18899 for investigation. Theoretically, we might be able to route input in such a way that focus events from the API don't hit this path. Ultimately, I am curious why this is being sent in the first place. That might affect the route we want to go in mitigating this.

claunia commented

@zadjii-msft commented on GitHub (Jun 9, 2022):

Hmm. Considering this is something who's root cause is up in libuv (https://github.com/neovim/neovim/issues/18899#issuecomment-1150456895), maybe we do need to think about this.

@zadjii-msft commented on GitHub (Jun 9, 2022): Hmm. Considering this is something who's root cause is up in libuv (https://github.com/neovim/neovim/issues/18899#issuecomment-1150456895), maybe we _do_ need to think about this.

claunia commented

@j4james commented on GitHub (Jun 9, 2022):

@zadjii-msft I'm quite partial to your idea of not sending the CSI I and CSI O reports when the focus events come from an API call.

If you look at the FocusEvent constructors, there's one that takes a FOCUS_EVENT_RECORD and one that takes a bool. As far as I can see, if it's constructed from an API call it'll use the former, and if it's the result of an actual focus change event it'll use the latter.

So I was thinking we could add a flag in FocusEvent that records whether it came from an input record, and then in TerminalInput, where we process those events, we just don't generate a focus report if that flag is set.

@j4james commented on GitHub (Jun 9, 2022): @zadjii-msft I'm quite partial to your idea of not sending the `CSI I` and `CSI O` reports when the focus events come from an API call. If you look at the `FocusEvent` constructors, there's one that takes a `FOCUS_EVENT_RECORD` and one that takes a `bool`. As far as I can see, if it's constructed from an API call it'll use the former, and if it's the result of an actual focus change event it'll use the latter. So I was thinking we could add a flag in `FocusEvent` that records whether it came from an input record, and then in `TerminalInput`, where we process those events, we just don't generate a focus report if that flag is set.

claunia commented