microsoft/Terminal

ScrollRegion function generates inappropriate EVENT_CONSOLE_UPDATE_SCROLL

Open

#12.656 aberto em 10 de mar. de 2022

Ver no GitHub
 (1 comment) (0 reactions) (0 assignees)C++ (3.212 forks)batch import
Area-OutputHelp WantedIssue-BugPriority-3Product-Conhost

Métricas do repositório

Stars
 (35.764 stars)
Métricas de merge de PR
 (Mesclagem média 27d 19h) (24 fundiu PRs em 30d)

Description

Windows Terminal version

n/a

Windows build number

10.0.19041.1415

Other Software

No response

Steps to reproduce

The ScrollRegion function is used in conhost to "scroll" an area of the screen buffer in multiple directions. It's used in a variety of places, but the closest equivalent in the public API is the ScrollConsoleScreenBuffer function. It doesn't pan the viewport, as typically occurs when the console scrolls - it's more akin to copying a chunk of the buffer from one position to another.

But if you look at the code for ScrollRegion, it makes a call to _ScrollScreen, which then makes a call to NotifyConsoleUpdateScrollEvent like this:

https://github.com/microsoft/terminal/blob/3bd3a4f712006d752b59d66e104aeb6bc5f7f9d6/src/host/output.cpp#L279

That code is a bit obfuscated by the use of Origin, but it essentially boils down to:

pNotifier->NotifyConsoleUpdateScrollEvent(target.Left() - source.Left(), target.Top() - source.RightInclusive());

Quite clearly the second parameter is nonsense.

That said, I'm not sure a scroll event should be used here at all. As mentioned above, it's not the equivalent of the viewport scrolling, so I don't think it makes sense to trigger an EVENT_CONSOLE_UPDATE_SCROLL.

Hopefully someone that is more knowledgable about accessibility can chime in here, but it seemed to me that EVENT_CONSOLE_UPDATE_SCROLL was how you would track the movement of the viewport. Other update events use absolute coordinates, so you need to know the viewport position in order to obtain their relative location. And if something like ScrollRegion is sending unrelated scroll events, that makes that data useless.

Expected Behavior

I don't think ScrollRegion should be triggering EVENT_CONSOLE_UPDATE_SCROLL at all, but if it does, it should at least give meaningful deltas.

Actual Behavior

ScrollRegion is triggering meaningless EVENT_CONSOLE_UPDATE_SCROLL notifications.

Guia do colaborador