Repository navigation
Rewrite the MSAA/UIA integration into conhost - #19344
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
| const auto& bufferBefore = gci.GetActiveOutputBuffer(); | ||
| const auto cursorBefore = bufferBefore.GetTextBuffer().GetCursor().GetPosition(); | ||
|
|
||
| auto raise = wil::scope_exit([&bufferBefore, cursorBefore] { |
There was a problem hiding this comment.
hit this with AVRF please. I'm worried about capturing this reference by reference.
can we just capture the pointer itself, since all we do is compare it?
There was a problem hiding this comment.
AFAIK lambda captures behave like auto so bufferBefore would capture a copy, not a reference. But I agree that capturing a pointer would be easier to read and understand.
| // Flags is expected to be 2, 1, or 0. 2 in selecting (whether or not visible), 1 if just visible, 0 if invisible/noselect. | ||
| if (WI_IsFlagSet(gci.Flags, CONSOLE_SELECTING)) | ||
| { | ||
| flags = IAccessibilityNotifier::ConsoleCaretEventFlags::CaretSelection; |
There was a problem hiding this comment.
📝 watch where Selection/Invisible/Visible go
| <ClCompile Include="..\writeData.cpp" /> | ||
| <ClCompile Include="..\_output.cpp" /> | ||
| <ClCompile Include="..\_stream.cpp" /> | ||
| <ClCompile Include="..\AccessibilityNotifier.cpp" /> |
There was a problem hiding this comment.
add to sources.inc
| void RemoteConsoleControl::Control(ControlType, PVOID, DWORD) noexcept | ||
| { | ||
| WI_ASSERT_FAIL(); | ||
| } | ||
|
|
||
| void RemoteConsoleControl::NotifyWinEvent(DWORD, HWND, LONG, LONG) noexcept | ||
| { | ||
| WI_ASSERT_FAIL(); | ||
| } |
There was a problem hiding this comment.
you need to test this out as a handoff recipient to verify these asserts never hit
There was a problem hiding this comment.
Will do. Theoretically nothing will happen because AccessibilityNotifier only exists if the Window class has been loaded.
| </ClCompile> | ||
| </ItemDefinitionGroup> | ||
| <ItemGroup> | ||
| <ClCompile Include="..\AccessibilityNotifier.cpp" /> |
There was a problem hiding this comment.
update sources
Dustin L. Howett (DHowett)
left a comment
Dustin L. Howett (DHowett)
left a comment
There was a problem hiding this comment.
Approving with comments! Glorious work, well done L.
| bool _fInterceptCopyPaste; | ||
|
|
||
| bool _TerminalScrolling; | ||
| bool _TerminalScrolling = true; |
There was a problem hiding this comment.
nit we should default to false for compatibility with old console
Goal: Remove
CursorBlinker.Problem: Spooky action at a distance via
Cursor::HasMoved.Solution: Moved all the a11y event raising into
_stream.cppand pray for the best.Goal: Prevent node.js from tanking conhost performance via MSAA (WHY).
Problem:
ServiceLocator.Solution: Unserviced the locator. Debounced event raising. Performance increased by >10x.
Problem 2: Lots of files changed.
This PR is a prerequisite for #19330
Validation Steps Performed
Ran NVDA with and without UIA enabled and with different delays. ✅