diff --git a/src/interactivity/win32/uiaTextRange.cpp b/src/interactivity/win32/uiaTextRange.cpp index ba3a0109d596..a33957bfc920 100644 --- a/src/interactivity/win32/uiaTextRange.cpp +++ b/src/interactivity/win32/uiaTextRange.cpp @@ -77,12 +77,6 @@ IFACEMETHODIMP UiaTextRange::Clone(_Outptr_result_maybenull_ ITextRangeProvider* return S_OK; } -void UiaTextRange::_ChangeViewport(const SMALL_RECT NewWindow) -{ - auto provider = static_cast(_pProvider); - provider->ChangeViewport(NewWindow); -} - void UiaTextRange::_TranslatePointToScreen(LPPOINT clientPoint) const { ClientToScreen(_getWindowHandle(), clientPoint); diff --git a/src/interactivity/win32/uiaTextRange.hpp b/src/interactivity/win32/uiaTextRange.hpp index 5d83f01e8de7..056e013181a8 100644 --- a/src/interactivity/win32/uiaTextRange.hpp +++ b/src/interactivity/win32/uiaTextRange.hpp @@ -56,7 +56,6 @@ namespace Microsoft::Console::Interactivity::Win32 IFACEMETHODIMP Clone(_Outptr_result_maybenull_ ITextRangeProvider** ppRetVal) override; protected: - void _ChangeViewport(const SMALL_RECT NewWindow) override; void _TranslatePointToScreen(LPPOINT clientPoint) const override; void _TranslatePointFromScreen(LPPOINT screenPoint) const override; diff --git a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp index 9faff4c0a640..e390201f8108 100644 --- a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp +++ b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp @@ -7,6 +7,7 @@ #include "CommonState.hpp" #include "uiaTextRange.hpp" +#include "../types/ScreenInfoUiaProviderBase.h" #include "../../../buffer/out/textBuffer.hpp" using namespace WEX::Common; @@ -24,46 +25,65 @@ using namespace Microsoft::Console::Interactivity::Win32; // it not doing anything for its implementation because it is not used // during the unit tests below. -class DummyElementProvider final : public IRawElementProviderSimple +class DummyElementProvider final : public ScreenInfoUiaProviderBase { public: - // IUnknown methods - IFACEMETHODIMP_(ULONG) - AddRef() + IFACEMETHODIMP Navigate(_In_ NavigateDirection /*direction*/, + _COM_Outptr_result_maybenull_ IRawElementProviderFragment** /*ppProvider*/) override { - return 1; + return E_NOTIMPL; } - IFACEMETHODIMP_(ULONG) - Release() + IFACEMETHODIMP get_BoundingRectangle(_Out_ UiaRect* /*pRect*/) override { - return 1; + return E_NOTIMPL; } - IFACEMETHODIMP QueryInterface(_In_ REFIID /*riid*/, - _COM_Outptr_result_maybenull_ void** /*ppInterface*/) + + IFACEMETHODIMP get_FragmentRoot(_COM_Outptr_result_maybenull_ IRawElementProviderFragmentRoot** /*ppProvider*/) override { return E_NOTIMPL; - }; + } + + void ChangeViewport(const SMALL_RECT /*NewWindow*/) + { + return; + } - // IRawElementProviderSimple methods - IFACEMETHODIMP get_ProviderOptions(_Out_ ProviderOptions* /*pOptions*/) + HRESULT GetSelectionRange(_In_ IRawElementProviderSimple* /*pProvider*/, const std::wstring_view /*wordDelimiters*/, _COM_Outptr_result_maybenull_ Microsoft::Console::Types::UiaTextRangeBase** /*ppUtr*/) override { return E_NOTIMPL; } - IFACEMETHODIMP GetPatternProvider(_In_ PATTERNID /*iid*/, - _COM_Outptr_result_maybenull_ IUnknown** /*ppInterface*/) + // degenerate range + HRESULT CreateTextRange(_In_ IRawElementProviderSimple* const /*pProvider*/, const std::wstring_view /*wordDelimiters*/, _COM_Outptr_result_maybenull_ Microsoft::Console::Types::UiaTextRangeBase** /*ppUtr*/) override { return E_NOTIMPL; } - IFACEMETHODIMP GetPropertyValue(_In_ PROPERTYID /*idProp*/, - _Out_ VARIANT* /*pVariant*/) + // degenerate range at cursor position + HRESULT CreateTextRange(_In_ IRawElementProviderSimple* const /*pProvider*/, + const Cursor& /*cursor*/, + const std::wstring_view /*wordDelimiters*/, + _COM_Outptr_result_maybenull_ Microsoft::Console::Types::UiaTextRangeBase** /*ppUtr*/) override { return E_NOTIMPL; } - IFACEMETHODIMP get_HostRawElementProvider(_COM_Outptr_result_maybenull_ IRawElementProviderSimple** /*ppProvider*/) + // specific endpoint range + HRESULT CreateTextRange(_In_ IRawElementProviderSimple* const /*pProvider*/, + const COORD /*start*/, + const COORD /*end*/, + const std::wstring_view /*wordDelimiters*/, + _COM_Outptr_result_maybenull_ Microsoft::Console::Types::UiaTextRangeBase** /*ppUtr*/) override + { + return E_NOTIMPL; + } + + // range from a UiaPoint + HRESULT CreateTextRange(_In_ IRawElementProviderSimple* const /*pProvider*/, + const UiaPoint /*point*/, + const std::wstring_view /*wordDelimiters*/, + _COM_Outptr_result_maybenull_ Microsoft::Console::Types::UiaTextRangeBase** /*ppUtr*/) override { return E_NOTIMPL; } @@ -106,6 +126,12 @@ class UiaTextRangeTests ExpectedResult expected; }; + struct ScrollTest + { + std::wstring comment; + short yPos; + }; + static constexpr wchar_t* toString(TextUnit unit) noexcept { // if a format is not supported, it goes to the next largest text unit @@ -1284,4 +1310,40 @@ class UiaTextRangeTests THROW_IF_FAILED(utr->GetText(-1, &text)); VERIFY_ARE_EQUAL(L"M", std::wstring_view{ text }); } + + TEST_METHOD(ScrollIntoView) + { + const auto bufferSize{ _pTextBuffer->GetSize() }; + const auto viewportSize{ _pUiaData->GetViewport() }; + + const std::vector testData{ + { L"Origin", bufferSize.Top() }, + { L"ViewportHeight From Top - 1", bufferSize.Top() + viewportSize.Height() - 1 }, + { L"ViewportHeight From Top", bufferSize.Top() + viewportSize.Height() }, + { L"ViewportHeight From Top + 1", bufferSize.Top() + viewportSize.Height() + 1 }, + { L"ViewportHeight From Bottom - 1", bufferSize.BottomInclusive() - viewportSize.Height() - 1 }, + { L"ViewportHeight From Bottom", bufferSize.BottomInclusive() - viewportSize.Height() }, + { L"ViewportHeight From Bottom + 1", bufferSize.BottomInclusive() - viewportSize.Height() + 1 }, + + // GH#7839: ExclusiveEnd is a non-existent space, + // so scrolling to it when !alignToTop used to crash + { L"Exclusive End", bufferSize.BottomExclusive() } + }; + + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"Data:alignToTop", L"{false, true}") + END_TEST_METHOD_PROPERTIES(); + + bool alignToTop; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"alignToTop", alignToTop), L"Get alignToTop variant"); + + Microsoft::WRL::ComPtr utr; + for (const auto test : testData) + { + Log::Comment(test.comment.c_str()); + const til::point pos{ bufferSize.Left(), test.yPos }; + THROW_IF_FAILED(Microsoft::WRL::MakeAndInitialize(&utr, _pUiaData, &_dummyProvider, pos, pos)); + VERIFY_SUCCEEDED(utr->ScrollIntoView(alignToTop)); + } + } }; diff --git a/src/types/TermControlUiaTextRange.cpp b/src/types/TermControlUiaTextRange.cpp index e2bfa950407a..ba5128885522 100644 --- a/src/types/TermControlUiaTextRange.cpp +++ b/src/types/TermControlUiaTextRange.cpp @@ -80,12 +80,6 @@ IFACEMETHODIMP TermControlUiaTextRange::Clone(_Outptr_result_maybenull_ ITextRan return S_OK; } -void TermControlUiaTextRange::_ChangeViewport(const SMALL_RECT NewWindow) -{ - const gsl::not_null provider = static_cast(_pProvider); - provider->ChangeViewport(NewWindow); -} - // Method Description: // - Transform coordinates relative to the client to relative to the screen // Arguments: diff --git a/src/types/TermControlUiaTextRange.hpp b/src/types/TermControlUiaTextRange.hpp index 696b876dff45..82c798431c68 100644 --- a/src/types/TermControlUiaTextRange.hpp +++ b/src/types/TermControlUiaTextRange.hpp @@ -55,7 +55,6 @@ namespace Microsoft::Terminal IFACEMETHODIMP Clone(_Outptr_result_maybenull_ ITextRangeProvider** ppRetVal) override; protected: - void _ChangeViewport(const SMALL_RECT NewWindow) override; void _TranslatePointToScreen(LPPOINT clientPoint) const override; void _TranslatePointFromScreen(LPPOINT screenPoint) const override; const COORD _getScreenFontSize() const override; diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index e264159997da..99589eeb0c46 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -784,7 +784,7 @@ try } else { - // we can align to the top so we'll just move the viewport + // we can't align to the top so we'll just move the viewport // to the bottom of the screen buffer newViewport.Bottom = bottomRow; newViewport.Top = bottomRow - viewportHeight + 1; @@ -796,9 +796,13 @@ try // check if we can align to the bottom if (static_cast(endScreenInfoRow) >= viewportHeight) { + // GH#7839: endScreenInfoRow may be ExclusiveEnd + // ExclusiveEnd is past the bottomRow + // so we need to clamp to the bottom row to stay in bounds + // we can align to bottom - newViewport.Bottom = endScreenInfoRow; - newViewport.Top = endScreenInfoRow - viewportHeight + 1; + newViewport.Bottom = std::min(endScreenInfoRow, bottomRow); + newViewport.Top = base::ClampedNumeric(newViewport.Bottom) - viewportHeight + 1; } else { @@ -815,7 +819,8 @@ try Unlock.reset(); - _ChangeViewport(newViewport); + const gsl::not_null provider = static_cast(_pProvider); + provider->ChangeViewport(newViewport); UiaTracing::TextRange::ScrollIntoView(alignToTop, *this); return S_OK; diff --git a/src/types/UiaTextRangeBase.hpp b/src/types/UiaTextRangeBase.hpp index ab62200e1528..16b1e4be3d0f 100644 --- a/src/types/UiaTextRangeBase.hpp +++ b/src/types/UiaTextRangeBase.hpp @@ -127,7 +127,6 @@ namespace Microsoft::Console::Types std::wstring _wordDelimiters{}; - virtual void _ChangeViewport(const SMALL_RECT NewWindow) = 0; virtual void _TranslatePointToScreen(LPPOINT clientPoint) const = 0; virtual void _TranslatePointFromScreen(LPPOINT screenPoint) const = 0;