From 84bca57d2dbd28d1a3b9b98182c50616f11e3136 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 3 Jun 2021 16:45:38 -0700 Subject: [PATCH 1/9] Expose Text Attributes to UI Automation --- src/buffer/out/textBuffer.cpp | 14 + src/buffer/out/textBuffer.hpp | 1 + src/buffer/out/textBufferCellIterator.cpp | 45 +- src/buffer/out/textBufferCellIterator.hpp | 4 + .../TerminalControl/XamlUiaTextRange.cpp | 54 +- .../UiaTextRangeTests.cpp | 323 ++++++++++++ src/types/UiaTextRangeBase.cpp | 484 +++++++++++++++++- src/types/UiaTextRangeBase.hpp | 5 + src/types/UiaTracing.cpp | 150 +++++- src/types/UiaTracing.h | 17 +- 10 files changed, 1070 insertions(+), 27 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 76d12708639..d322c871a36 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -184,6 +184,20 @@ TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport return TextBufferCellIterator(*this, at, limit); } +// Routine Description: +// - Retrieves read-only cell iterator at the given buffer location +// but restricted to operate only inside the given viewport. +// Arguments: +// - at - X,Y position in buffer for iterator start position +// - limit - boundaries for the iterator to operate within +// - until - X,Y position in buffer for last position for the iterator to read (inclusive) +// Return Value: +// - Read-only iterator of cell data. +TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport limit, const COORD until) const +{ + return TextBufferCellIterator(*this, at, limit, until); +} + //Routine Description: // - Corrects and enforces consistent double byte character state (KAttrs line) within a row of the text buffer. // - This will take the given double byte information and check that it will be consistent when inserted into the buffer diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 468a9fc4f34..702a57d493b 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -81,6 +81,7 @@ class TextBuffer final TextBufferCellIterator GetCellDataAt(const COORD at) const; TextBufferCellIterator GetCellLineDataAt(const COORD at) const; TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit) const; + TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit, const COORD until) const; TextBufferTextIterator GetTextDataAt(const COORD at) const; TextBufferTextIterator GetTextLineDataAt(const COORD at) const; TextBufferTextIterator GetTextDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit) const; diff --git a/src/buffer/out/textBufferCellIterator.cpp b/src/buffer/out/textBufferCellIterator.cpp index 7ab512c72cd..a8402f7b8a8 100644 --- a/src/buffer/out/textBufferCellIterator.cpp +++ b/src/buffer/out/textBufferCellIterator.cpp @@ -50,6 +50,25 @@ TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD p _GenerateView(); } +// Routine Description: +// - Creates a new read-only iterator to seek through cell data stored within a screen buffer +// Arguments: +// - buffer - Text buffer to seek through +// - pos - Starting position to retrieve text data from (within screen buffer bounds) +// - limits - Viewport limits to restrict the iterator within the buffer bounds (smaller than the buffer itself) +// - endPosInclusive - last position to iterate through (inclusive) +TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport limits, const COORD endPosInclusive) : + TextBufferCellIterator(buffer, pos, limits) +{ + // Throw if the coordinate is not limited to the inside of the given buffer. + THROW_HR_IF(E_INVALIDARG, !_bounds.IsInBounds(endPosInclusive)); + + // Throw if pos is past endPos + THROW_HR_IF(E_INVALIDARG, _bounds.CompareInBounds(pos, endPosInclusive) > 0); + + _endPosInclusive = endPosInclusive; +} + // Routine Description: // - Tells if the iterator is still valid (hasn't exceeded boundaries of underlying text buffer) // Return Value: @@ -72,7 +91,8 @@ bool TextBufferCellIterator::operator==(const TextBufferCellIterator& it) const _exceeded == it._exceeded && _bounds == it._bounds && _pRow == it._pRow && - _attrIter == it._attrIter; + _attrIter == it._attrIter && + _endPosInclusive == _endPosInclusive; } // Routine Description: @@ -98,12 +118,26 @@ TextBufferCellIterator& TextBufferCellIterator::operator+=(const ptrdiff_t& move auto newPos = _pos; while (move > 0 && !_exceeded) { - _exceeded = !_bounds.IncrementInBounds(newPos); + // If we have an endPos, check if we've exceeded it + if (_endPosInclusive.has_value()) + { + _exceeded = _bounds.CompareInBounds(newPos, *_endPosInclusive) > 0; + } + + // If we already exceeded from endPos, we'll short-circuit and _not_ increment + _exceeded |= !_bounds.IncrementInBounds(newPos); move--; } while (move < 0 && !_exceeded) { - _exceeded = !_bounds.DecrementInBounds(newPos); + // If we have an endPos, check if we've exceeded it + if (_endPosInclusive.has_value()) + { + _exceeded = _bounds.CompareInBounds(newPos, *_endPosInclusive) < 0; + } + + // If we already exceeded from endPos, we'll short-circuit and _not_ decrement + _exceeded |= !_bounds.DecrementInBounds(newPos); move++; } _SetPos(newPos); @@ -265,3 +299,8 @@ const OutputCellView* TextBufferCellIterator::operator->() const noexcept { return &_view; } + +COORD TextBufferCellIterator::Pos() const noexcept +{ + return _pos; +} diff --git a/src/buffer/out/textBufferCellIterator.hpp b/src/buffer/out/textBufferCellIterator.hpp index 8b7604bb6e4..c5fe69142e8 100644 --- a/src/buffer/out/textBufferCellIterator.hpp +++ b/src/buffer/out/textBufferCellIterator.hpp @@ -27,6 +27,7 @@ class TextBufferCellIterator public: TextBufferCellIterator(const TextBuffer& buffer, COORD pos); TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport limits); + TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport limits, const COORD endPosInclusive); operator bool() const noexcept; @@ -47,6 +48,8 @@ class TextBufferCellIterator const OutputCellView& operator*() const noexcept; const OutputCellView* operator->() const noexcept; + COORD Pos() const noexcept; + protected: void _SetPos(const COORD newPos); void _GenerateView(); @@ -60,6 +63,7 @@ class TextBufferCellIterator const Microsoft::Console::Types::Viewport _bounds; bool _exceeded; COORD _pos; + std::optional _endPosInclusive; #if UNIT_TESTING friend class TextBufferIteratorTests; diff --git a/src/cascadia/TerminalControl/XamlUiaTextRange.cpp b/src/cascadia/TerminalControl/XamlUiaTextRange.cpp index 7fb8d3b3d74..73b447d0a30 100644 --- a/src/cascadia/TerminalControl/XamlUiaTextRange.cpp +++ b/src/cascadia/TerminalControl/XamlUiaTextRange.cpp @@ -5,6 +5,7 @@ #include "XamlUiaTextRange.h" #include "../types/TermControlUiaTextRange.hpp" #include +#include // the same as COR_E_NOTSUPPORTED // we don't want to import the CLR headers to get it @@ -89,12 +90,56 @@ namespace winrt::Microsoft::Terminal::Control::implementation winrt::Windows::Foundation::IInspectable XamlUiaTextRange::GetAttributeValue(int32_t textAttributeId) const { - // Copied functionality from Types::UiaTextRange.cpp - if (textAttributeId == UIA_IsReadOnlyAttributeId) + // Call the function off of the underlying UiaTextRange. + VARIANT result; + THROW_IF_FAILED(_uiaProvider->GetAttributeValue(textAttributeId, &result)); + + // Convert the resulting VARIANT into a format that is consumable by XAML. + switch (result.vt) + { + case VT_BSTR: + { + return box_value(result.bstrVal); + } + case VT_I4: + { + return box_value(result.iVal); + } + case VT_R8: + { + return box_value(result.dblVal); + } + case VT_BOOL: + { + return box_value(result.boolVal); + } + case VT_UNKNOWN: { - return winrt::box_value(false); + // This one is particularly special. + // We might return a special value like UiaGetReservedMixedAttributeValue + // or UiaGetReservedNotSupportedValue. + // Some text attributes may return a real value, however, none of those + // are supported at this time. + // So we need to figure out what was actually intended to be returned. + + IUnknown* notSupportedVal; + UiaGetReservedNotSupportedValue(¬SupportedVal); + if (result.punkVal == notSupportedVal) + { + // See below for why we need to throw this special value. + winrt::throw_hresult(XAML_E_NOT_SUPPORTED); + } + + IUnknown* mixedAttributeVal; + UiaGetReservedMixedAttributeValue(&mixedAttributeVal); + if (result.punkVal == mixedAttributeVal) + { + return Windows::UI::Xaml::DependencyProperty::UnsetValue(); + } + + __fallthrough; } - else + default: { // We _need_ to return XAML_E_NOT_SUPPORTED here. // Returning nullptr is an improper implementation of it being unsupported. @@ -103,6 +148,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Magically, this doesn't affect other forms of navigation... winrt::throw_hresult(XAML_E_NOT_SUPPORTED); } + } } void XamlUiaTextRange::GetBoundingRectangles(com_array& returnValue) const diff --git a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp index 17c771830fd..f5ed538ab0a 100644 --- a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp +++ b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp @@ -9,6 +9,7 @@ #include "uiaTextRange.hpp" #include "../types/ScreenInfoUiaProviderBase.h" #include "../../../buffer/out/textBuffer.hpp" +#include "../types/UiaTracing.h" using namespace WEX::Common; using namespace WEX::Logging; @@ -132,6 +133,20 @@ class UiaTextRangeTests short yPos; }; + enum SupportedTextAttributes + { + FontName = UIA_FontNameAttributeId, // special handling + FontSize = UIA_FontSizeAttributeId, // special handling + FontWeight = UIA_FontWeightAttributeId, + ForegroundColor = UIA_ForegroundColorAttributeId, + Italic = UIA_IsItalicAttributeId, + IsReadOnly = UIA_IsReadOnlyAttributeId, // special handling (always false) + StrikethroughColor = UIA_StrikethroughColorAttributeId, + StrikethroughStyle = UIA_StrikethroughStyleAttributeId, + UnderlineColor = UIA_UnderlineColorAttributeId, + UnderlineStyle = UIA_UnderlineStyleAttributeId + }; + static constexpr wchar_t* toString(TextUnit unit) noexcept { // if a format is not supported, it goes to the next largest text unit @@ -1346,4 +1361,312 @@ class UiaTextRangeTests VERIFY_SUCCEEDED(utr->ScrollIntoView(alignToTop)); } } + + TEST_METHOD(GetAttributeValue) + { + Log::Comment(L"Check supported attributes"); + IUnknown* notSupportedVal; + UiaGetReservedNotSupportedValue(¬SupportedVal); + for (long uiaAttributeId = 40000; uiaAttributeId <= 40042; ++uiaAttributeId) + { + Microsoft::WRL::ComPtr utr; + THROW_IF_FAILED(Microsoft::WRL::MakeAndInitialize(&utr, _pUiaData, &_dummyProvider)); + + Log::Comment(UiaTracing::convertAttributeId(uiaAttributeId).c_str()); + VARIANT result; + VERIFY_SUCCEEDED(utr->GetAttributeValue(uiaAttributeId, &result)); + + switch (uiaAttributeId) + { + case FontName: + { + VERIFY_ARE_EQUAL(VT_BSTR, result.vt); + break; + } + case FontSize: + { + VERIFY_ARE_EQUAL(VT_R8, result.vt); + break; + } + case FontWeight: + case ForegroundColor: + case StrikethroughColor: + case StrikethroughStyle: + case UnderlineColor: + case UnderlineStyle: + { + VERIFY_ARE_EQUAL(VT_I4, result.vt); + break; + } + case Italic: + case IsReadOnly: + { + VERIFY_ARE_EQUAL(VT_BOOL, result.vt); + break; + } + default: + { + // Expected: not supported + VERIFY_ARE_EQUAL(VT_UNKNOWN, result.vt); + VERIFY_ARE_EQUAL(notSupportedVal, result.punkVal); + break; + } + } + } + + // This is the text attribute we'll use to update the text buffer. + // We'll modify it, then test if the UiaTextRange can extract/interpret the data properly. + // updateBuffer() will write that text attribute to the first cell in the buffer. + TextAttribute attr; + auto updateBuffer = [&](TextAttribute outputAttr) { + _pTextBuffer->Write({ outputAttr }, { 0, 0 }); + }; + + Microsoft::WRL::ComPtr utr; + THROW_IF_FAILED(Microsoft::WRL::MakeAndInitialize(&utr, _pUiaData, &_dummyProvider)); + { + Log::Comment(L"Test Font Weight"); + attr.SetBold(true); + updateBuffer(attr); + VARIANT result; + VERIFY_SUCCEEDED(utr->GetAttributeValue(FontWeight, &result)); + VERIFY_ARE_EQUAL(FW_BOLD, result.iVal); + + attr.SetBold(false); + updateBuffer(attr); + VERIFY_SUCCEEDED(utr->GetAttributeValue(FontWeight, &result)); + VERIFY_ARE_EQUAL(FW_NORMAL, result.iVal); + + attr.SetFaint(true); + updateBuffer(attr); + VERIFY_SUCCEEDED(utr->GetAttributeValue(FontWeight, &result)); + VERIFY_ARE_EQUAL(FW_LIGHT, result.iVal); + + attr.SetFaint(false); + updateBuffer(attr); + VERIFY_SUCCEEDED(utr->GetAttributeValue(FontWeight, &result)); + VERIFY_ARE_EQUAL(FW_NORMAL, result.iVal); + } + { + Log::Comment(L"Test Foreground"); + const auto foregroundColor{ RGB(255, 0, 0) }; + attr.SetForeground(foregroundColor); + updateBuffer(attr); + VARIANT result; + VERIFY_SUCCEEDED(utr->GetAttributeValue(ForegroundColor, &result)); + VERIFY_ARE_EQUAL(foregroundColor, static_cast(result.iVal)); + } + { + Log::Comment(L"Test Italic"); + attr.SetItalic(true); + updateBuffer(attr); + VARIANT result; + VERIFY_SUCCEEDED(utr->GetAttributeValue(Italic, &result)); + VERIFY_IS_TRUE(result.boolVal); + + attr.SetItalic(false); + updateBuffer(attr); + VERIFY_SUCCEEDED(utr->GetAttributeValue(Italic, &result)); + VERIFY_IS_FALSE(result.boolVal); + } + { + Log::Comment(L"Test Strikethrough"); + const auto foregroundColor{ attr.GetForeground().GetRGB() }; + attr.SetCrossedOut(true); + updateBuffer(attr); + VARIANT result; + VERIFY_SUCCEEDED(utr->GetAttributeValue(StrikethroughColor, &result)); + VERIFY_ARE_EQUAL(foregroundColor, static_cast(result.iVal)); + VERIFY_SUCCEEDED(utr->GetAttributeValue(StrikethroughStyle, &result)); + VERIFY_ARE_EQUAL(TextDecorationLineStyle_Single, result.iVal); + + attr.SetCrossedOut(false); + updateBuffer(attr); + VERIFY_SUCCEEDED(utr->GetAttributeValue(StrikethroughColor, &result)); + VERIFY_ARE_EQUAL(0, result.iVal); + VERIFY_SUCCEEDED(utr->GetAttributeValue(StrikethroughStyle, &result)); + VERIFY_ARE_EQUAL(TextDecorationLineStyle_None, result.iVal); + } + { + Log::Comment(L"Test Underline"); + const auto foregroundColor{ attr.GetForeground().GetRGB() }; + attr.SetUnderlined(true); + updateBuffer(attr); + VARIANT result; + VERIFY_SUCCEEDED(utr->GetAttributeValue(UnderlineColor, &result)); + VERIFY_ARE_EQUAL(foregroundColor, static_cast(result.iVal)); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UnderlineStyle, &result)); + VERIFY_ARE_EQUAL(TextDecorationLineStyle_Single, result.iVal); + + attr.SetUnderlined(false); + updateBuffer(attr); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UnderlineColor, &result)); + VERIFY_ARE_EQUAL(0, result.iVal); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UnderlineStyle, &result)); + VERIFY_ARE_EQUAL(TextDecorationLineStyle_None, result.iVal); + } + { + Log::Comment(L"Test Font Name (special)"); + VARIANT result; + VERIFY_SUCCEEDED(utr->GetAttributeValue(FontName, &result)); + const std::wstring actualFontName{ result.bstrVal }; + const auto expectedFontName{ _pUiaData->GetFontInfo().GetFaceName() }; + VERIFY_ARE_EQUAL(expectedFontName, actualFontName); + } + { + Log::Comment(L"Test Font Size (special)"); + VARIANT result; + VERIFY_SUCCEEDED(utr->GetAttributeValue(FontSize, &result)); + const auto actualFontSize{ result.dblVal }; + const auto expectedFontSize{ 0 }; //_pUiaData->GetFontInfo().GetUnscaledSize() }; + VERIFY_ARE_EQUAL(expectedFontSize, actualFontSize); + + // TODO CARLOS: Fix this test when we actually know how to convert the font size into points + VERIFY_IS_TRUE(false); + } + { + Log::Comment(L"Test Read Only (special)"); + VARIANT result; + VERIFY_SUCCEEDED(utr->GetAttributeValue(IsReadOnly, &result)); + VERIFY_IS_FALSE(result.boolVal); + } + } + + TEST_METHOD(FindAttribute) + { + Microsoft::WRL::ComPtr utr; + const COORD startPos{ 0, 0 }; + const COORD endPos{ 0, 2 }; + THROW_IF_FAILED(Microsoft::WRL::MakeAndInitialize(&utr, _pUiaData, &_dummyProvider, startPos, endPos)); + { + Log::Comment(L"Test Font Name (special)"); + + // Populate query with font name currently in use. + const auto fontName{ _pUiaData->GetFontInfo().GetFaceName() }; + VARIANT var{}; + var.vt = VT_BSTR; + var.bstrVal = SysAllocString(fontName.data()); + + Microsoft::WRL::ComPtr result; + VERIFY_SUCCEEDED(utr->FindAttribute(FontName, var, false, result.GetAddressOf())); + + // Expecting the same text range endpoints + BOOL isEqual; + THROW_IF_FAILED(utr->Compare(result.Get(), &isEqual)); + VERIFY_IS_TRUE(isEqual); + + // Now perform the same test, but searching backwards + Log::Comment(L"Test Font Name (special) - Backwards"); + Microsoft::WRL::ComPtr resultBackwards; + VERIFY_SUCCEEDED(utr->FindAttribute(FontName, var, true, resultBackwards.GetAddressOf())); + + // Expecting the same text range endpoints + THROW_IF_FAILED(result->Compare(resultBackwards.Get(), &isEqual)); + VERIFY_IS_TRUE(isEqual); + } + { + Log::Comment(L"Test Font Size (special)"); + + // Populate query with font size currently in use. + const auto fontSize{ 0.0 }; //{ _pUiaData->GetFontInfo().GetUnscaledSize() }; + VARIANT var{}; + var.vt = VT_R8; + var.dblVal = 0; + + Microsoft::WRL::ComPtr result; + VERIFY_SUCCEEDED(utr->FindAttribute(FontSize, var, false, result.GetAddressOf())); + + // Expecting the same text range endpoints + BOOL isEqual; + THROW_IF_FAILED(utr->Compare(result.Get(), &isEqual)); + VERIFY_IS_TRUE(isEqual); + + // Now perform the same test, but searching backwards + Log::Comment(L"Test Font Size (special) - Backwards"); + Microsoft::WRL::ComPtr resultBackwards; + VERIFY_SUCCEEDED(utr->FindAttribute(FontSize, var, true, resultBackwards.GetAddressOf())); + + // Expecting the same text range endpoints + THROW_IF_FAILED(result->Compare(resultBackwards.Get(), &isEqual)); + VERIFY_IS_TRUE(isEqual); + + // TODO CARLOS: Fix this test when we actually know how to convert the font size into points + VERIFY_IS_TRUE(false); + } + { + Log::Comment(L"Test Read Only (special)"); + + VARIANT var{}; + var.vt = VT_BOOL; + var.boolVal = false; + + Microsoft::WRL::ComPtr result; + VERIFY_SUCCEEDED(utr->FindAttribute(IsReadOnly, var, false, result.GetAddressOf())); + + // Expecting the same text range endpoints + BOOL isEqual; + THROW_IF_FAILED(utr->Compare(result.Get(), &isEqual)); + VERIFY_IS_TRUE(isEqual); + + // Now perform the same test, but searching backwards + Log::Comment(L"Test Read Only (special) - Backwards"); + Microsoft::WRL::ComPtr resultBackwards; + VERIFY_SUCCEEDED(utr->FindAttribute(IsReadOnly, var, true, resultBackwards.GetAddressOf())); + + // Expecting the same text range endpoints + THROW_IF_FAILED(result->Compare(resultBackwards.Get(), &isEqual)); + VERIFY_IS_TRUE(isEqual); + } + { + Log::Comment(L"Test IsItalic (standard attribute)"); + + // Since all of the other attributes operate very similarly, + // we're just going to pick one of them and test that. + // The "GetAttribute" tests provide code coverage for + // retrieving an attribute verification function. + // This test is intended to provide code coverage for + // finding a text range with the desired attribute. + + // Set up the buffer's attributes. + TextAttribute italicAttr; + italicAttr.SetItalic(true); + auto iter{ _pUiaData->GetTextBuffer().GetCellDataAt(startPos) }; + for (auto i = 0; i < 5; ++i) + { + _pTextBuffer->Write({ italicAttr }, iter.Pos()); + ++iter; + } + + // set the expected end (exclusive) + auto expectedEndPos{ iter.Pos() }; + _pUiaData->GetTextBuffer().GetSize().IncrementInBounds(expectedEndPos); + + VARIANT var{}; + var.vt = VT_BOOL; + var.boolVal = true; + + Microsoft::WRL::ComPtr result; + VERIFY_SUCCEEDED(utr->FindAttribute(Italic, var, false, result.GetAddressOf())); + + // TODO CARLOS: The "end" verification is failing. Here's a trace of what's going on: + // - UiaTextRangeBase: + // - checkIfAttrFound is properly set and used + // - the search loop properly sets the "start" + // - the search loop iterates through the entire utr because checkIfAttrFound + // continues to return true for the whole range + // I have no clue why this is happening. A fresh pair of eyes would be appreciated here :) + VERIFY_ARE_EQUAL(startPos, utr->_start); + VERIFY_ARE_EQUAL(expectedEndPos, utr->_end); + + // Now perform the same test, but searching backwards + Log::Comment(L"Test IsItalic (standard attribute) - Backwards"); + Microsoft::WRL::ComPtr resultBackwards; + VERIFY_SUCCEEDED(utr->FindAttribute(Italic, var, true, resultBackwards.GetAddressOf())); + + // Expecting the same text range endpoints + BOOL isEqual; + THROW_IF_FAILED(result->Compare(resultBackwards.Get(), &isEqual)); + VERIFY_IS_TRUE(isEqual); + } + } }; diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index 3d61d23f314..c77cbef2630 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -310,14 +310,319 @@ IFACEMETHODIMP UiaTextRangeBase::ExpandToEnclosingUnit(_In_ TextUnit unit) noexc CATCH_RETURN(); } -// we don't support this currently -IFACEMETHODIMP UiaTextRangeBase::FindAttribute(_In_ TEXTATTRIBUTEID /*textAttributeId*/, - _In_ VARIANT /*val*/, - _In_ BOOL /*searchBackward*/, - _Outptr_result_maybenull_ ITextRangeProvider** /*ppRetVal*/) noexcept +// Method Description: +// - Generate an attribute verification function for that attributeId and sub-type +// Arguments: +// - attributeId - the UIA text attribute identifier we're looking for +// - val - the attributeId's sub-type we're looking for +// Return Value: +// - a function that can be used to verify if a given TextAttribute meets the attributeId's sub-type +std::function UiaTextRangeBase::_getAttrVerificationFn(TEXTATTRIBUTEID attributeId, VARIANT val) const { - UiaTracing::TextRange::FindAttribute(*this); - return E_NOTIMPL; + // Most of the attributes we're looking for just require us to check TextAttribute. + // So if we support it, we'll return a function to verify if the TextAttribute + // has the desired attribute. + switch (attributeId) + { + case UIA_FontWeightAttributeId: + { + // Expected type: VT_I4 + THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); + + // The font weight can be any value from 0 to 900. + // The text buffer doesn't store the actual value, + // we just store "IsBold" and "IsFaint". + const auto queryFontWeight{ val.iVal }; + + if (queryFontWeight > FW_NORMAL) + { + // we're looking for a bold font weight + return [](const TextAttribute& attr) { + return attr.IsBold(); + }; + } + else if (queryFontWeight < FW_NORMAL) + { + // we're looking for a faint font weight + return [](const TextAttribute& attr) { + return attr.IsFaint(); + }; + } + else + { + // we're looking for "normal" font weight + return [](const TextAttribute& attr) { + return !attr.IsBold() && !attr.IsFaint(); + }; + } + } + case UIA_ForegroundColorAttributeId: + { + // Expected type: VT_I4 + THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); + + // The foreground color is stored as a COLORREF. + const COLORREF queryForegroundColor{ base::ClampedNumeric(val.iVal) }; + return [queryForegroundColor](const TextAttribute& attr) { + return attr.GetForeground().GetRGB() == queryForegroundColor; + }; + } + case UIA_IsItalicAttributeId: + { + // Expected type: VT_I4 + THROW_HR_IF(E_INVALIDARG, val.vt != VT_BOOL); + + // The text is either italic or it isn't. + const auto queryIsItalic{ val.boolVal }; + if (queryIsItalic) + { + return [](const TextAttribute& attr) { + return attr.IsItalic(); + }; + } + else + { + return [](const TextAttribute& attr) { + return !attr.IsItalic(); + }; + } + } + case UIA_StrikethroughColorAttributeId: + { + // Expected type: VT_I4 + THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); + + // The strikethrough color is stored as a COLORREF. + // However, the text buffer doesn't actually store the color. + // Instead, we just store whether or not the text is crossed out. + // So we'll assume that _any_ query here (except 0) is interpreted as "is the + // text crossed out?" + + if (val.iVal == 0) + { + return [](const TextAttribute& attr) { + return !attr.IsCrossedOut(); + }; + } + else + { + return [](const TextAttribute& attr) { + return attr.IsCrossedOut(); + }; + } + } + case UIA_StrikethroughStyleAttributeId: + { + // Expected type: VT_I4 + THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); + + // The strikethrough style is stored as a TextDecorationLineStyle. + // However, The text buffer doesn't have different styles for being crossed out. + // Instead, we just store whether or not the text is crossed out. + // So we'll assume that _any_ query here is interpreted as "is the + // text crossed out?" UNLESS we are asked for TextDecorationLineStyle_None + const TextDecorationLineStyle queryStrikethroughStyle{ static_cast(val.iVal) }; + if (queryStrikethroughStyle == TextDecorationLineStyle_None) + { + return [](const TextAttribute& attr) { + return !attr.IsCrossedOut(); + }; + } + else + { + return [](const TextAttribute& attr) { + return attr.IsCrossedOut(); + }; + } + } + case UIA_UnderlineColorAttributeId: + { + // Expected type: VT_I4 + THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); + + // The underline color is stored as a COLORREF. + // However, the text buffer doesn't actually store the color. + // Instead, we just store whether or not the text is underlined. + // So we'll assume that _any_ query here (except 0) is interpreted as "is the + // text underlined?" + + if (val.iVal == 0) + { + return [](const TextAttribute& attr) { + return !attr.IsUnderlined(); + }; + } + else + { + return [](const TextAttribute& attr) { + return attr.IsUnderlined(); + }; + } + } + case UIA_UnderlineStyleAttributeId: + { + // Expected type: VT_I4 + THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); + + // The underline style is stored as a TextDecorationLineStyle. + // However, The text buffer doesn't have different styles for being underlined. + // Instead, we just store whether or not the text is underlined. + // So we'll assume that _any_ query here is interpreted as "is the + // text underlined?" UNLESS we are asked for TextDecorationLineStyle_None + const TextDecorationLineStyle queryUnderlineStyle{ static_cast(val.iVal) }; + if (queryUnderlineStyle == TextDecorationLineStyle_None) + { + return [](const TextAttribute& attr) { + return !attr.IsUnderlined(); + }; + } + else + { + return [](const TextAttribute& attr) { + return attr.IsUnderlined(); + }; + } + } + default: + return nullptr; + } +} + +IFACEMETHODIMP UiaTextRangeBase::FindAttribute(_In_ TEXTATTRIBUTEID attributeId, + _In_ VARIANT val, + _In_ BOOL searchBackwards, + _Outptr_result_maybenull_ ITextRangeProvider** ppRetVal) noexcept +{ + RETURN_HR_IF(E_INVALIDARG, ppRetVal == nullptr); + *ppRetVal = nullptr; + + // AttributeIDs that require special handling + switch (attributeId) + { + case UIA_FontNameAttributeId: + { + RETURN_HR_IF(E_INVALIDARG, val.vt != VT_BSTR); + const std::wstring queryFontName{ val.bstrVal }; + if (queryFontName == _pData->GetFontInfo().GetFaceName()) + { + Clone(ppRetVal); + } + UiaTracing::TextRange::FindAttribute(*this, attributeId, val, searchBackwards, static_cast(**ppRetVal)); + return S_OK; + } + case UIA_FontSizeAttributeId: + { + RETURN_HR_IF(E_INVALIDARG, val.vt != VT_R8); + const auto queryFontSize{ val.dblVal }; + + // TODO CARLOS: how do I get the font size in points? + if (queryFontSize == 0) //_pData->GetFontInfo().GetUnscaledSize()) + { + Clone(ppRetVal); + } + UiaTracing::TextRange::FindAttribute(*this, attributeId, val, searchBackwards, static_cast(**ppRetVal)); + return S_OK; + } + case UIA_IsReadOnlyAttributeId: + { + RETURN_HR_IF(E_INVALIDARG, val.vt != VT_BOOL); + if (!val.boolVal) + { + Clone(ppRetVal); + } + UiaTracing::TextRange::FindAttribute(*this, attributeId, val, searchBackwards, static_cast(**ppRetVal)); + return S_OK; + } + default: + __fallthrough; + } + + // AttributeIDs that are exposed via TextAttribute + std::function checkIfAttrFound; + try + { + checkIfAttrFound = _getAttrVerificationFn(attributeId, val); + if (!checkIfAttrFound) + { + // The AttributeID is not supported. + UiaTracing::TextRange::FindAttribute(*this, attributeId, val, searchBackwards, static_cast(**ppRetVal), UiaTracing::AttributeType::Unsupported); + return E_NOTIMPL; + } + } + catch (...) + { + LOG_HR(wil::ResultFromCaughtException()); + UiaTracing::TextRange::FindAttribute(*this, attributeId, val, searchBackwards, static_cast(**ppRetVal), UiaTracing::AttributeType::Error); + return E_INVALIDARG; + } + + // Get some useful variables + const auto& buffer{ _pData->GetTextBuffer() }; + const auto bufferSize{ buffer.GetSize() }; + const auto inclusiveEnd{ _getInclusiveEnd() }; + + // Start/End for the resulting range. + // NOTE: we store these as "first" and "second" anchor because, + // we just want to know what the inclusive range is. + // We'll do some post-processing to fix this on the way out. + std::optional resultFirstAnchor; + std::optional resultSecondAnchor; + + // Start/End for the direction to perform the search in + const auto searchStart{ searchBackwards ? inclusiveEnd : _start }; + const auto searchEnd{ searchBackwards ? _start : inclusiveEnd }; + + // Iterate from searchStart to searchEnd in the buffer. + // If we find the attribute we're looking for, we update resultFirstAnchor/SecondAnchor appropriately. + const Viewport viewportRange{ _blockRange ? Viewport::FromDimensions(_start, inclusiveEnd.X - _start.X + 1, inclusiveEnd.Y - _start.Y + 1) : bufferSize }; + auto iter{ buffer.GetCellDataAt(searchStart, viewportRange, searchEnd) }; + for (; iter; searchBackwards ? --iter : ++iter) + { + const auto& attr{ iter->TextAttr() }; + if (checkIfAttrFound(attr)) + { + // populate the first anchor if it's not populated. + // otherwise, populate the second anchor. + if (!resultFirstAnchor.has_value()) + { + resultFirstAnchor = iter.Pos(); + } + else + { + resultSecondAnchor = iter.Pos(); + } + } + else if (resultFirstAnchor.has_value() && resultSecondAnchor.has_value()) + { + // Exit the loop early if... + // - the cell we're looking at doesn't have the attr we're looking for + // - the anchors have been populated + // This means that we've found a contiguous range where the text attribute was found. + // No point in searching through the rest of the search space. + break; + } + } + + // If a result was found, populate ppRetVal with the UiaTextRange + // representing the found selection anchors. + if (resultFirstAnchor.has_value() && resultSecondAnchor.has_value()) + { + RETURN_IF_FAILED(Clone(ppRetVal)); + UiaTextRangeBase& range = static_cast(**ppRetVal); + + // IMPORTANT: resultFirstAnchor and resultSecondAnchor make up an inclusive range. + range._start = searchBackwards ? *resultSecondAnchor : *resultFirstAnchor; + range._end = searchBackwards ? *resultFirstAnchor : *resultSecondAnchor; + + // We need to make the end exclusive! + // But be careful here, we might be a block range + auto exclusiveIter{ buffer.GetCellDataAt(range._end, viewportRange) }; + exclusiveIter++; + range._end = exclusiveIter.Pos(); + } + + UiaTracing::TextRange::FindAttribute(*this, attributeId, val, searchBackwards, static_cast(**ppRetVal)); + return S_OK; } IFACEMETHODIMP UiaTextRangeBase::FindText(_In_ BSTR text, @@ -372,22 +677,168 @@ try } CATCH_RETURN(); -IFACEMETHODIMP UiaTextRangeBase::GetAttributeValue(_In_ TEXTATTRIBUTEID textAttributeId, +// Method Description: +// - (1) Checks the current range for the attributeId's sub-type +// - (2) Record the attributeId's sub-type +// - (3) Generate an attribute verification function for that attributeId sub-type +// Arguments: +// - attributeId - the UIA text attribute identifier we're looking for +// - pRetVal - the attributeId's sub-type for the first cell in the range (i.e. foreground color) +// Return Value: +// - a function that can be used to verify if a given TextAttribute meets the attributeId's sub-type +std::function UiaTextRangeBase::_getAttrVerificationFnForFirstAttr(TEXTATTRIBUTEID attributeId, VARIANT* pRetVal) const +{ + THROW_HR_IF(E_INVALIDARG, pRetVal == nullptr); + + const auto attr{ _pData->GetTextBuffer().GetCellDataAt(_start)->TextAttr() }; + switch (attributeId) + { + case UIA_FontWeightAttributeId: + { + // The font weight can be any value from 0 to 900. + // The text buffer doesn't store the actual value, + // we just store "IsBold" and "IsFaint". + // Source: https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids + pRetVal->vt = VT_I4; + if (attr.IsBold()) + { + pRetVal->iVal = FW_BOLD; + } + else if (attr.IsFaint()) + { + pRetVal->iVal = FW_LIGHT; + } + else + { + pRetVal->iVal = FW_NORMAL; + } + return _getAttrVerificationFn(attributeId, *pRetVal); + } + case UIA_ForegroundColorAttributeId: + { + pRetVal->vt = VT_I4; + pRetVal->iVal = base::ClampedNumeric(attr.GetForeground().GetRGB()); + return _getAttrVerificationFn(attributeId, *pRetVal); + } + case UIA_IsItalicAttributeId: + { + pRetVal->vt = VT_BOOL; + pRetVal->iVal = attr.IsItalic(); + return _getAttrVerificationFn(attributeId, *pRetVal); + } + case UIA_StrikethroughColorAttributeId: + { + // Default Value: 0 + // Source: https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids + pRetVal->vt = VT_I4; + pRetVal->iVal = base::ClampedNumeric(attr.IsCrossedOut() ? attr.GetForeground().GetRGB() : 0); + return _getAttrVerificationFn(attributeId, *pRetVal); + } + case UIA_StrikethroughStyleAttributeId: + { + pRetVal->vt = VT_I4; + pRetVal->iVal = static_cast(attr.IsCrossedOut() ? TextDecorationLineStyle_Single : TextDecorationLineStyle_None); + return _getAttrVerificationFn(attributeId, *pRetVal); + } + case UIA_UnderlineColorAttributeId: + { + // Default Value: 0 + // Source: https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids + pRetVal->vt = VT_I4; + pRetVal->iVal = attr.IsUnderlined() ? base::ClampedNumeric(attr.GetForeground().GetRGB()) : 0; + return _getAttrVerificationFn(attributeId, *pRetVal); + } + case UIA_UnderlineStyleAttributeId: + { + pRetVal->vt = VT_I4; + pRetVal->iVal = static_cast(attr.IsUnderlined() ? TextDecorationLineStyle_Single : TextDecorationLineStyle_None); + return _getAttrVerificationFn(attributeId, *pRetVal); + } + default: + // This attribute is not supported. + pRetVal->vt = VT_UNKNOWN; + UiaGetReservedNotSupportedValue(&pRetVal->punkVal); + return nullptr; + } +} + +IFACEMETHODIMP UiaTextRangeBase::GetAttributeValue(_In_ TEXTATTRIBUTEID attributeId, _Out_ VARIANT* pRetVal) noexcept { RETURN_HR_IF(E_INVALIDARG, pRetVal == nullptr); - if (textAttributeId == UIA_IsReadOnlyAttributeId) + // AttributeIDs that require special handling + switch (attributeId) + { + case UIA_FontNameAttributeId: + { + pRetVal->vt = VT_BSTR; + pRetVal->bstrVal = SysAllocString(_pData->GetFontInfo().GetFaceName().data()); + UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal); + return S_OK; + } + case UIA_FontSizeAttributeId: + { + pRetVal->vt = VT_R8; + // TODO CARLOS: how do I get the font size in points? + pRetVal->dblVal = 0; //_pData->GetFontInfo().GetUnscaledSize()) + UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal); + return S_OK; + } + case UIA_IsReadOnlyAttributeId: { pRetVal->vt = VT_BOOL; pRetVal->boolVal = VARIANT_FALSE; + UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal); + return S_OK; } - else + default: + __fallthrough; + } + + // AttributeIDs that are exposed via TextAttribute + std::function checkIfAttrFound; + try { - pRetVal->vt = VT_UNKNOWN; - UiaGetReservedNotSupportedValue(&pRetVal->punkVal); + checkIfAttrFound = _getAttrVerificationFnForFirstAttr(attributeId, pRetVal); + if (!checkIfAttrFound) + { + // The AttributeID is not supported. + pRetVal->vt = VT_UNKNOWN; + UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal, UiaTracing::AttributeType::Unsupported); + return UiaGetReservedNotSupportedValue(&pRetVal->punkVal); + } + } + catch (...) + { + LOG_HR(wil::ResultFromCaughtException()); + UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal, UiaTracing::AttributeType::Error); + return E_INVALIDARG; } - UiaTracing::TextRange::GetAttributeValue(*this, textAttributeId, *pRetVal); + + // Get some useful variables + const auto& buffer{ _pData->GetTextBuffer() }; + const auto bufferSize{ buffer.GetSize() }; + const auto inclusiveEnd{ _getInclusiveEnd() }; + + // Check if the entire text range has that text attribute + const Viewport viewportRange{ _blockRange ? Viewport::FromDimensions(_start, inclusiveEnd.X - _start.X + 1, inclusiveEnd.Y - _start.Y + 1) : bufferSize }; + auto iter{ buffer.GetCellDataAt(_start, viewportRange, inclusiveEnd) }; + for (; iter; ++iter) + { + const auto& attr{ iter->TextAttr() }; + if (!checkIfAttrFound(attr)) + { + // The value of the specified attribute varies over the text range + // return UiaGetReservedMixedAttributeValue. + // Source: https://docs.microsoft.com/en-us/windows/win32/api/uiautomationcore/nf-uiautomationcore-itextrangeprovider-getattributevalue + pRetVal->vt = VT_UNKNOWN; + UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal, UiaTracing::AttributeType::Mixed); + return UiaGetReservedMixedAttributeValue(&pRetVal->punkVal); + } + } + + UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal); return S_OK; } @@ -1244,3 +1695,10 @@ RECT UiaTextRangeBase::_getTerminalRect() const gsl::narrow(result.top + result.height) }; } + +COORD UiaTextRangeBase::_getInclusiveEnd() +{ + auto result{ _end }; + _pData->GetTextBuffer().GetSize().DecrementInBounds(result, true); + return result; +} diff --git a/src/types/UiaTextRangeBase.hpp b/src/types/UiaTextRangeBase.hpp index 7122cefcf44..19a3c130c16 100644 --- a/src/types/UiaTextRangeBase.hpp +++ b/src/types/UiaTextRangeBase.hpp @@ -177,6 +177,11 @@ namespace Microsoft::Console::Types gsl::not_null const pAmountMoved, _In_ const bool preventBufferEnd = false) noexcept; + std::function _getAttrVerificationFn(TEXTATTRIBUTEID attributeId, VARIANT val) const; + std::function _getAttrVerificationFnForFirstAttr(TEXTATTRIBUTEID attributeId, VARIANT* pRetVal) const; + + COORD _getInclusiveEnd(); + #ifdef UNIT_TESTING friend class ::UiaTextRangeTests; #endif diff --git a/src/types/UiaTracing.cpp b/src/types/UiaTracing.cpp index efee217d0b9..461fccb6906 100644 --- a/src/types/UiaTracing.cpp +++ b/src/types/UiaTracing.cpp @@ -126,6 +126,140 @@ inline std::wstring UiaTracing::_getValue(const TextUnit unit) noexcept } } +std::wstring UiaTracing::convertAttributeId(const TEXTATTRIBUTEID attrId) noexcept +{ + // Source: https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids + switch (attrId) + { + case UIA_AfterParagraphSpacingAttributeId: + return L"AfterParagraphSpacing"; + case UIA_AnimationStyleAttributeId: + return L"AnimationStyle"; + case UIA_AnnotationObjectsAttributeId: + return L"AnnotationObjects"; + case UIA_AnnotationTypesAttributeId: + return L"AnnotationTypes"; + case UIA_BackgroundColorAttributeId: + return L"BackgroundColor"; + case UIA_BeforeParagraphSpacingAttributeId: + return L"BeforeParagraphSpacing"; + case UIA_BulletStyleAttributeId: + return L"BulletStyle"; + case UIA_CapStyleAttributeId: + return L"CapStyle"; + case UIA_CaretBidiModeAttributeId: + return L"CaretBidiMode"; + case UIA_CaretPositionAttributeId: + return L"CaretPosition"; + case UIA_CultureAttributeId: + return L"Culture"; + case UIA_FontNameAttributeId: + return L"FontName"; + case UIA_FontSizeAttributeId: + return L"FontSize"; + case UIA_FontWeightAttributeId: + return L"FontWeight"; + case UIA_ForegroundColorAttributeId: + return L"ForegroundColor"; + case UIA_HorizontalTextAlignmentAttributeId: + return L"HorizontalTextAlignment"; + case UIA_IndentationFirstLineAttributeId: + return L"IndentationFirstLine"; + case UIA_IndentationLeadingAttributeId: + return L"IndentationLeading"; + case UIA_IndentationTrailingAttributeId: + return L"IndentationTrailing"; + case UIA_IsActiveAttributeId: + return L"IsActive"; + case UIA_IsHiddenAttributeId: + return L"IsHidden"; + case UIA_IsItalicAttributeId: + return L"IsItalic"; + case UIA_IsReadOnlyAttributeId: + return L"IsReadOnly"; + case UIA_IsSubscriptAttributeId: + return L"IsSubscript"; + case UIA_IsSuperscriptAttributeId: + return L"IsSuperscript"; + case UIA_LineSpacingAttributeId: + return L"LineSpacing"; + case UIA_LinkAttributeId: + return L"Link"; + case UIA_MarginBottomAttributeId: + return L"MarginBottom"; + case UIA_MarginLeadingAttributeId: + return L"MarginLeading"; + case UIA_MarginTopAttributeId: + return L"MarginTop"; + case UIA_MarginTrailingAttributeId: + return L"MarginTrailing"; + case UIA_OutlineStylesAttributeId: + return L"OutlineStyles"; + case UIA_OverlineColorAttributeId: + return L"OverlineColor"; + case UIA_OverlineStyleAttributeId: + return L"OverlineStyle"; + case UIA_SelectionActiveEndAttributeId: + return L"SelectionActiveEnd"; + case UIA_StrikethroughColorAttributeId: + return L"StrikethroughColor"; + case UIA_StrikethroughStyleAttributeId: + return L"StrikethroughStyle"; + case UIA_StyleIdAttributeId: + return L"StyleId"; + case UIA_StyleNameAttributeId: + return L"StyleName"; + case UIA_TabsAttributeId: + return L"Tabs"; + case UIA_TextFlowDirectionsAttributeId: + return L"TextFlowDirections"; + case UIA_UnderlineColorAttributeId: + return L"UnderlineColor"; + case UIA_UnderlineStyleAttributeId: + return L"UnderlineStyle"; + default: + return L"Unknown attribute"; + } +} + +inline std::wstring UiaTracing::_getValue(const VARIANT val) noexcept +{ + // This is not a comprehensive conversion of VARIANT result to string + // We're only including the one's we need at this time. + switch (val.vt) + { + case VT_BSTR: + return val.bstrVal; + case VT_R8: + return std::to_wstring(val.dblVal); + case VT_BOOL: + return std::to_wstring(val.boolVal); + case VT_I4: + return std::to_wstring(val.iVal); + case VT_UNKNOWN: + default: + { + return L"unknown"; + } + } +} + +inline std::wstring UiaTracing::_getValue(const AttributeType attrType) noexcept +{ + switch (attrType) + { + case AttributeType::Mixed: + return L"Mixed"; + case AttributeType::Unsupported: + return L"Unsupported"; + case AttributeType::Error: + return L"Error"; + case AttributeType::Standard: + default: + return L"Standard"; + } +} + void UiaTracing::TextRange::Constructor(UiaTextRangeBase& result) noexcept { EnsureRegistration(); @@ -206,15 +340,20 @@ void UiaTracing::TextRange::ExpandToEnclosingUnit(TextUnit unit, const UiaTextRa } } -void UiaTracing::TextRange::FindAttribute(const UiaTextRangeBase& utr) noexcept +void UiaTracing::TextRange::FindAttribute(const UiaTextRangeBase& utr, TEXTATTRIBUTEID id, VARIANT val, BOOL searchBackwards, const UiaTextRangeBase& result, AttributeType attrType) noexcept { EnsureRegistration(); if (TraceLoggingProviderEnabled(g_UiaProviderTraceProvider, WINEVENT_LEVEL_VERBOSE, TIL_KEYWORD_TRACE)) { TraceLoggingWrite( g_UiaProviderTraceProvider, - "UiaTextRange::FindAttribute (UNSUPPORTED)", + "UiaTextRange::FindAttribute", TraceLoggingValue(_getValue(utr).c_str(), "base"), + TraceLoggingValue(convertAttributeId(id).c_str(), "text attribute ID"), + TraceLoggingValue(_getValue(val).c_str(), "text attribute sub-data"), + TraceLoggingValue(searchBackwards ? L"true" : L"false", "search backwards"), + TraceLoggingValue(_getValue(attrType).c_str(), "attribute type"), + TraceLoggingValue(_getValue(result).c_str(), "result"), TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); } @@ -238,7 +377,7 @@ void UiaTracing::TextRange::FindText(const UiaTextRangeBase& base, std::wstring } } -void UiaTracing::TextRange::GetAttributeValue(const UiaTextRangeBase& base, TEXTATTRIBUTEID id, VARIANT result) noexcept +void UiaTracing::TextRange::GetAttributeValue(const UiaTextRangeBase& base, TEXTATTRIBUTEID id, VARIANT result, AttributeType attrType) noexcept { EnsureRegistration(); if (TraceLoggingProviderEnabled(g_UiaProviderTraceProvider, WINEVENT_LEVEL_VERBOSE, TIL_KEYWORD_TRACE)) @@ -247,8 +386,9 @@ void UiaTracing::TextRange::GetAttributeValue(const UiaTextRangeBase& base, TEXT g_UiaProviderTraceProvider, "UiaTextRange::GetAttributeValue", TraceLoggingValue(_getValue(base).c_str(), "base"), - TraceLoggingValue(id, "textAttributeId"), - TraceLoggingValue(result.vt, "result (type)"), + TraceLoggingValue(convertAttributeId(id).c_str(), "text attribute ID"), + TraceLoggingValue(_getValue(result).c_str(), "result"), + TraceLoggingValue(_getValue(attrType).c_str(), "attribute type"), TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); } diff --git a/src/types/UiaTracing.h b/src/types/UiaTracing.h index 6a8c0a00885..a1bd20a04a2 100644 --- a/src/types/UiaTracing.h +++ b/src/types/UiaTracing.h @@ -29,6 +29,16 @@ namespace Microsoft::Console::Types class UiaTracing final { public: + static std::wstring convertAttributeId(const TEXTATTRIBUTEID attrId) noexcept; + + enum class AttributeType + { + Standard, + Mixed, + Unsupported, + Error + }; + class TextRange final { public: @@ -37,9 +47,9 @@ namespace Microsoft::Console::Types static void Compare(const UiaTextRangeBase& base, const UiaTextRangeBase& other, bool result) noexcept; static void CompareEndpoints(const UiaTextRangeBase& base, const TextPatternRangeEndpoint endpoint, const UiaTextRangeBase& other, TextPatternRangeEndpoint otherEndpoint, int result) noexcept; static void ExpandToEnclosingUnit(TextUnit unit, const UiaTextRangeBase& result) noexcept; - static void FindAttribute(const UiaTextRangeBase& base) noexcept; + static void FindAttribute(const UiaTextRangeBase& base, TEXTATTRIBUTEID attributeId, VARIANT val, BOOL searchBackwards, const UiaTextRangeBase& result, AttributeType attrType = AttributeType::Standard) noexcept; static void FindText(const UiaTextRangeBase& base, std::wstring text, bool searchBackward, bool ignoreCase, const UiaTextRangeBase& result) noexcept; - static void GetAttributeValue(const UiaTextRangeBase& base, TEXTATTRIBUTEID id, VARIANT result) noexcept; + static void GetAttributeValue(const UiaTextRangeBase& base, TEXTATTRIBUTEID id, VARIANT result, AttributeType attrType = AttributeType::Standard) noexcept; static void GetBoundingRectangles(const UiaTextRangeBase& base) noexcept; static void GetEnclosingElement(const UiaTextRangeBase& base) noexcept; static void GetText(const UiaTextRangeBase& base, int maxLength, std::wstring result) noexcept; @@ -101,6 +111,9 @@ namespace Microsoft::Console::Types static inline std::wstring _getValue(const TextPatternRangeEndpoint endpoint) noexcept; static inline std::wstring _getValue(const TextUnit unit) noexcept; + static inline std::wstring _getValue(const AttributeType attrType) noexcept; + static inline std::wstring _getValue(const VARIANT val) noexcept; + // these are used to assign IDs to new UiaTextRanges and ScreenInfoUiaProviders respectively static IdType _utrId; static IdType _siupId; From 136a54898b6965fc060921602fc0ce4c7cf96c03 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 22 Jun 2021 16:29:41 -0700 Subject: [PATCH 2/9] fix/add tests; remove invalid attributes --- src/buffer/out/textBuffer.cpp | 34 +-- src/buffer/out/textBuffer.hpp | 6 +- src/buffer/out/textBufferCellIterator.cpp | 39 ++-- src/buffer/out/textBufferCellIterator.hpp | 6 +- src/cascadia/TerminalControl/ControlCore.cpp | 2 + .../TerminalControl/XamlUiaTextRange.cpp | 24 +- src/cascadia/TerminalCore/Terminal.hpp | 4 +- .../TerminalCore/terminalrenderdata.cpp | 10 + src/host/renderData.hpp | 3 +- src/host/ut_host/TextBufferIteratorTests.cpp | 138 ++++++++++-- .../UiaTextRangeTests.cpp | 207 +++++++----------- src/renderer/inc/IRenderData.hpp | 2 - src/types/IBaseData.h | 1 + src/types/UiaTextRangeBase.cpp | 195 ++++++----------- 14 files changed, 346 insertions(+), 325 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index d322c871a36..e56d4f7c630 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -149,13 +149,13 @@ TextBufferTextIterator TextBuffer::GetTextLineDataAt(const COORD at) const // - Read-only iterator of cell data. TextBufferCellIterator TextBuffer::GetCellLineDataAt(const COORD at) const { - SMALL_RECT limit; - limit.Top = at.Y; - limit.Bottom = at.Y; - limit.Left = 0; - limit.Right = GetSize().RightInclusive(); + SMALL_RECT bounds; + bounds.Top = at.Y; + bounds.Bottom = at.Y; + bounds.Left = 0; + bounds.Right = GetSize().RightInclusive(); - return TextBufferCellIterator(*this, at, Viewport::FromInclusive(limit)); + return TextBufferCellIterator(*this, at, Viewport::FromInclusive(bounds)); } // Routine Description: @@ -163,12 +163,12 @@ TextBufferCellIterator TextBuffer::GetCellLineDataAt(const COORD at) const // but restricted to operate only inside the given viewport. // Arguments: // - at - X,Y position in buffer for iterator start position -// - limit - boundaries for the iterator to operate within +// - bounds - boundaries for the iterator to operate within // Return Value: // - Read-only iterator of text data only. -TextBufferTextIterator TextBuffer::GetTextDataAt(const COORD at, const Viewport limit) const +TextBufferTextIterator TextBuffer::GetTextDataAt(const COORD at, const Viewport bounds) const { - return TextBufferTextIterator(GetCellDataAt(at, limit)); + return TextBufferTextIterator(GetCellDataAt(at, bounds)); } // Routine Description: @@ -176,12 +176,12 @@ TextBufferTextIterator TextBuffer::GetTextDataAt(const COORD at, const Viewport // but restricted to operate only inside the given viewport. // Arguments: // - at - X,Y position in buffer for iterator start position -// - limit - boundaries for the iterator to operate within +// - bounds - boundaries for the iterator to operate within // Return Value: // - Read-only iterator of cell data. -TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport limit) const +TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport bounds) const { - return TextBufferCellIterator(*this, at, limit); + return TextBufferCellIterator(*this, at, bounds); } // Routine Description: @@ -189,13 +189,15 @@ TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport // but restricted to operate only inside the given viewport. // Arguments: // - at - X,Y position in buffer for iterator start position -// - limit - boundaries for the iterator to operate within -// - until - X,Y position in buffer for last position for the iterator to read (inclusive) +// - bounds - viewport boundaries for the iterator to operate within. +// Allows for us to iterate over a sub-grid of the buffer. +// - limit - X,Y position in buffer for the iterator end position (inclusive). +// Allows for us to iterate through "bounds" until we hit the end of "bounds" or the limit. // Return Value: // - Read-only iterator of cell data. -TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport limit, const COORD until) const +TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport bounds, const COORD limit) const { - return TextBufferCellIterator(*this, at, limit, until); + return TextBufferCellIterator(*this, at, bounds, limit); } //Routine Description: diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 702a57d493b..d84b8d6c5d0 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -80,11 +80,11 @@ class TextBuffer final TextBufferCellIterator GetCellDataAt(const COORD at) const; TextBufferCellIterator GetCellLineDataAt(const COORD at) const; - TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit) const; - TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit, const COORD until) const; + TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport bounds) const; + TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport bounds, const COORD limit) const; TextBufferTextIterator GetTextDataAt(const COORD at) const; TextBufferTextIterator GetTextLineDataAt(const COORD at) const; - TextBufferTextIterator GetTextDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit) const; + TextBufferTextIterator GetTextDataAt(const COORD at, const Microsoft::Console::Types::Viewport bounds) const; // Text insertion functions OutputCellIterator Write(const OutputCellIterator givenIt); diff --git a/src/buffer/out/textBufferCellIterator.cpp b/src/buffer/out/textBufferCellIterator.cpp index a8402f7b8a8..ba6b04ebc0b 100644 --- a/src/buffer/out/textBufferCellIterator.cpp +++ b/src/buffer/out/textBufferCellIterator.cpp @@ -29,21 +29,21 @@ TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD p // Arguments: // - buffer - Pointer to screen buffer to seek through // - pos - Starting position to retrieve text data from (within screen buffer bounds) -// - limits - Viewport limits to restrict the iterator within the buffer bounds (smaller than the buffer itself) -TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport limits) : +// - bounds - Viewport boundaries to restrict the iterator within the buffer bounds (smaller than the buffer itself) +TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport bounds) : _buffer(buffer), _pos(pos), _pRow(s_GetRow(buffer, pos)), - _bounds(limits), + _bounds(bounds), _exceeded(false), _view({}, {}, {}, TextAttributeBehavior::Stored), _attrIter(s_GetRow(buffer, pos)->GetAttrRow().cbegin()) { // Throw if the bounds rectangle is not limited to the inside of the given buffer. - THROW_HR_IF(E_INVALIDARG, !buffer.GetSize().IsInBounds(limits)); + THROW_HR_IF(E_INVALIDARG, !buffer.GetSize().IsInBounds(bounds)); // Throw if the coordinate is not limited to the inside of the given buffer. - THROW_HR_IF(E_INVALIDARG, !limits.IsInBounds(pos)); + THROW_HR_IF(E_INVALIDARG, !bounds.IsInBounds(pos)); _attrIter += pos.X; @@ -55,18 +55,15 @@ TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD p // Arguments: // - buffer - Text buffer to seek through // - pos - Starting position to retrieve text data from (within screen buffer bounds) -// - limits - Viewport limits to restrict the iterator within the buffer bounds (smaller than the buffer itself) -// - endPosInclusive - last position to iterate through (inclusive) -TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport limits, const COORD endPosInclusive) : - TextBufferCellIterator(buffer, pos, limits) +// - bounds - Viewport boundaries to restrict the iterator within the buffer bounds (smaller than the buffer itself) +// - limit - last position to iterate through (inclusive) +TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport bounds, const COORD limit) : + TextBufferCellIterator(buffer, pos, bounds) { // Throw if the coordinate is not limited to the inside of the given buffer. - THROW_HR_IF(E_INVALIDARG, !_bounds.IsInBounds(endPosInclusive)); + THROW_HR_IF(E_INVALIDARG, !_bounds.IsInBounds(limit)); - // Throw if pos is past endPos - THROW_HR_IF(E_INVALIDARG, _bounds.CompareInBounds(pos, endPosInclusive) > 0); - - _endPosInclusive = endPosInclusive; + _limit = limit; } // Routine Description: @@ -92,7 +89,7 @@ bool TextBufferCellIterator::operator==(const TextBufferCellIterator& it) const _bounds == it._bounds && _pRow == it._pRow && _attrIter == it._attrIter && - _endPosInclusive == _endPosInclusive; + _limit == _limit; } // Routine Description: @@ -118,22 +115,22 @@ TextBufferCellIterator& TextBufferCellIterator::operator+=(const ptrdiff_t& move auto newPos = _pos; while (move > 0 && !_exceeded) { - // If we have an endPos, check if we've exceeded it - if (_endPosInclusive.has_value()) + // If we have a limit, check if we've exceeded it + if (_limit.has_value()) { - _exceeded = _bounds.CompareInBounds(newPos, *_endPosInclusive) > 0; + _exceeded |= (newPos == _limit); } - // If we already exceeded from endPos, we'll short-circuit and _not_ increment + // If we already exceeded limit, we'll short-circuit and _not_ increment _exceeded |= !_bounds.IncrementInBounds(newPos); move--; } while (move < 0 && !_exceeded) { // If we have an endPos, check if we've exceeded it - if (_endPosInclusive.has_value()) + if (_limit.has_value()) { - _exceeded = _bounds.CompareInBounds(newPos, *_endPosInclusive) < 0; + _exceeded |= (newPos == _limit); } // If we already exceeded from endPos, we'll short-circuit and _not_ decrement diff --git a/src/buffer/out/textBufferCellIterator.hpp b/src/buffer/out/textBufferCellIterator.hpp index c5fe69142e8..1877834412d 100644 --- a/src/buffer/out/textBufferCellIterator.hpp +++ b/src/buffer/out/textBufferCellIterator.hpp @@ -26,8 +26,8 @@ class TextBufferCellIterator { public: TextBufferCellIterator(const TextBuffer& buffer, COORD pos); - TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport limits); - TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport limits, const COORD endPosInclusive); + TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport bounds); + TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport bounds, const COORD limit); operator bool() const noexcept; @@ -63,7 +63,7 @@ class TextBufferCellIterator const Microsoft::Console::Types::Viewport _bounds; bool _exceeded; COORD _pos; - std::optional _endPosInclusive; + std::optional _limit; #if UNIT_TESTING friend class TextBufferIteratorTests; diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 7e9e9c7d78c..932042a8b1f 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -592,6 +592,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation const int newDpi = static_cast(static_cast(USER_DEFAULT_SCREEN_DPI) * _compositionScale); + _terminal->SetFontInfo(_actualFont); + // TODO: MSFT:20895307 If the font doesn't exist, this doesn't // actually fail. We need a way to gracefully fallback. _renderer->TriggerFontChange(newDpi, _desiredFont, _actualFont); diff --git a/src/cascadia/TerminalControl/XamlUiaTextRange.cpp b/src/cascadia/TerminalControl/XamlUiaTextRange.cpp index 73b447d0a30..6ecb35c44e1 100644 --- a/src/cascadia/TerminalControl/XamlUiaTextRange.cpp +++ b/src/cascadia/TerminalControl/XamlUiaTextRange.cpp @@ -103,7 +103,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation } case VT_I4: { - return box_value(result.iVal); + // Surprisingly, `long` is _not_ a WinRT type. + // So we have to use `int32_t` to make sure this is output properly. + // Otherwise, you'll get "Attribute does not exist" out the other end. + return box_value(result.lVal); } case VT_R8: { @@ -122,17 +125,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation // are supported at this time. // So we need to figure out what was actually intended to be returned. - IUnknown* notSupportedVal; - UiaGetReservedNotSupportedValue(¬SupportedVal); - if (result.punkVal == notSupportedVal) - { - // See below for why we need to throw this special value. - winrt::throw_hresult(XAML_E_NOT_SUPPORTED); - } + // use C++11 magic statics to make sure we only do this once. + static const auto mixedAttributeVal = []() { + IUnknown* resultRaw; + com_ptr result; + UiaGetReservedMixedAttributeValue(&resultRaw); + result.attach(resultRaw); + return result; + }(); - IUnknown* mixedAttributeVal; - UiaGetReservedMixedAttributeValue(&mixedAttributeVal); - if (result.punkVal == mixedAttributeVal) + if (result.punkVal == mixedAttributeVal.get()) { return Windows::UI::Xaml::DependencyProperty::UnsetValue(); } diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index df9b1bc71ae..f4d173649d6 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -68,6 +68,7 @@ class Microsoft::Terminal::Core::Terminal final : void UpdateSettings(winrt::Microsoft::Terminal::Core::ICoreSettings settings); void UpdateAppearance(const winrt::Microsoft::Terminal::Core::ICoreAppearance& appearance); + void SetFontInfo(const FontInfo& fontInfo); // Write goes through the parser void Write(std::wstring_view stringView); @@ -160,6 +161,7 @@ class Microsoft::Terminal::Core::Terminal final : COORD GetTextBufferEndPosition() const noexcept override; const TextBuffer& GetTextBuffer() noexcept override; const FontInfo& GetFontInfo() noexcept override; + std::pair GetAttributeColors(const TextAttribute& attr) const noexcept override; void LockConsole() noexcept override; void UnlockConsole() noexcept override; @@ -168,7 +170,6 @@ class Microsoft::Terminal::Core::Terminal final : #pragma region IRenderData // These methods are defined in TerminalRenderData.cpp const TextAttribute GetDefaultBrushColors() noexcept override; - std::pair GetAttributeColors(const TextAttribute& attr) const noexcept override; COORD GetCursorPosition() const noexcept override; bool IsCursorVisible() const noexcept override; bool IsCursorOn() const noexcept override; @@ -276,6 +277,7 @@ class Microsoft::Terminal::Core::Terminal final : size_t _hyperlinkPatternId; std::wstring _workingDirectory; + std::optional _fontInfo; #pragma region Text Selection // a selection is represented as a range between two COORDs (start and end) // the pivot is the COORD that remains selected when you extend a selection in any direction diff --git a/src/cascadia/TerminalCore/terminalrenderdata.cpp b/src/cascadia/TerminalCore/terminalrenderdata.cpp index 17b711db748..47e2a28022f 100644 --- a/src/cascadia/TerminalCore/terminalrenderdata.cpp +++ b/src/cascadia/TerminalCore/terminalrenderdata.cpp @@ -34,6 +34,11 @@ const TextBuffer& Terminal::GetTextBuffer() noexcept #pragma warning(disable : 26447) const FontInfo& Terminal::GetFontInfo() noexcept { + if (_fontInfo) + { + return *_fontInfo; + } + // TODO: This font value is only used to check if the font is a raster font. // Otherwise, the font is changed with the renderer via TriggerFontChange. // The renderer never uses any of the other members from the value returned @@ -45,6 +50,11 @@ const FontInfo& Terminal::GetFontInfo() noexcept } #pragma warning(pop) +void Terminal::SetFontInfo(const FontInfo& fontInfo) +{ + _fontInfo = fontInfo; +} + const TextAttribute Terminal::GetDefaultBrushColors() noexcept { return TextAttribute{}; diff --git a/src/host/renderData.hpp b/src/host/renderData.hpp index 9e1e79b0b56..8b858c541e5 100644 --- a/src/host/renderData.hpp +++ b/src/host/renderData.hpp @@ -27,6 +27,7 @@ class RenderData final : COORD GetTextBufferEndPosition() const noexcept override; const TextBuffer& GetTextBuffer() noexcept override; const FontInfo& GetFontInfo() noexcept override; + std::pair GetAttributeColors(const TextAttribute& attr) const noexcept override; std::vector GetSelectionRects() noexcept override; @@ -37,8 +38,6 @@ class RenderData final : #pragma region IRenderData const TextAttribute GetDefaultBrushColors() noexcept override; - std::pair GetAttributeColors(const TextAttribute& attr) const noexcept override; - COORD GetCursorPosition() const noexcept override; bool IsCursorVisible() const noexcept override; bool IsCursorOn() const noexcept override; diff --git a/src/host/ut_host/TextBufferIteratorTests.cpp b/src/host/ut_host/TextBufferIteratorTests.cpp index 197deffa143..278ebaaf13d 100644 --- a/src/host/ut_host/TextBufferIteratorTests.cpp +++ b/src/host/ut_host/TextBufferIteratorTests.cpp @@ -264,8 +264,11 @@ class TextBufferIteratorTests TEST_METHOD(DereferenceOperatorText); TEST_METHOD(DereferenceOperatorCell); - TEST_METHOD(ConstructedNoLimit); - TEST_METHOD(ConstructedLimits); + TEST_METHOD(ConstructedNoBounds); + TEST_METHOD(ConstructedBounds); + TEST_METHOD(ConstructedLimit); + TEST_METHOD(ConstructedLimitBackwards); + TEST_METHOD(ConstructedBoundsAndLimit); }; template @@ -510,7 +513,7 @@ void TextBufferIteratorTests::DereferenceOperatorCell() VERIFY_ARE_EQUAL(attrExpected, attrActual); } -void TextBufferIteratorTests::ConstructedNoLimit() +void TextBufferIteratorTests::ConstructedNoBounds() { m_state->FillTextBuffer(); @@ -523,6 +526,7 @@ void TextBufferIteratorTests::ConstructedNoLimit() VERIFY_IS_TRUE(it, L"Iterator is valid."); VERIFY_ARE_EQUAL(bufferSize, it._bounds, L"Bounds match the bounds of the text buffer."); + VERIFY_IS_FALSE(it._limit.has_value(), L"Positional limit is not defined."); const auto totalBufferDistance = bufferSize.Width() * bufferSize.Height(); @@ -538,7 +542,7 @@ void TextBufferIteratorTests::ConstructedNoLimit() VERIFY_THROWS_SPECIFIC(TextBufferCellIterator(textBuffer, { -1, -1 }), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); } -void TextBufferIteratorTests::ConstructedLimits() +void TextBufferIteratorTests::ConstructedBounds() { m_state->FillTextBuffer(); @@ -546,21 +550,22 @@ void TextBufferIteratorTests::ConstructedLimits() const auto& outputBuffer = gci.GetActiveOutputBuffer(); const auto& textBuffer = outputBuffer.GetTextBuffer(); - SMALL_RECT limits; - limits.Top = 1; - limits.Bottom = 1; - limits.Left = 3; - limits.Right = 5; - const auto viewport = Microsoft::Console::Types::Viewport::FromInclusive(limits); + SMALL_RECT bounds; + bounds.Top = 1; + bounds.Bottom = 1; + bounds.Left = 3; + bounds.Right = 5; + const auto viewport = Microsoft::Console::Types::Viewport::FromInclusive(bounds); COORD pos; - pos.X = limits.Left; - pos.Y = limits.Top; + pos.X = bounds.Left; + pos.Y = bounds.Top; TextBufferCellIterator it(textBuffer, pos, viewport); VERIFY_IS_TRUE(it, L"Iterator is valid."); VERIFY_ARE_EQUAL(viewport, it._bounds, L"Bounds match the bounds given."); + VERIFY_IS_FALSE(it._limit.has_value(), L"Positional limit is not defined."); const auto totalBufferDistance = viewport.Width() * viewport.Height(); @@ -579,7 +584,7 @@ void TextBufferIteratorTests::ConstructedLimits() wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); - // Verify throws for limit not inside buffer + // Verify throws for bounds not inside buffer const auto bufferSize = textBuffer.GetSize(); VERIFY_THROWS_SPECIFIC(TextBufferCellIterator(textBuffer, pos, @@ -587,3 +592,110 @@ void TextBufferIteratorTests::ConstructedLimits() wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); } + +void TextBufferIteratorTests::ConstructedLimit() +{ + m_state->FillTextBuffer(); + + const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + const auto& outputBuffer = gci.GetActiveOutputBuffer(); + const auto& textBuffer = outputBuffer.GetTextBuffer(); + const auto bufferSize = textBuffer.GetSize(); + + const COORD pos{ bufferSize.Origin() }; + const COORD limitPos{ 5, 1 }; + + TextBufferCellIterator it(textBuffer, pos, bufferSize, limitPos); + + VERIFY_IS_TRUE(it, L"Iterator is valid."); + VERIFY_ARE_EQUAL(bufferSize, it._bounds, L"Bounds match the bounds given."); + VERIFY_IS_TRUE(it._limit.has_value(), L"Positional limit is defined."); + VERIFY_ARE_EQUAL(limitPos, it._limit.value(), L"Positional limit matches the one given."); + + const auto totalBufferDistance = bufferSize.Width() + 5; + + // Advance buffer to one before the positional limit. + it += totalBufferDistance; + VERIFY_IS_TRUE(it, L"Iterator is still valid."); + + // Advance over the positional limit. + it++; + VERIFY_IS_FALSE(it, L"Iterator invalid now."); + + // Verify throws for positional limit not inside buffer + VERIFY_THROWS_SPECIFIC(TextBufferCellIterator(textBuffer, + pos, + bufferSize, + bufferSize.BottomRightExclusive()), + wil::ResultException, + [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); +} + +void TextBufferIteratorTests::ConstructedLimitBackwards() +{ + m_state->FillTextBuffer(); + + const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + const auto& outputBuffer = gci.GetActiveOutputBuffer(); + const auto& textBuffer = outputBuffer.GetTextBuffer(); + const auto bufferSize = textBuffer.GetSize(); + + const COORD pos{ 5, 1 }; + const COORD limitPos{ 2, 0 }; + + TextBufferCellIterator it(textBuffer, pos, bufferSize, limitPos); + + VERIFY_IS_TRUE(it, L"Iterator is valid."); + VERIFY_ARE_EQUAL(bufferSize, it._bounds, L"Bounds match the bounds given."); + VERIFY_IS_TRUE(it._limit.has_value(), L"Positional limit is defined."); + VERIFY_ARE_EQUAL(limitPos, it._limit.value(), L"Positional limit matches the one given."); + + const auto totalBufferDistance = bufferSize.Width() + 5 - 2; + + // Advance buffer to one before the positional limit. + it -= totalBufferDistance; + VERIFY_IS_TRUE(it, L"Iterator is still valid."); + + // Advance over the positional limit. + it--; + VERIFY_IS_FALSE(it, L"Iterator invalid now."); +} + +void TextBufferIteratorTests::ConstructedBoundsAndLimit() +{ + m_state->FillTextBuffer(); + + const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + const auto& outputBuffer = gci.GetActiveOutputBuffer(); + const auto& textBuffer = outputBuffer.GetTextBuffer(); + + SMALL_RECT bounds; + bounds.Top = 1; + bounds.Bottom = 2; + bounds.Left = 2; + bounds.Right = 5; + const auto viewport = Microsoft::Console::Types::Viewport::FromInclusive(bounds); + + COORD pos; + pos.X = bounds.Left; + pos.Y = bounds.Top; + + const COORD limitPos{ 4, 1 }; + + TextBufferCellIterator it(textBuffer, pos, viewport, limitPos); + + VERIFY_IS_TRUE(it, L"Iterator is valid."); + VERIFY_ARE_EQUAL(viewport, it._bounds, L"Bounds match the bounds given."); + VERIFY_IS_TRUE(it._limit.has_value(), L"Positional limit is defined."); + VERIFY_ARE_EQUAL(limitPos, it._limit.value(), L"Positional limit matches the one given."); + + const auto totalBufferDistance = 2; + + // Advance buffer to one before the positional limit. + it += totalBufferDistance; + VERIFY_IS_TRUE(it, L"Iterator is still valid."); + + // Advance over the positional limit. + it++; + VERIFY_IS_FALSE(it, L"Iterator invalid now."); +} diff --git a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp index f5ed538ab0a..4b8a6d2bf10 100644 --- a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp +++ b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp @@ -133,20 +133,6 @@ class UiaTextRangeTests short yPos; }; - enum SupportedTextAttributes - { - FontName = UIA_FontNameAttributeId, // special handling - FontSize = UIA_FontSizeAttributeId, // special handling - FontWeight = UIA_FontWeightAttributeId, - ForegroundColor = UIA_ForegroundColorAttributeId, - Italic = UIA_IsItalicAttributeId, - IsReadOnly = UIA_IsReadOnlyAttributeId, // special handling (always false) - StrikethroughColor = UIA_StrikethroughColorAttributeId, - StrikethroughStyle = UIA_StrikethroughStyleAttributeId, - UnderlineColor = UIA_UnderlineColorAttributeId, - UnderlineStyle = UIA_UnderlineStyleAttributeId - }; - static constexpr wchar_t* toString(TextUnit unit) noexcept { // if a format is not supported, it goes to the next largest text unit @@ -1365,12 +1351,17 @@ class UiaTextRangeTests TEST_METHOD(GetAttributeValue) { Log::Comment(L"Check supported attributes"); - IUnknown* notSupportedVal; + Microsoft::WRL::ComPtr notSupportedVal; UiaGetReservedNotSupportedValue(¬SupportedVal); + + // Iterate over UIA's Text Attribute Identifiers + // Validate that we know which ones are (not) supported + // source: https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids for (long uiaAttributeId = 40000; uiaAttributeId <= 40042; ++uiaAttributeId) { Microsoft::WRL::ComPtr utr; THROW_IF_FAILED(Microsoft::WRL::MakeAndInitialize(&utr, _pUiaData, &_dummyProvider)); + THROW_IF_FAILED(utr->ExpandToEnclosingUnit(TextUnit_Character)); Log::Comment(UiaTracing::convertAttributeId(uiaAttributeId).c_str()); VARIANT result; @@ -1378,28 +1369,22 @@ class UiaTextRangeTests switch (uiaAttributeId) { - case FontName: + case UIA_FontNameAttributeId: { VERIFY_ARE_EQUAL(VT_BSTR, result.vt); break; } - case FontSize: - { - VERIFY_ARE_EQUAL(VT_R8, result.vt); - break; - } - case FontWeight: - case ForegroundColor: - case StrikethroughColor: - case StrikethroughStyle: - case UnderlineColor: - case UnderlineStyle: + case UIA_BackgroundColorAttributeId: + case UIA_FontWeightAttributeId: + case UIA_ForegroundColorAttributeId: + case UIA_StrikethroughStyleAttributeId: + case UIA_UnderlineStyleAttributeId: { VERIFY_ARE_EQUAL(VT_I4, result.vt); break; } - case Italic: - case IsReadOnly: + case UIA_IsItalicAttributeId: + case UIA_IsReadOnlyAttributeId: { VERIFY_ARE_EQUAL(VT_BOOL, result.vt); break; @@ -1408,7 +1393,7 @@ class UiaTextRangeTests { // Expected: not supported VERIFY_ARE_EQUAL(VT_UNKNOWN, result.vt); - VERIFY_ARE_EQUAL(notSupportedVal, result.punkVal); + VERIFY_ARE_EQUAL(notSupportedVal.Get(), result.punkVal); break; } } @@ -1424,110 +1409,109 @@ class UiaTextRangeTests Microsoft::WRL::ComPtr utr; THROW_IF_FAILED(Microsoft::WRL::MakeAndInitialize(&utr, _pUiaData, &_dummyProvider)); + THROW_IF_FAILED(utr->ExpandToEnclosingUnit(TextUnit_Character)); + { + Log::Comment(L"Test Background"); + const auto rawBackgroundColor{ RGB(255, 0, 0) }; + attr.SetBackground(rawBackgroundColor); + updateBuffer(attr); + VARIANT result; + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_BackgroundColorAttributeId, &result)); + + const COLORREF realBackgroundColor{ _pUiaData->GetAttributeColors(attr).second & 0x00ffffff }; + VERIFY_ARE_EQUAL(realBackgroundColor, static_cast(result.lVal)); + } { Log::Comment(L"Test Font Weight"); attr.SetBold(true); updateBuffer(attr); VARIANT result; - VERIFY_SUCCEEDED(utr->GetAttributeValue(FontWeight, &result)); - VERIFY_ARE_EQUAL(FW_BOLD, result.iVal); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_FontWeightAttributeId, &result)); + VERIFY_ARE_EQUAL(FW_BOLD, result.lVal); attr.SetBold(false); updateBuffer(attr); - VERIFY_SUCCEEDED(utr->GetAttributeValue(FontWeight, &result)); - VERIFY_ARE_EQUAL(FW_NORMAL, result.iVal); - - attr.SetFaint(true); - updateBuffer(attr); - VERIFY_SUCCEEDED(utr->GetAttributeValue(FontWeight, &result)); - VERIFY_ARE_EQUAL(FW_LIGHT, result.iVal); - - attr.SetFaint(false); - updateBuffer(attr); - VERIFY_SUCCEEDED(utr->GetAttributeValue(FontWeight, &result)); - VERIFY_ARE_EQUAL(FW_NORMAL, result.iVal); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_FontWeightAttributeId, &result)); + VERIFY_ARE_EQUAL(FW_NORMAL, result.lVal); + ; } { Log::Comment(L"Test Foreground"); - const auto foregroundColor{ RGB(255, 0, 0) }; - attr.SetForeground(foregroundColor); + const auto rawForegroundColor{ RGB(255, 0, 0) }; + attr.SetForeground(rawForegroundColor); updateBuffer(attr); VARIANT result; - VERIFY_SUCCEEDED(utr->GetAttributeValue(ForegroundColor, &result)); - VERIFY_ARE_EQUAL(foregroundColor, static_cast(result.iVal)); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_ForegroundColorAttributeId, &result)); + + const auto realForegroundColor{ _pUiaData->GetAttributeColors(attr).first & 0x00ffffff }; + VERIFY_ARE_EQUAL(realForegroundColor, static_cast(result.lVal)); } { Log::Comment(L"Test Italic"); attr.SetItalic(true); updateBuffer(attr); VARIANT result; - VERIFY_SUCCEEDED(utr->GetAttributeValue(Italic, &result)); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_IsItalicAttributeId, &result)); VERIFY_IS_TRUE(result.boolVal); attr.SetItalic(false); updateBuffer(attr); - VERIFY_SUCCEEDED(utr->GetAttributeValue(Italic, &result)); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_IsItalicAttributeId, &result)); VERIFY_IS_FALSE(result.boolVal); } { Log::Comment(L"Test Strikethrough"); - const auto foregroundColor{ attr.GetForeground().GetRGB() }; attr.SetCrossedOut(true); updateBuffer(attr); VARIANT result; - VERIFY_SUCCEEDED(utr->GetAttributeValue(StrikethroughColor, &result)); - VERIFY_ARE_EQUAL(foregroundColor, static_cast(result.iVal)); - VERIFY_SUCCEEDED(utr->GetAttributeValue(StrikethroughStyle, &result)); - VERIFY_ARE_EQUAL(TextDecorationLineStyle_Single, result.iVal); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_StrikethroughStyleAttributeId, &result)); + VERIFY_ARE_EQUAL(TextDecorationLineStyle_Single, result.lVal); attr.SetCrossedOut(false); updateBuffer(attr); - VERIFY_SUCCEEDED(utr->GetAttributeValue(StrikethroughColor, &result)); - VERIFY_ARE_EQUAL(0, result.iVal); - VERIFY_SUCCEEDED(utr->GetAttributeValue(StrikethroughStyle, &result)); - VERIFY_ARE_EQUAL(TextDecorationLineStyle_None, result.iVal); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_StrikethroughStyleAttributeId, &result)); + VERIFY_ARE_EQUAL(TextDecorationLineStyle_None, result.lVal); } { Log::Comment(L"Test Underline"); - const auto foregroundColor{ attr.GetForeground().GetRGB() }; + + // Single underline attr.SetUnderlined(true); updateBuffer(attr); VARIANT result; - VERIFY_SUCCEEDED(utr->GetAttributeValue(UnderlineColor, &result)); - VERIFY_ARE_EQUAL(foregroundColor, static_cast(result.iVal)); - VERIFY_SUCCEEDED(utr->GetAttributeValue(UnderlineStyle, &result)); - VERIFY_ARE_EQUAL(TextDecorationLineStyle_Single, result.iVal); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_UnderlineStyleAttributeId, &result)); + VERIFY_ARE_EQUAL(TextDecorationLineStyle_Single, result.lVal); + // Double underline + attr.SetDoublyUnderlined(true); + updateBuffer(attr); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_UnderlineStyleAttributeId, &result)); + VERIFY_ARE_EQUAL(TextDecorationLineStyle_Double, result.lVal); + + // Double underline attr.SetUnderlined(false); updateBuffer(attr); - VERIFY_SUCCEEDED(utr->GetAttributeValue(UnderlineColor, &result)); - VERIFY_ARE_EQUAL(0, result.iVal); - VERIFY_SUCCEEDED(utr->GetAttributeValue(UnderlineStyle, &result)); - VERIFY_ARE_EQUAL(TextDecorationLineStyle_None, result.iVal); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_UnderlineStyleAttributeId, &result)); + VERIFY_ARE_EQUAL(TextDecorationLineStyle_Double, result.lVal); + + // No underline + attr.SetDoublyUnderlined(false); + updateBuffer(attr); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_UnderlineStyleAttributeId, &result)); + VERIFY_ARE_EQUAL(TextDecorationLineStyle_None, result.lVal); } { Log::Comment(L"Test Font Name (special)"); VARIANT result; - VERIFY_SUCCEEDED(utr->GetAttributeValue(FontName, &result)); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_FontNameAttributeId, &result)); const std::wstring actualFontName{ result.bstrVal }; const auto expectedFontName{ _pUiaData->GetFontInfo().GetFaceName() }; VERIFY_ARE_EQUAL(expectedFontName, actualFontName); } - { - Log::Comment(L"Test Font Size (special)"); - VARIANT result; - VERIFY_SUCCEEDED(utr->GetAttributeValue(FontSize, &result)); - const auto actualFontSize{ result.dblVal }; - const auto expectedFontSize{ 0 }; //_pUiaData->GetFontInfo().GetUnscaledSize() }; - VERIFY_ARE_EQUAL(expectedFontSize, actualFontSize); - - // TODO CARLOS: Fix this test when we actually know how to convert the font size into points - VERIFY_IS_TRUE(false); - } { Log::Comment(L"Test Read Only (special)"); VARIANT result; - VERIFY_SUCCEEDED(utr->GetAttributeValue(IsReadOnly, &result)); + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_IsReadOnlyAttributeId, &result)); VERIFY_IS_FALSE(result.boolVal); } } @@ -1548,7 +1532,7 @@ class UiaTextRangeTests var.bstrVal = SysAllocString(fontName.data()); Microsoft::WRL::ComPtr result; - VERIFY_SUCCEEDED(utr->FindAttribute(FontName, var, false, result.GetAddressOf())); + VERIFY_SUCCEEDED(utr->FindAttribute(UIA_FontNameAttributeId, var, false, result.GetAddressOf())); // Expecting the same text range endpoints BOOL isEqual; @@ -1558,41 +1542,12 @@ class UiaTextRangeTests // Now perform the same test, but searching backwards Log::Comment(L"Test Font Name (special) - Backwards"); Microsoft::WRL::ComPtr resultBackwards; - VERIFY_SUCCEEDED(utr->FindAttribute(FontName, var, true, resultBackwards.GetAddressOf())); + VERIFY_SUCCEEDED(utr->FindAttribute(UIA_FontNameAttributeId, var, true, resultBackwards.GetAddressOf())); // Expecting the same text range endpoints THROW_IF_FAILED(result->Compare(resultBackwards.Get(), &isEqual)); VERIFY_IS_TRUE(isEqual); } - { - Log::Comment(L"Test Font Size (special)"); - - // Populate query with font size currently in use. - const auto fontSize{ 0.0 }; //{ _pUiaData->GetFontInfo().GetUnscaledSize() }; - VARIANT var{}; - var.vt = VT_R8; - var.dblVal = 0; - - Microsoft::WRL::ComPtr result; - VERIFY_SUCCEEDED(utr->FindAttribute(FontSize, var, false, result.GetAddressOf())); - - // Expecting the same text range endpoints - BOOL isEqual; - THROW_IF_FAILED(utr->Compare(result.Get(), &isEqual)); - VERIFY_IS_TRUE(isEqual); - - // Now perform the same test, but searching backwards - Log::Comment(L"Test Font Size (special) - Backwards"); - Microsoft::WRL::ComPtr resultBackwards; - VERIFY_SUCCEEDED(utr->FindAttribute(FontSize, var, true, resultBackwards.GetAddressOf())); - - // Expecting the same text range endpoints - THROW_IF_FAILED(result->Compare(resultBackwards.Get(), &isEqual)); - VERIFY_IS_TRUE(isEqual); - - // TODO CARLOS: Fix this test when we actually know how to convert the font size into points - VERIFY_IS_TRUE(false); - } { Log::Comment(L"Test Read Only (special)"); @@ -1601,7 +1556,7 @@ class UiaTextRangeTests var.boolVal = false; Microsoft::WRL::ComPtr result; - VERIFY_SUCCEEDED(utr->FindAttribute(IsReadOnly, var, false, result.GetAddressOf())); + VERIFY_SUCCEEDED(utr->FindAttribute(UIA_IsReadOnlyAttributeId, var, false, result.GetAddressOf())); // Expecting the same text range endpoints BOOL isEqual; @@ -1611,7 +1566,7 @@ class UiaTextRangeTests // Now perform the same test, but searching backwards Log::Comment(L"Test Read Only (special) - Backwards"); Microsoft::WRL::ComPtr resultBackwards; - VERIFY_SUCCEEDED(utr->FindAttribute(IsReadOnly, var, true, resultBackwards.GetAddressOf())); + VERIFY_SUCCEEDED(utr->FindAttribute(UIA_IsReadOnlyAttributeId, var, true, resultBackwards.GetAddressOf())); // Expecting the same text range endpoints THROW_IF_FAILED(result->Compare(resultBackwards.Get(), &isEqual)); @@ -1633,35 +1588,29 @@ class UiaTextRangeTests auto iter{ _pUiaData->GetTextBuffer().GetCellDataAt(startPos) }; for (auto i = 0; i < 5; ++i) { - _pTextBuffer->Write({ italicAttr }, iter.Pos()); + _pTextBuffer->Write({ L"X", italicAttr }, iter.Pos()); ++iter; } // set the expected end (exclusive) - auto expectedEndPos{ iter.Pos() }; - _pUiaData->GetTextBuffer().GetSize().IncrementInBounds(expectedEndPos); + const auto expectedEndPos{ iter.Pos() }; VARIANT var{}; var.vt = VT_BOOL; var.boolVal = true; Microsoft::WRL::ComPtr result; - VERIFY_SUCCEEDED(utr->FindAttribute(Italic, var, false, result.GetAddressOf())); - - // TODO CARLOS: The "end" verification is failing. Here's a trace of what's going on: - // - UiaTextRangeBase: - // - checkIfAttrFound is properly set and used - // - the search loop properly sets the "start" - // - the search loop iterates through the entire utr because checkIfAttrFound - // continues to return true for the whole range - // I have no clue why this is happening. A fresh pair of eyes would be appreciated here :) - VERIFY_ARE_EQUAL(startPos, utr->_start); - VERIFY_ARE_EQUAL(expectedEndPos, utr->_end); + THROW_IF_FAILED(utr->ExpandToEnclosingUnit(TextUnit_Document)); + VERIFY_SUCCEEDED(utr->FindAttribute(UIA_IsItalicAttributeId, var, false, result.GetAddressOf())); + + Microsoft::WRL::ComPtr resultUtr{ static_cast(result.Get()) }; + VERIFY_ARE_EQUAL(startPos, resultUtr->_start); + VERIFY_ARE_EQUAL(expectedEndPos, resultUtr->_end); // Now perform the same test, but searching backwards Log::Comment(L"Test IsItalic (standard attribute) - Backwards"); Microsoft::WRL::ComPtr resultBackwards; - VERIFY_SUCCEEDED(utr->FindAttribute(Italic, var, true, resultBackwards.GetAddressOf())); + VERIFY_SUCCEEDED(utr->FindAttribute(UIA_IsItalicAttributeId, var, true, resultBackwards.GetAddressOf())); // Expecting the same text range endpoints BOOL isEqual; diff --git a/src/renderer/inc/IRenderData.hpp b/src/renderer/inc/IRenderData.hpp index 56ab872e217..a8c96be268a 100644 --- a/src/renderer/inc/IRenderData.hpp +++ b/src/renderer/inc/IRenderData.hpp @@ -48,8 +48,6 @@ namespace Microsoft::Console::Render virtual const TextAttribute GetDefaultBrushColors() noexcept = 0; - virtual std::pair GetAttributeColors(const TextAttribute& attr) const noexcept = 0; - virtual COORD GetCursorPosition() const noexcept = 0; virtual bool IsCursorVisible() const noexcept = 0; virtual bool IsCursorOn() const noexcept = 0; diff --git a/src/types/IBaseData.h b/src/types/IBaseData.h index 58804c38269..6b9f8b722f7 100644 --- a/src/types/IBaseData.h +++ b/src/types/IBaseData.h @@ -37,6 +37,7 @@ namespace Microsoft::Console::Types virtual COORD GetTextBufferEndPosition() const noexcept = 0; virtual const TextBuffer& GetTextBuffer() noexcept = 0; virtual const FontInfo& GetFontInfo() noexcept = 0; + virtual std::pair GetAttributeColors(const TextAttribute& attr) const noexcept = 0; virtual std::vector GetSelectionRects() noexcept = 0; diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index c77cbef2630..4a7e4b23883 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -324,6 +324,17 @@ std::function UiaTextRangeBase::_getAttrVerification // has the desired attribute. switch (attributeId) { + case UIA_BackgroundColorAttributeId: + { + // Expected type: VT_I4 + THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); + + // The foreground color is stored as a COLORREF. + const auto queryBackgroundColor{ base::ClampedNumeric(val.lVal) }; + return [this, queryBackgroundColor](const TextAttribute& attr) { + return (_pData->GetAttributeColors(attr).second & 0x00ffffff) == queryBackgroundColor; + }; + } case UIA_FontWeightAttributeId: { // Expected type: VT_I4 @@ -332,7 +343,7 @@ std::function UiaTextRangeBase::_getAttrVerification // The font weight can be any value from 0 to 900. // The text buffer doesn't store the actual value, // we just store "IsBold" and "IsFaint". - const auto queryFontWeight{ val.iVal }; + const auto queryFontWeight{ val.lVal }; if (queryFontWeight > FW_NORMAL) { @@ -341,18 +352,11 @@ std::function UiaTextRangeBase::_getAttrVerification return attr.IsBold(); }; } - else if (queryFontWeight < FW_NORMAL) - { - // we're looking for a faint font weight - return [](const TextAttribute& attr) { - return attr.IsFaint(); - }; - } else { // we're looking for "normal" font weight return [](const TextAttribute& attr) { - return !attr.IsBold() && !attr.IsFaint(); + return !attr.IsBold(); }; } } @@ -362,9 +366,9 @@ std::function UiaTextRangeBase::_getAttrVerification THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); // The foreground color is stored as a COLORREF. - const COLORREF queryForegroundColor{ base::ClampedNumeric(val.iVal) }; - return [queryForegroundColor](const TextAttribute& attr) { - return attr.GetForeground().GetRGB() == queryForegroundColor; + const auto queryForegroundColor{ base::ClampedNumeric(val.lVal) }; + return [this, queryForegroundColor](const TextAttribute& attr) { + return (_pData->GetAttributeColors(attr).first & 0x00ffffff) == queryForegroundColor; }; } case UIA_IsItalicAttributeId: @@ -387,30 +391,6 @@ std::function UiaTextRangeBase::_getAttrVerification }; } } - case UIA_StrikethroughColorAttributeId: - { - // Expected type: VT_I4 - THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); - - // The strikethrough color is stored as a COLORREF. - // However, the text buffer doesn't actually store the color. - // Instead, we just store whether or not the text is crossed out. - // So we'll assume that _any_ query here (except 0) is interpreted as "is the - // text crossed out?" - - if (val.iVal == 0) - { - return [](const TextAttribute& attr) { - return !attr.IsCrossedOut(); - }; - } - else - { - return [](const TextAttribute& attr) { - return attr.IsCrossedOut(); - }; - } - } case UIA_StrikethroughStyleAttributeId: { // Expected type: VT_I4 @@ -419,44 +399,18 @@ std::function UiaTextRangeBase::_getAttrVerification // The strikethrough style is stored as a TextDecorationLineStyle. // However, The text buffer doesn't have different styles for being crossed out. // Instead, we just store whether or not the text is crossed out. - // So we'll assume that _any_ query here is interpreted as "is the - // text crossed out?" UNLESS we are asked for TextDecorationLineStyle_None - const TextDecorationLineStyle queryStrikethroughStyle{ static_cast(val.iVal) }; - if (queryStrikethroughStyle == TextDecorationLineStyle_None) + switch (val.lVal) { + case TextDecorationLineStyle_None: return [](const TextAttribute& attr) { return !attr.IsCrossedOut(); }; - } - else - { + case TextDecorationLineStyle_Single: return [](const TextAttribute& attr) { return attr.IsCrossedOut(); }; - } - } - case UIA_UnderlineColorAttributeId: - { - // Expected type: VT_I4 - THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); - - // The underline color is stored as a COLORREF. - // However, the text buffer doesn't actually store the color. - // Instead, we just store whether or not the text is underlined. - // So we'll assume that _any_ query here (except 0) is interpreted as "is the - // text underlined?" - - if (val.iVal == 0) - { - return [](const TextAttribute& attr) { - return !attr.IsUnderlined(); - }; - } - else - { - return [](const TextAttribute& attr) { - return attr.IsUnderlined(); - }; + default: + return nullptr; } } case UIA_UnderlineStyleAttributeId: @@ -465,22 +419,24 @@ std::function UiaTextRangeBase::_getAttrVerification THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); // The underline style is stored as a TextDecorationLineStyle. - // However, The text buffer doesn't have different styles for being underlined. - // Instead, we just store whether or not the text is underlined. - // So we'll assume that _any_ query here is interpreted as "is the - // text underlined?" UNLESS we are asked for TextDecorationLineStyle_None - const TextDecorationLineStyle queryUnderlineStyle{ static_cast(val.iVal) }; - if (queryUnderlineStyle == TextDecorationLineStyle_None) + // However, The text buffer doesn't have that many different styles for being underlined. + // Instead, we only have single and double underlined. + switch (val.lVal) { + case TextDecorationLineStyle_None: return [](const TextAttribute& attr) { - return !attr.IsUnderlined(); + return !attr.IsUnderlined() && !attr.IsDoublyUnderlined(); }; - } - else - { + case TextDecorationLineStyle_Double: + return [](const TextAttribute& attr) { + return attr.IsDoublyUnderlined(); + }; + case TextDecorationLineStyle_Single: return [](const TextAttribute& attr) { return attr.IsUnderlined(); }; + default: + return nullptr; } } default: @@ -510,19 +466,6 @@ IFACEMETHODIMP UiaTextRangeBase::FindAttribute(_In_ TEXTATTRIBUTEID attributeId, UiaTracing::TextRange::FindAttribute(*this, attributeId, val, searchBackwards, static_cast(**ppRetVal)); return S_OK; } - case UIA_FontSizeAttributeId: - { - RETURN_HR_IF(E_INVALIDARG, val.vt != VT_R8); - const auto queryFontSize{ val.dblVal }; - - // TODO CARLOS: how do I get the font size in points? - if (queryFontSize == 0) //_pData->GetFontInfo().GetUnscaledSize()) - { - Clone(ppRetVal); - } - UiaTracing::TextRange::FindAttribute(*this, attributeId, val, searchBackwards, static_cast(**ppRetVal)); - return S_OK; - } case UIA_IsReadOnlyAttributeId: { RETURN_HR_IF(E_INVALIDARG, val.vt != VT_BOOL); @@ -693,6 +636,12 @@ std::function UiaTextRangeBase::_getAttrVerification const auto attr{ _pData->GetTextBuffer().GetCellDataAt(_start)->TextAttr() }; switch (attributeId) { + case UIA_BackgroundColorAttributeId: + { + pRetVal->vt = VT_I4; + pRetVal->lVal = base::ClampedNumeric(_pData->GetAttributeColors(attr).second & 0x00ffffff); + return _getAttrVerificationFn(attributeId, *pRetVal); + } case UIA_FontWeightAttributeId: { // The font weight can be any value from 0 to 900. @@ -702,56 +651,50 @@ std::function UiaTextRangeBase::_getAttrVerification pRetVal->vt = VT_I4; if (attr.IsBold()) { - pRetVal->iVal = FW_BOLD; - } - else if (attr.IsFaint()) - { - pRetVal->iVal = FW_LIGHT; + pRetVal->lVal = FW_BOLD; } else { - pRetVal->iVal = FW_NORMAL; + pRetVal->lVal = FW_NORMAL; } return _getAttrVerificationFn(attributeId, *pRetVal); } case UIA_ForegroundColorAttributeId: { pRetVal->vt = VT_I4; - pRetVal->iVal = base::ClampedNumeric(attr.GetForeground().GetRGB()); + const auto x{ _pData->GetAttributeColors(attr).first }; + const auto y{ x & 0x00ffffff }; + pRetVal->lVal = y; + //pRetVal->lVal = base::ClampedNumeric(_pData->GetAttributeColors(attr).first & 0x00ffffff); return _getAttrVerificationFn(attributeId, *pRetVal); } case UIA_IsItalicAttributeId: { pRetVal->vt = VT_BOOL; - pRetVal->iVal = attr.IsItalic(); - return _getAttrVerificationFn(attributeId, *pRetVal); - } - case UIA_StrikethroughColorAttributeId: - { - // Default Value: 0 - // Source: https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids - pRetVal->vt = VT_I4; - pRetVal->iVal = base::ClampedNumeric(attr.IsCrossedOut() ? attr.GetForeground().GetRGB() : 0); + pRetVal->lVal = attr.IsItalic(); return _getAttrVerificationFn(attributeId, *pRetVal); } case UIA_StrikethroughStyleAttributeId: { pRetVal->vt = VT_I4; - pRetVal->iVal = static_cast(attr.IsCrossedOut() ? TextDecorationLineStyle_Single : TextDecorationLineStyle_None); - return _getAttrVerificationFn(attributeId, *pRetVal); - } - case UIA_UnderlineColorAttributeId: - { - // Default Value: 0 - // Source: https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids - pRetVal->vt = VT_I4; - pRetVal->iVal = attr.IsUnderlined() ? base::ClampedNumeric(attr.GetForeground().GetRGB()) : 0; + pRetVal->lVal = static_cast(attr.IsCrossedOut() ? TextDecorationLineStyle_Single : TextDecorationLineStyle_None); return _getAttrVerificationFn(attributeId, *pRetVal); } case UIA_UnderlineStyleAttributeId: { pRetVal->vt = VT_I4; - pRetVal->iVal = static_cast(attr.IsUnderlined() ? TextDecorationLineStyle_Single : TextDecorationLineStyle_None); + if (attr.IsDoublyUnderlined()) + { + pRetVal->lVal = static_cast(TextDecorationLineStyle_Double); + } + else if (attr.IsUnderlined()) + { + pRetVal->lVal = static_cast(TextDecorationLineStyle_Single); + } + else + { + pRetVal->lVal = static_cast(TextDecorationLineStyle_None); + } return _getAttrVerificationFn(attributeId, *pRetVal); } default: @@ -766,6 +709,7 @@ IFACEMETHODIMP UiaTextRangeBase::GetAttributeValue(_In_ TEXTATTRIBUTEID attribut _Out_ VARIANT* pRetVal) noexcept { RETURN_HR_IF(E_INVALIDARG, pRetVal == nullptr); + VariantInit(pRetVal); // AttributeIDs that require special handling switch (attributeId) @@ -777,14 +721,6 @@ IFACEMETHODIMP UiaTextRangeBase::GetAttributeValue(_In_ TEXTATTRIBUTEID attribut UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal); return S_OK; } - case UIA_FontSizeAttributeId: - { - pRetVal->vt = VT_R8; - // TODO CARLOS: how do I get the font size in points? - pRetVal->dblVal = 0; //_pData->GetFontInfo().GetUnscaledSize()) - UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal); - return S_OK; - } case UIA_IsReadOnlyAttributeId: { pRetVal->vt = VT_BOOL; @@ -816,6 +752,17 @@ IFACEMETHODIMP UiaTextRangeBase::GetAttributeValue(_In_ TEXTATTRIBUTEID attribut return E_INVALIDARG; } + if (IsDegenerate()) + { + // Unlike a normal text editor, which applies formatting at the caret, + // we don't know what attributes are written at a degenerate range. + // So let's return UiaGetReservedMixedAttributeValue. + // Source: https://docs.microsoft.com/en-us/windows/win32/api/uiautomationcore/nf-uiautomationcore-itextrangeprovider-getattributevalue + pRetVal->vt = VT_UNKNOWN; + UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal, UiaTracing::AttributeType::Mixed); + return UiaGetReservedMixedAttributeValue(&pRetVal->punkVal); + } + // Get some useful variables const auto& buffer{ _pData->GetTextBuffer() }; const auto bufferSize{ buffer.GetSize() }; From a7bbf9d3de0b2f5133dc0e9b8421e31639a0ae14 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 29 Jun 2021 15:12:15 -0700 Subject: [PATCH 3/9] address PR feedback --- src/buffer/out/textBufferCellIterator.cpp | 5 +- .../TerminalControl/XamlUiaTextRange.cpp | 12 +-- src/cascadia/TerminalCore/Terminal.cpp | 1 - src/cascadia/TerminalCore/Terminal.hpp | 6 +- .../TerminalCore/terminalrenderdata.cpp | 20 +--- .../UiaTextRangeTests.cpp | 8 +- src/renderer/base/FontInfoBase.cpp | 2 +- src/renderer/inc/FontInfoBase.hpp | 2 +- src/types/UiaTextRangeBase.cpp | 93 +++++++++------- src/types/UiaTextRangeBase.hpp | 2 +- src/types/UiaTracing.cpp | 102 +----------------- src/types/UiaTracing.h | 2 - 12 files changed, 77 insertions(+), 178 deletions(-) diff --git a/src/buffer/out/textBufferCellIterator.cpp b/src/buffer/out/textBufferCellIterator.cpp index ba6b04ebc0b..318e999ffc8 100644 --- a/src/buffer/out/textBufferCellIterator.cpp +++ b/src/buffer/out/textBufferCellIterator.cpp @@ -35,6 +35,7 @@ TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD p _pos(pos), _pRow(s_GetRow(buffer, pos)), _bounds(bounds), + _limit(std::nullopt), _exceeded(false), _view({}, {}, {}, TextAttributeBehavior::Stored), _attrIter(s_GetRow(buffer, pos)->GetAttrRow().cbegin()) @@ -118,11 +119,11 @@ TextBufferCellIterator& TextBufferCellIterator::operator+=(const ptrdiff_t& move // If we have a limit, check if we've exceeded it if (_limit.has_value()) { - _exceeded |= (newPos == _limit); + _exceeded |= (newPos == *_limit); } // If we already exceeded limit, we'll short-circuit and _not_ increment - _exceeded |= !_bounds.IncrementInBounds(newPos); + _exceeded = _exceeded || !_bounds.IncrementInBounds(newPos); move--; } while (move < 0 && !_exceeded) diff --git a/src/cascadia/TerminalControl/XamlUiaTextRange.cpp b/src/cascadia/TerminalControl/XamlUiaTextRange.cpp index 6ecb35c44e1..eb40018e22c 100644 --- a/src/cascadia/TerminalControl/XamlUiaTextRange.cpp +++ b/src/cascadia/TerminalControl/XamlUiaTextRange.cpp @@ -125,21 +125,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation // are supported at this time. // So we need to figure out what was actually intended to be returned. - // use C++11 magic statics to make sure we only do this once. - static const auto mixedAttributeVal = []() { - IUnknown* resultRaw; - com_ptr result; - UiaGetReservedMixedAttributeValue(&resultRaw); - result.attach(resultRaw); - return result; - }(); + com_ptr mixedAttributeVal; + UiaGetReservedMixedAttributeValue(mixedAttributeVal.put()); if (result.punkVal == mixedAttributeVal.get()) { return Windows::UI::Xaml::DependencyProperty::UnsetValue(); } - __fallthrough; + [[fallthrough]]; } default: { diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 9d1ae957009..d0dfbe604ab 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -6,7 +6,6 @@ #include "../../terminal/parser/OutputStateMachineEngine.hpp" #include "TerminalDispatch.hpp" #include "../../inc/unicode.hpp" -#include "../../inc/DefaultSettings.h" #include "../../inc/argb.h" #include "../../types/inc/utils.hpp" #include "../../types/inc/colorTable.hpp" diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index f4d173649d6..65fd5f031a3 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -5,6 +5,7 @@ #include +#include "../../inc/DefaultSettings.h" #include "../../buffer/out/textBuffer.hpp" #include "../../types/inc/sgrStack.hpp" #include "../../renderer/inc/BlinkingState.hpp" @@ -277,7 +278,10 @@ class Microsoft::Terminal::Core::Terminal final : size_t _hyperlinkPatternId; std::wstring _workingDirectory; - std::optional _fontInfo; + + // This font value is only used to check if the font is a raster font. + // Otherwise, the font is changed with the renderer via TriggerFontChange. + FontInfo _fontInfo{ DEFAULT_FONT_FACE, TMPF_TRUETYPE, 10, { 0, DEFAULT_FONT_SIZE }, CP_UTF8, false }; #pragma region Text Selection // a selection is represented as a range between two COORDs (start and end) // the pivot is the COORD that remains selected when you extend a selection in any direction diff --git a/src/cascadia/TerminalCore/terminalrenderdata.cpp b/src/cascadia/TerminalCore/terminalrenderdata.cpp index 47e2a28022f..b2ed30f1566 100644 --- a/src/cascadia/TerminalCore/terminalrenderdata.cpp +++ b/src/cascadia/TerminalCore/terminalrenderdata.cpp @@ -27,28 +27,10 @@ const TextBuffer& Terminal::GetTextBuffer() noexcept return *_buffer; } -// Creating a FontInfo can technically throw (on string allocation) and this is noexcept. -// That means this will std::terminate. We could come back and make there be a default constructor -// backup to FontInfo that throws no exceptions and allocates a default FontInfo structure. -#pragma warning(push) -#pragma warning(disable : 26447) const FontInfo& Terminal::GetFontInfo() noexcept { - if (_fontInfo) - { - return *_fontInfo; - } - - // TODO: This font value is only used to check if the font is a raster font. - // Otherwise, the font is changed with the renderer via TriggerFontChange. - // The renderer never uses any of the other members from the value returned - // by this method. - // We could very likely replace this with just an IsRasterFont method - // (which would return false) - static const FontInfo _fakeFontInfo(DEFAULT_FONT_FACE, TMPF_TRUETYPE, 10, { 0, DEFAULT_FONT_SIZE }, CP_UTF8, false); - return _fakeFontInfo; + return _fontInfo; } -#pragma warning(pop) void Terminal::SetFontInfo(const FontInfo& fontInfo) { diff --git a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp index 4b8a6d2bf10..c1dba0be436 100644 --- a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp +++ b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp @@ -1357,13 +1357,13 @@ class UiaTextRangeTests // Iterate over UIA's Text Attribute Identifiers // Validate that we know which ones are (not) supported // source: https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids - for (long uiaAttributeId = 40000; uiaAttributeId <= 40042; ++uiaAttributeId) + for (long uiaAttributeId = UIA_AnimationStyleAttributeId; uiaAttributeId <= UIA_AfterParagraphSpacingAttributeId; ++uiaAttributeId) { Microsoft::WRL::ComPtr utr; THROW_IF_FAILED(Microsoft::WRL::MakeAndInitialize(&utr, _pUiaData, &_dummyProvider)); THROW_IF_FAILED(utr->ExpandToEnclosingUnit(TextUnit_Character)); - Log::Comment(UiaTracing::convertAttributeId(uiaAttributeId).c_str()); + Log::Comment(NoThrowString().Format(L"Attribute ID: %d", uiaAttributeId)); VARIANT result; VERIFY_SUCCEEDED(utr->GetAttributeValue(uiaAttributeId, &result)); @@ -1482,13 +1482,13 @@ class UiaTextRangeTests VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_UnderlineStyleAttributeId, &result)); VERIFY_ARE_EQUAL(TextDecorationLineStyle_Single, result.lVal); - // Double underline + // Double underline (double supercedes single) attr.SetDoublyUnderlined(true); updateBuffer(attr); VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_UnderlineStyleAttributeId, &result)); VERIFY_ARE_EQUAL(TextDecorationLineStyle_Double, result.lVal); - // Double underline + // Double underline (double on its own) attr.SetUnderlined(false); updateBuffer(attr); VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_UnderlineStyleAttributeId, &result)); diff --git a/src/renderer/base/FontInfoBase.cpp b/src/renderer/base/FontInfoBase.cpp index 8de26d8baf3..40891cf4940 100644 --- a/src/renderer/base/FontInfoBase.cpp +++ b/src/renderer/base/FontInfoBase.cpp @@ -61,7 +61,7 @@ unsigned int FontInfoBase::GetWeight() const return _weight; } -const std::wstring_view FontInfoBase::GetFaceName() const +const std::wstring_view FontInfoBase::GetFaceName() const noexcept { return _faceName; } diff --git a/src/renderer/inc/FontInfoBase.hpp b/src/renderer/inc/FontInfoBase.hpp index 8e3f32d8210..aa9f3cddbb6 100644 --- a/src/renderer/inc/FontInfoBase.hpp +++ b/src/renderer/inc/FontInfoBase.hpp @@ -38,7 +38,7 @@ class FontInfoBase unsigned char GetFamily() const; unsigned int GetWeight() const; - const std::wstring_view GetFaceName() const; + const std::wstring_view GetFaceName() const noexcept; unsigned int GetCodePage() const; HRESULT FillLegacyNameBuffer(gsl::span buffer) const; diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index 4a7e4b23883..55a93b5576e 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -9,6 +9,12 @@ using namespace Microsoft::Console::Types; +// Foreground/Background text color doesn't care about the alpha. +static long _RemoveAlpha(COLORREF color) +{ + return color & 0x00ffffff; +} + // degenerate range constructor. #pragma warning(suppress : 26434) // WRL RuntimeClassInitialize base is a no-op and we need this for MakeAndInitialize HRESULT UiaTextRangeBase::RuntimeClassInitialize(_In_ IUiaData* pData, _In_ IRawElementProviderSimple* const pProvider, _In_ std::wstring_view wordDelimiters) noexcept @@ -330,9 +336,9 @@ std::function UiaTextRangeBase::_getAttrVerification THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); // The foreground color is stored as a COLORREF. - const auto queryBackgroundColor{ base::ClampedNumeric(val.lVal) }; - return [this, queryBackgroundColor](const TextAttribute& attr) { - return (_pData->GetAttributeColors(attr).second & 0x00ffffff) == queryBackgroundColor; + const auto queryBackgroundColor{ val.lVal }; + return [this, queryBackgroundColor](const TextAttribute& attr) noexcept { + return _RemoveAlpha(_pData->GetAttributeColors(attr).second) == queryBackgroundColor; }; } case UIA_FontWeightAttributeId: @@ -348,14 +354,14 @@ std::function UiaTextRangeBase::_getAttrVerification if (queryFontWeight > FW_NORMAL) { // we're looking for a bold font weight - return [](const TextAttribute& attr) { + return [](const TextAttribute& attr) noexcept { return attr.IsBold(); }; } else { // we're looking for "normal" font weight - return [](const TextAttribute& attr) { + return [](const TextAttribute& attr) noexcept { return !attr.IsBold(); }; } @@ -367,8 +373,8 @@ std::function UiaTextRangeBase::_getAttrVerification // The foreground color is stored as a COLORREF. const auto queryForegroundColor{ base::ClampedNumeric(val.lVal) }; - return [this, queryForegroundColor](const TextAttribute& attr) { - return (_pData->GetAttributeColors(attr).first & 0x00ffffff) == queryForegroundColor; + return [this, queryForegroundColor](const TextAttribute& attr) noexcept { + return _RemoveAlpha(_pData->GetAttributeColors(attr).first) == queryForegroundColor; }; } case UIA_IsItalicAttributeId: @@ -380,13 +386,13 @@ std::function UiaTextRangeBase::_getAttrVerification const auto queryIsItalic{ val.boolVal }; if (queryIsItalic) { - return [](const TextAttribute& attr) { + return [](const TextAttribute& attr) noexcept { return attr.IsItalic(); }; } else { - return [](const TextAttribute& attr) { + return [](const TextAttribute& attr) noexcept { return !attr.IsItalic(); }; } @@ -402,11 +408,11 @@ std::function UiaTextRangeBase::_getAttrVerification switch (val.lVal) { case TextDecorationLineStyle_None: - return [](const TextAttribute& attr) { + return [](const TextAttribute& attr) noexcept { return !attr.IsCrossedOut(); }; case TextDecorationLineStyle_Single: - return [](const TextAttribute& attr) { + return [](const TextAttribute& attr) noexcept { return attr.IsCrossedOut(); }; default: @@ -424,15 +430,15 @@ std::function UiaTextRangeBase::_getAttrVerification switch (val.lVal) { case TextDecorationLineStyle_None: - return [](const TextAttribute& attr) { + return [](const TextAttribute& attr) noexcept { return !attr.IsUnderlined() && !attr.IsDoublyUnderlined(); }; case TextDecorationLineStyle_Double: - return [](const TextAttribute& attr) { + return [](const TextAttribute& attr) noexcept { return attr.IsDoublyUnderlined(); }; case TextDecorationLineStyle_Single: - return [](const TextAttribute& attr) { + return [](const TextAttribute& attr) noexcept { return attr.IsUnderlined(); }; default: @@ -448,6 +454,7 @@ IFACEMETHODIMP UiaTextRangeBase::FindAttribute(_In_ TEXTATTRIBUTEID attributeId, _In_ VARIANT val, _In_ BOOL searchBackwards, _Outptr_result_maybenull_ ITextRangeProvider** ppRetVal) noexcept +try { RETURN_HR_IF(E_INVALIDARG, ppRetVal == nullptr); *ppRetVal = nullptr; @@ -477,7 +484,7 @@ IFACEMETHODIMP UiaTextRangeBase::FindAttribute(_In_ TEXTATTRIBUTEID attributeId, return S_OK; } default: - __fallthrough; + break; } // AttributeIDs that are exposed via TextAttribute @@ -517,9 +524,18 @@ IFACEMETHODIMP UiaTextRangeBase::FindAttribute(_In_ TEXTATTRIBUTEID attributeId, // Iterate from searchStart to searchEnd in the buffer. // If we find the attribute we're looking for, we update resultFirstAnchor/SecondAnchor appropriately. - const Viewport viewportRange{ _blockRange ? Viewport::FromDimensions(_start, inclusiveEnd.X - _start.X + 1, inclusiveEnd.Y - _start.Y + 1) : bufferSize }; + Viewport viewportRange{ bufferSize }; + if (_blockRange) + { + const auto originX{ std::min(_start.X, inclusiveEnd.X) }; + const auto originY{ std::min(_start.Y, inclusiveEnd.Y) }; + const auto width{ gsl::narrow_cast(std::abs(inclusiveEnd.X - _start.X + 1)) }; + const auto height{ gsl::narrow_cast(std::abs(inclusiveEnd.Y - _start.Y + 1)) }; + viewportRange = Viewport::FromDimensions({ originX, originY }, width, height); + } auto iter{ buffer.GetCellDataAt(searchStart, viewportRange, searchEnd) }; - for (; iter; searchBackwards ? --iter : ++iter) + const auto iterStep{ searchBackwards ? -1 : 1 }; + for (; iter; iter += iterStep) { const auto& attr{ iter->TextAttr() }; if (checkIfAttrFound(attr)) @@ -560,13 +576,14 @@ IFACEMETHODIMP UiaTextRangeBase::FindAttribute(_In_ TEXTATTRIBUTEID attributeId, // We need to make the end exclusive! // But be careful here, we might be a block range auto exclusiveIter{ buffer.GetCellDataAt(range._end, viewportRange) }; - exclusiveIter++; + ++exclusiveIter; range._end = exclusiveIter.Pos(); } UiaTracing::TextRange::FindAttribute(*this, attributeId, val, searchBackwards, static_cast(**ppRetVal)); return S_OK; } +CATCH_RETURN(); IFACEMETHODIMP UiaTextRangeBase::FindText(_In_ BSTR text, _In_ BOOL searchBackward, @@ -639,7 +656,7 @@ std::function UiaTextRangeBase::_getAttrVerification case UIA_BackgroundColorAttributeId: { pRetVal->vt = VT_I4; - pRetVal->lVal = base::ClampedNumeric(_pData->GetAttributeColors(attr).second & 0x00ffffff); + pRetVal->lVal = _RemoveAlpha(_pData->GetAttributeColors(attr).second); return _getAttrVerificationFn(attributeId, *pRetVal); } case UIA_FontWeightAttributeId: @@ -649,23 +666,13 @@ std::function UiaTextRangeBase::_getAttrVerification // we just store "IsBold" and "IsFaint". // Source: https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids pRetVal->vt = VT_I4; - if (attr.IsBold()) - { - pRetVal->lVal = FW_BOLD; - } - else - { - pRetVal->lVal = FW_NORMAL; - } + pRetVal->lVal = attr.IsBold() ? FW_BOLD : FW_NORMAL; return _getAttrVerificationFn(attributeId, *pRetVal); } case UIA_ForegroundColorAttributeId: { pRetVal->vt = VT_I4; - const auto x{ _pData->GetAttributeColors(attr).first }; - const auto y{ x & 0x00ffffff }; - pRetVal->lVal = y; - //pRetVal->lVal = base::ClampedNumeric(_pData->GetAttributeColors(attr).first & 0x00ffffff); + pRetVal->lVal = _RemoveAlpha(_pData->GetAttributeColors(attr).first); return _getAttrVerificationFn(attributeId, *pRetVal); } case UIA_IsItalicAttributeId: @@ -677,7 +684,7 @@ std::function UiaTextRangeBase::_getAttrVerification case UIA_StrikethroughStyleAttributeId: { pRetVal->vt = VT_I4; - pRetVal->lVal = static_cast(attr.IsCrossedOut() ? TextDecorationLineStyle_Single : TextDecorationLineStyle_None); + pRetVal->lVal = attr.IsCrossedOut() ? TextDecorationLineStyle_Single : TextDecorationLineStyle_None; return _getAttrVerificationFn(attributeId, *pRetVal); } case UIA_UnderlineStyleAttributeId: @@ -685,15 +692,15 @@ std::function UiaTextRangeBase::_getAttrVerification pRetVal->vt = VT_I4; if (attr.IsDoublyUnderlined()) { - pRetVal->lVal = static_cast(TextDecorationLineStyle_Double); + pRetVal->lVal = TextDecorationLineStyle_Double; } else if (attr.IsUnderlined()) { - pRetVal->lVal = static_cast(TextDecorationLineStyle_Single); + pRetVal->lVal = TextDecorationLineStyle_Single; } else { - pRetVal->lVal = static_cast(TextDecorationLineStyle_None); + pRetVal->lVal = TextDecorationLineStyle_None; } return _getAttrVerificationFn(attributeId, *pRetVal); } @@ -707,6 +714,7 @@ std::function UiaTextRangeBase::_getAttrVerification IFACEMETHODIMP UiaTextRangeBase::GetAttributeValue(_In_ TEXTATTRIBUTEID attributeId, _Out_ VARIANT* pRetVal) noexcept +try { RETURN_HR_IF(E_INVALIDARG, pRetVal == nullptr); VariantInit(pRetVal); @@ -729,7 +737,7 @@ IFACEMETHODIMP UiaTextRangeBase::GetAttributeValue(_In_ TEXTATTRIBUTEID attribut return S_OK; } default: - __fallthrough; + break; } // AttributeIDs that are exposed via TextAttribute @@ -769,7 +777,15 @@ IFACEMETHODIMP UiaTextRangeBase::GetAttributeValue(_In_ TEXTATTRIBUTEID attribut const auto inclusiveEnd{ _getInclusiveEnd() }; // Check if the entire text range has that text attribute - const Viewport viewportRange{ _blockRange ? Viewport::FromDimensions(_start, inclusiveEnd.X - _start.X + 1, inclusiveEnd.Y - _start.Y + 1) : bufferSize }; + Viewport viewportRange{ bufferSize }; + if (_blockRange) + { + const auto originX{ std::min(_start.X, inclusiveEnd.X) }; + const auto originY{ std::min(_start.Y, inclusiveEnd.Y) }; + const auto width{ gsl::narrow_cast(std::abs(inclusiveEnd.X - _start.X + 1)) }; + const auto height{ gsl::narrow_cast(std::abs(inclusiveEnd.Y - _start.Y + 1)) }; + viewportRange = Viewport::FromDimensions({ originX, originY }, width, height); + } auto iter{ buffer.GetCellDataAt(_start, viewportRange, inclusiveEnd) }; for (; iter; ++iter) { @@ -788,6 +804,7 @@ IFACEMETHODIMP UiaTextRangeBase::GetAttributeValue(_In_ TEXTATTRIBUTEID attribut UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal); return S_OK; } +CATCH_RETURN(); IFACEMETHODIMP UiaTextRangeBase::GetBoundingRectangles(_Outptr_result_maybenull_ SAFEARRAY** ppRetVal) noexcept { @@ -1643,7 +1660,7 @@ RECT UiaTextRangeBase::_getTerminalRect() const }; } -COORD UiaTextRangeBase::_getInclusiveEnd() +COORD UiaTextRangeBase::_getInclusiveEnd() noexcept { auto result{ _end }; _pData->GetTextBuffer().GetSize().DecrementInBounds(result, true); diff --git a/src/types/UiaTextRangeBase.hpp b/src/types/UiaTextRangeBase.hpp index 19a3c130c16..15e27e269cc 100644 --- a/src/types/UiaTextRangeBase.hpp +++ b/src/types/UiaTextRangeBase.hpp @@ -180,7 +180,7 @@ namespace Microsoft::Console::Types std::function _getAttrVerificationFn(TEXTATTRIBUTEID attributeId, VARIANT val) const; std::function _getAttrVerificationFnForFirstAttr(TEXTATTRIBUTEID attributeId, VARIANT* pRetVal) const; - COORD _getInclusiveEnd(); + COORD _getInclusiveEnd() noexcept; #ifdef UNIT_TESTING friend class ::UiaTextRangeTests; diff --git a/src/types/UiaTracing.cpp b/src/types/UiaTracing.cpp index 461fccb6906..8c8a1c0d315 100644 --- a/src/types/UiaTracing.cpp +++ b/src/types/UiaTracing.cpp @@ -126,102 +126,6 @@ inline std::wstring UiaTracing::_getValue(const TextUnit unit) noexcept } } -std::wstring UiaTracing::convertAttributeId(const TEXTATTRIBUTEID attrId) noexcept -{ - // Source: https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids - switch (attrId) - { - case UIA_AfterParagraphSpacingAttributeId: - return L"AfterParagraphSpacing"; - case UIA_AnimationStyleAttributeId: - return L"AnimationStyle"; - case UIA_AnnotationObjectsAttributeId: - return L"AnnotationObjects"; - case UIA_AnnotationTypesAttributeId: - return L"AnnotationTypes"; - case UIA_BackgroundColorAttributeId: - return L"BackgroundColor"; - case UIA_BeforeParagraphSpacingAttributeId: - return L"BeforeParagraphSpacing"; - case UIA_BulletStyleAttributeId: - return L"BulletStyle"; - case UIA_CapStyleAttributeId: - return L"CapStyle"; - case UIA_CaretBidiModeAttributeId: - return L"CaretBidiMode"; - case UIA_CaretPositionAttributeId: - return L"CaretPosition"; - case UIA_CultureAttributeId: - return L"Culture"; - case UIA_FontNameAttributeId: - return L"FontName"; - case UIA_FontSizeAttributeId: - return L"FontSize"; - case UIA_FontWeightAttributeId: - return L"FontWeight"; - case UIA_ForegroundColorAttributeId: - return L"ForegroundColor"; - case UIA_HorizontalTextAlignmentAttributeId: - return L"HorizontalTextAlignment"; - case UIA_IndentationFirstLineAttributeId: - return L"IndentationFirstLine"; - case UIA_IndentationLeadingAttributeId: - return L"IndentationLeading"; - case UIA_IndentationTrailingAttributeId: - return L"IndentationTrailing"; - case UIA_IsActiveAttributeId: - return L"IsActive"; - case UIA_IsHiddenAttributeId: - return L"IsHidden"; - case UIA_IsItalicAttributeId: - return L"IsItalic"; - case UIA_IsReadOnlyAttributeId: - return L"IsReadOnly"; - case UIA_IsSubscriptAttributeId: - return L"IsSubscript"; - case UIA_IsSuperscriptAttributeId: - return L"IsSuperscript"; - case UIA_LineSpacingAttributeId: - return L"LineSpacing"; - case UIA_LinkAttributeId: - return L"Link"; - case UIA_MarginBottomAttributeId: - return L"MarginBottom"; - case UIA_MarginLeadingAttributeId: - return L"MarginLeading"; - case UIA_MarginTopAttributeId: - return L"MarginTop"; - case UIA_MarginTrailingAttributeId: - return L"MarginTrailing"; - case UIA_OutlineStylesAttributeId: - return L"OutlineStyles"; - case UIA_OverlineColorAttributeId: - return L"OverlineColor"; - case UIA_OverlineStyleAttributeId: - return L"OverlineStyle"; - case UIA_SelectionActiveEndAttributeId: - return L"SelectionActiveEnd"; - case UIA_StrikethroughColorAttributeId: - return L"StrikethroughColor"; - case UIA_StrikethroughStyleAttributeId: - return L"StrikethroughStyle"; - case UIA_StyleIdAttributeId: - return L"StyleId"; - case UIA_StyleNameAttributeId: - return L"StyleName"; - case UIA_TabsAttributeId: - return L"Tabs"; - case UIA_TextFlowDirectionsAttributeId: - return L"TextFlowDirections"; - case UIA_UnderlineColorAttributeId: - return L"UnderlineColor"; - case UIA_UnderlineStyleAttributeId: - return L"UnderlineStyle"; - default: - return L"Unknown attribute"; - } -} - inline std::wstring UiaTracing::_getValue(const VARIANT val) noexcept { // This is not a comprehensive conversion of VARIANT result to string @@ -235,7 +139,7 @@ inline std::wstring UiaTracing::_getValue(const VARIANT val) noexcept case VT_BOOL: return std::to_wstring(val.boolVal); case VT_I4: - return std::to_wstring(val.iVal); + return std::to_wstring(val.lVal); case VT_UNKNOWN: default: { @@ -349,7 +253,7 @@ void UiaTracing::TextRange::FindAttribute(const UiaTextRangeBase& utr, TEXTATTRI g_UiaProviderTraceProvider, "UiaTextRange::FindAttribute", TraceLoggingValue(_getValue(utr).c_str(), "base"), - TraceLoggingValue(convertAttributeId(id).c_str(), "text attribute ID"), + TraceLoggingValue(id, "text attribute ID"), TraceLoggingValue(_getValue(val).c_str(), "text attribute sub-data"), TraceLoggingValue(searchBackwards ? L"true" : L"false", "search backwards"), TraceLoggingValue(_getValue(attrType).c_str(), "attribute type"), @@ -386,7 +290,7 @@ void UiaTracing::TextRange::GetAttributeValue(const UiaTextRangeBase& base, TEXT g_UiaProviderTraceProvider, "UiaTextRange::GetAttributeValue", TraceLoggingValue(_getValue(base).c_str(), "base"), - TraceLoggingValue(convertAttributeId(id).c_str(), "text attribute ID"), + TraceLoggingValue(id, "text attribute ID"), TraceLoggingValue(_getValue(result).c_str(), "result"), TraceLoggingValue(_getValue(attrType).c_str(), "attribute type"), TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), diff --git a/src/types/UiaTracing.h b/src/types/UiaTracing.h index a1bd20a04a2..3cb4b7e9106 100644 --- a/src/types/UiaTracing.h +++ b/src/types/UiaTracing.h @@ -29,8 +29,6 @@ namespace Microsoft::Console::Types class UiaTracing final { public: - static std::wstring convertAttributeId(const TEXTATTRIBUTEID attrId) noexcept; - enum class AttributeType { Standard, From 2be632bae85790840c9d95b9b9f6af01806108c2 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 29 Jun 2021 16:32:43 -0700 Subject: [PATCH 4/9] remove 'limit' from new iterator --- src/buffer/out/textBuffer.cpp | 40 ++---- src/buffer/out/textBuffer.hpp | 5 +- src/buffer/out/textBufferCellIterator.cpp | 48 ++----- src/buffer/out/textBufferCellIterator.hpp | 4 +- src/host/ut_host/TextBufferIteratorTests.cpp | 138 ++----------------- src/types/UiaTextRangeBase.cpp | 8 +- 6 files changed, 40 insertions(+), 203 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index e56d4f7c630..76d12708639 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -149,13 +149,13 @@ TextBufferTextIterator TextBuffer::GetTextLineDataAt(const COORD at) const // - Read-only iterator of cell data. TextBufferCellIterator TextBuffer::GetCellLineDataAt(const COORD at) const { - SMALL_RECT bounds; - bounds.Top = at.Y; - bounds.Bottom = at.Y; - bounds.Left = 0; - bounds.Right = GetSize().RightInclusive(); + SMALL_RECT limit; + limit.Top = at.Y; + limit.Bottom = at.Y; + limit.Left = 0; + limit.Right = GetSize().RightInclusive(); - return TextBufferCellIterator(*this, at, Viewport::FromInclusive(bounds)); + return TextBufferCellIterator(*this, at, Viewport::FromInclusive(limit)); } // Routine Description: @@ -163,12 +163,12 @@ TextBufferCellIterator TextBuffer::GetCellLineDataAt(const COORD at) const // but restricted to operate only inside the given viewport. // Arguments: // - at - X,Y position in buffer for iterator start position -// - bounds - boundaries for the iterator to operate within +// - limit - boundaries for the iterator to operate within // Return Value: // - Read-only iterator of text data only. -TextBufferTextIterator TextBuffer::GetTextDataAt(const COORD at, const Viewport bounds) const +TextBufferTextIterator TextBuffer::GetTextDataAt(const COORD at, const Viewport limit) const { - return TextBufferTextIterator(GetCellDataAt(at, bounds)); + return TextBufferTextIterator(GetCellDataAt(at, limit)); } // Routine Description: @@ -176,28 +176,12 @@ TextBufferTextIterator TextBuffer::GetTextDataAt(const COORD at, const Viewport // but restricted to operate only inside the given viewport. // Arguments: // - at - X,Y position in buffer for iterator start position -// - bounds - boundaries for the iterator to operate within +// - limit - boundaries for the iterator to operate within // Return Value: // - Read-only iterator of cell data. -TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport bounds) const +TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport limit) const { - return TextBufferCellIterator(*this, at, bounds); -} - -// Routine Description: -// - Retrieves read-only cell iterator at the given buffer location -// but restricted to operate only inside the given viewport. -// Arguments: -// - at - X,Y position in buffer for iterator start position -// - bounds - viewport boundaries for the iterator to operate within. -// Allows for us to iterate over a sub-grid of the buffer. -// - limit - X,Y position in buffer for the iterator end position (inclusive). -// Allows for us to iterate through "bounds" until we hit the end of "bounds" or the limit. -// Return Value: -// - Read-only iterator of cell data. -TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport bounds, const COORD limit) const -{ - return TextBufferCellIterator(*this, at, bounds, limit); + return TextBufferCellIterator(*this, at, limit); } //Routine Description: diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index d84b8d6c5d0..468a9fc4f34 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -80,11 +80,10 @@ class TextBuffer final TextBufferCellIterator GetCellDataAt(const COORD at) const; TextBufferCellIterator GetCellLineDataAt(const COORD at) const; - TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport bounds) const; - TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport bounds, const COORD limit) const; + TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit) const; TextBufferTextIterator GetTextDataAt(const COORD at) const; TextBufferTextIterator GetTextLineDataAt(const COORD at) const; - TextBufferTextIterator GetTextDataAt(const COORD at, const Microsoft::Console::Types::Viewport bounds) const; + TextBufferTextIterator GetTextDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit) const; // Text insertion functions OutputCellIterator Write(const OutputCellIterator givenIt); diff --git a/src/buffer/out/textBufferCellIterator.cpp b/src/buffer/out/textBufferCellIterator.cpp index 318e999ffc8..64ed7acefc4 100644 --- a/src/buffer/out/textBufferCellIterator.cpp +++ b/src/buffer/out/textBufferCellIterator.cpp @@ -29,44 +29,27 @@ TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD p // Arguments: // - buffer - Pointer to screen buffer to seek through // - pos - Starting position to retrieve text data from (within screen buffer bounds) -// - bounds - Viewport boundaries to restrict the iterator within the buffer bounds (smaller than the buffer itself) -TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport bounds) : +// - limits - Viewport limits to restrict the iterator within the buffer bounds (smaller than the buffer itself) +TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport limits) : _buffer(buffer), _pos(pos), _pRow(s_GetRow(buffer, pos)), - _bounds(bounds), - _limit(std::nullopt), + _bounds(limits), _exceeded(false), _view({}, {}, {}, TextAttributeBehavior::Stored), _attrIter(s_GetRow(buffer, pos)->GetAttrRow().cbegin()) { // Throw if the bounds rectangle is not limited to the inside of the given buffer. - THROW_HR_IF(E_INVALIDARG, !buffer.GetSize().IsInBounds(bounds)); + THROW_HR_IF(E_INVALIDARG, !buffer.GetSize().IsInBounds(limits)); // Throw if the coordinate is not limited to the inside of the given buffer. - THROW_HR_IF(E_INVALIDARG, !bounds.IsInBounds(pos)); + THROW_HR_IF(E_INVALIDARG, !limits.IsInBounds(pos)); _attrIter += pos.X; _GenerateView(); } -// Routine Description: -// - Creates a new read-only iterator to seek through cell data stored within a screen buffer -// Arguments: -// - buffer - Text buffer to seek through -// - pos - Starting position to retrieve text data from (within screen buffer bounds) -// - bounds - Viewport boundaries to restrict the iterator within the buffer bounds (smaller than the buffer itself) -// - limit - last position to iterate through (inclusive) -TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport bounds, const COORD limit) : - TextBufferCellIterator(buffer, pos, bounds) -{ - // Throw if the coordinate is not limited to the inside of the given buffer. - THROW_HR_IF(E_INVALIDARG, !_bounds.IsInBounds(limit)); - - _limit = limit; -} - // Routine Description: // - Tells if the iterator is still valid (hasn't exceeded boundaries of underlying text buffer) // Return Value: @@ -89,8 +72,7 @@ bool TextBufferCellIterator::operator==(const TextBufferCellIterator& it) const _exceeded == it._exceeded && _bounds == it._bounds && _pRow == it._pRow && - _attrIter == it._attrIter && - _limit == _limit; + _attrIter == it._attrIter; } // Routine Description: @@ -116,26 +98,12 @@ TextBufferCellIterator& TextBufferCellIterator::operator+=(const ptrdiff_t& move auto newPos = _pos; while (move > 0 && !_exceeded) { - // If we have a limit, check if we've exceeded it - if (_limit.has_value()) - { - _exceeded |= (newPos == *_limit); - } - - // If we already exceeded limit, we'll short-circuit and _not_ increment - _exceeded = _exceeded || !_bounds.IncrementInBounds(newPos); + _exceeded = !_bounds.IncrementInBounds(newPos); move--; } while (move < 0 && !_exceeded) { - // If we have an endPos, check if we've exceeded it - if (_limit.has_value()) - { - _exceeded |= (newPos == _limit); - } - - // If we already exceeded from endPos, we'll short-circuit and _not_ decrement - _exceeded |= !_bounds.DecrementInBounds(newPos); + _exceeded = !_bounds.DecrementInBounds(newPos); move++; } _SetPos(newPos); diff --git a/src/buffer/out/textBufferCellIterator.hpp b/src/buffer/out/textBufferCellIterator.hpp index 1877834412d..db8a3c89ce6 100644 --- a/src/buffer/out/textBufferCellIterator.hpp +++ b/src/buffer/out/textBufferCellIterator.hpp @@ -26,8 +26,7 @@ class TextBufferCellIterator { public: TextBufferCellIterator(const TextBuffer& buffer, COORD pos); - TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport bounds); - TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport bounds, const COORD limit); + TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport limits); operator bool() const noexcept; @@ -63,7 +62,6 @@ class TextBufferCellIterator const Microsoft::Console::Types::Viewport _bounds; bool _exceeded; COORD _pos; - std::optional _limit; #if UNIT_TESTING friend class TextBufferIteratorTests; diff --git a/src/host/ut_host/TextBufferIteratorTests.cpp b/src/host/ut_host/TextBufferIteratorTests.cpp index 278ebaaf13d..197deffa143 100644 --- a/src/host/ut_host/TextBufferIteratorTests.cpp +++ b/src/host/ut_host/TextBufferIteratorTests.cpp @@ -264,11 +264,8 @@ class TextBufferIteratorTests TEST_METHOD(DereferenceOperatorText); TEST_METHOD(DereferenceOperatorCell); - TEST_METHOD(ConstructedNoBounds); - TEST_METHOD(ConstructedBounds); - TEST_METHOD(ConstructedLimit); - TEST_METHOD(ConstructedLimitBackwards); - TEST_METHOD(ConstructedBoundsAndLimit); + TEST_METHOD(ConstructedNoLimit); + TEST_METHOD(ConstructedLimits); }; template @@ -513,7 +510,7 @@ void TextBufferIteratorTests::DereferenceOperatorCell() VERIFY_ARE_EQUAL(attrExpected, attrActual); } -void TextBufferIteratorTests::ConstructedNoBounds() +void TextBufferIteratorTests::ConstructedNoLimit() { m_state->FillTextBuffer(); @@ -526,7 +523,6 @@ void TextBufferIteratorTests::ConstructedNoBounds() VERIFY_IS_TRUE(it, L"Iterator is valid."); VERIFY_ARE_EQUAL(bufferSize, it._bounds, L"Bounds match the bounds of the text buffer."); - VERIFY_IS_FALSE(it._limit.has_value(), L"Positional limit is not defined."); const auto totalBufferDistance = bufferSize.Width() * bufferSize.Height(); @@ -542,7 +538,7 @@ void TextBufferIteratorTests::ConstructedNoBounds() VERIFY_THROWS_SPECIFIC(TextBufferCellIterator(textBuffer, { -1, -1 }), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); } -void TextBufferIteratorTests::ConstructedBounds() +void TextBufferIteratorTests::ConstructedLimits() { m_state->FillTextBuffer(); @@ -550,22 +546,21 @@ void TextBufferIteratorTests::ConstructedBounds() const auto& outputBuffer = gci.GetActiveOutputBuffer(); const auto& textBuffer = outputBuffer.GetTextBuffer(); - SMALL_RECT bounds; - bounds.Top = 1; - bounds.Bottom = 1; - bounds.Left = 3; - bounds.Right = 5; - const auto viewport = Microsoft::Console::Types::Viewport::FromInclusive(bounds); + SMALL_RECT limits; + limits.Top = 1; + limits.Bottom = 1; + limits.Left = 3; + limits.Right = 5; + const auto viewport = Microsoft::Console::Types::Viewport::FromInclusive(limits); COORD pos; - pos.X = bounds.Left; - pos.Y = bounds.Top; + pos.X = limits.Left; + pos.Y = limits.Top; TextBufferCellIterator it(textBuffer, pos, viewport); VERIFY_IS_TRUE(it, L"Iterator is valid."); VERIFY_ARE_EQUAL(viewport, it._bounds, L"Bounds match the bounds given."); - VERIFY_IS_FALSE(it._limit.has_value(), L"Positional limit is not defined."); const auto totalBufferDistance = viewport.Width() * viewport.Height(); @@ -584,7 +579,7 @@ void TextBufferIteratorTests::ConstructedBounds() wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); - // Verify throws for bounds not inside buffer + // Verify throws for limit not inside buffer const auto bufferSize = textBuffer.GetSize(); VERIFY_THROWS_SPECIFIC(TextBufferCellIterator(textBuffer, pos, @@ -592,110 +587,3 @@ void TextBufferIteratorTests::ConstructedBounds() wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); } - -void TextBufferIteratorTests::ConstructedLimit() -{ - m_state->FillTextBuffer(); - - const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - const auto& outputBuffer = gci.GetActiveOutputBuffer(); - const auto& textBuffer = outputBuffer.GetTextBuffer(); - const auto bufferSize = textBuffer.GetSize(); - - const COORD pos{ bufferSize.Origin() }; - const COORD limitPos{ 5, 1 }; - - TextBufferCellIterator it(textBuffer, pos, bufferSize, limitPos); - - VERIFY_IS_TRUE(it, L"Iterator is valid."); - VERIFY_ARE_EQUAL(bufferSize, it._bounds, L"Bounds match the bounds given."); - VERIFY_IS_TRUE(it._limit.has_value(), L"Positional limit is defined."); - VERIFY_ARE_EQUAL(limitPos, it._limit.value(), L"Positional limit matches the one given."); - - const auto totalBufferDistance = bufferSize.Width() + 5; - - // Advance buffer to one before the positional limit. - it += totalBufferDistance; - VERIFY_IS_TRUE(it, L"Iterator is still valid."); - - // Advance over the positional limit. - it++; - VERIFY_IS_FALSE(it, L"Iterator invalid now."); - - // Verify throws for positional limit not inside buffer - VERIFY_THROWS_SPECIFIC(TextBufferCellIterator(textBuffer, - pos, - bufferSize, - bufferSize.BottomRightExclusive()), - wil::ResultException, - [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); -} - -void TextBufferIteratorTests::ConstructedLimitBackwards() -{ - m_state->FillTextBuffer(); - - const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - const auto& outputBuffer = gci.GetActiveOutputBuffer(); - const auto& textBuffer = outputBuffer.GetTextBuffer(); - const auto bufferSize = textBuffer.GetSize(); - - const COORD pos{ 5, 1 }; - const COORD limitPos{ 2, 0 }; - - TextBufferCellIterator it(textBuffer, pos, bufferSize, limitPos); - - VERIFY_IS_TRUE(it, L"Iterator is valid."); - VERIFY_ARE_EQUAL(bufferSize, it._bounds, L"Bounds match the bounds given."); - VERIFY_IS_TRUE(it._limit.has_value(), L"Positional limit is defined."); - VERIFY_ARE_EQUAL(limitPos, it._limit.value(), L"Positional limit matches the one given."); - - const auto totalBufferDistance = bufferSize.Width() + 5 - 2; - - // Advance buffer to one before the positional limit. - it -= totalBufferDistance; - VERIFY_IS_TRUE(it, L"Iterator is still valid."); - - // Advance over the positional limit. - it--; - VERIFY_IS_FALSE(it, L"Iterator invalid now."); -} - -void TextBufferIteratorTests::ConstructedBoundsAndLimit() -{ - m_state->FillTextBuffer(); - - const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - const auto& outputBuffer = gci.GetActiveOutputBuffer(); - const auto& textBuffer = outputBuffer.GetTextBuffer(); - - SMALL_RECT bounds; - bounds.Top = 1; - bounds.Bottom = 2; - bounds.Left = 2; - bounds.Right = 5; - const auto viewport = Microsoft::Console::Types::Viewport::FromInclusive(bounds); - - COORD pos; - pos.X = bounds.Left; - pos.Y = bounds.Top; - - const COORD limitPos{ 4, 1 }; - - TextBufferCellIterator it(textBuffer, pos, viewport, limitPos); - - VERIFY_IS_TRUE(it, L"Iterator is valid."); - VERIFY_ARE_EQUAL(viewport, it._bounds, L"Bounds match the bounds given."); - VERIFY_IS_TRUE(it._limit.has_value(), L"Positional limit is defined."); - VERIFY_ARE_EQUAL(limitPos, it._limit.value(), L"Positional limit matches the one given."); - - const auto totalBufferDistance = 2; - - // Advance buffer to one before the positional limit. - it += totalBufferDistance; - VERIFY_IS_TRUE(it, L"Iterator is still valid."); - - // Advance over the positional limit. - it++; - VERIFY_IS_FALSE(it, L"Iterator invalid now."); -} diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index 55a93b5576e..915511eb75b 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -533,9 +533,9 @@ try const auto height{ gsl::narrow_cast(std::abs(inclusiveEnd.Y - _start.Y + 1)) }; viewportRange = Viewport::FromDimensions({ originX, originY }, width, height); } - auto iter{ buffer.GetCellDataAt(searchStart, viewportRange, searchEnd) }; + auto iter{ buffer.GetCellDataAt(searchStart, viewportRange) }; const auto iterStep{ searchBackwards ? -1 : 1 }; - for (; iter; iter += iterStep) + for (; iter && iter.Pos() != searchEnd; iter += iterStep) { const auto& attr{ iter->TextAttr() }; if (checkIfAttrFound(attr)) @@ -786,8 +786,8 @@ try const auto height{ gsl::narrow_cast(std::abs(inclusiveEnd.Y - _start.Y + 1)) }; viewportRange = Viewport::FromDimensions({ originX, originY }, width, height); } - auto iter{ buffer.GetCellDataAt(_start, viewportRange, inclusiveEnd) }; - for (; iter; ++iter) + auto iter{ buffer.GetCellDataAt(_start, viewportRange) }; + for (; iter && iter.Pos() != inclusiveEnd; ++iter) { const auto& attr{ iter->TextAttr() }; if (!checkIfAttrFound(attr)) From 84fdf5add15d4dbf3a69229b09d0e790c76e4326 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 29 Jun 2021 16:43:24 -0700 Subject: [PATCH 5/9] bugfix: italic was not detected properly --- src/cascadia/TerminalControl/XamlUiaTextRange.cpp | 2 +- src/types/UiaTextRangeBase.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalControl/XamlUiaTextRange.cpp b/src/cascadia/TerminalControl/XamlUiaTextRange.cpp index eb40018e22c..7e5dbcf6767 100644 --- a/src/cascadia/TerminalControl/XamlUiaTextRange.cpp +++ b/src/cascadia/TerminalControl/XamlUiaTextRange.cpp @@ -114,7 +114,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation } case VT_BOOL: { - return box_value(result.boolVal); + return box_value(result.boolVal); } case VT_UNKNOWN: { diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index 915511eb75b..b623916fae9 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -678,7 +678,7 @@ std::function UiaTextRangeBase::_getAttrVerification case UIA_IsItalicAttributeId: { pRetVal->vt = VT_BOOL; - pRetVal->lVal = attr.IsItalic(); + pRetVal->boolVal = attr.IsItalic(); return _getAttrVerificationFn(attributeId, *pRetVal); } case UIA_StrikethroughStyleAttributeId: From 3fa8c95f639e814522ef398c02d2e7dd13ae284c Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 6 Jul 2021 16:26:47 -0700 Subject: [PATCH 6/9] fix 1 char wide case; better degenerate range handling; don't return lambda for attr verification --- src/cascadia/TerminalCore/Terminal.hpp | 4 +- src/types/UiaTextRangeBase.cpp | 143 +++++++++++-------------- src/types/UiaTextRangeBase.hpp | 4 +- 3 files changed, 68 insertions(+), 83 deletions(-) diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 65fd5f031a3..e1feeafd1b9 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -279,8 +279,8 @@ class Microsoft::Terminal::Core::Terminal final : std::wstring _workingDirectory; - // This font value is only used to check if the font is a raster font. - // Otherwise, the font is changed with the renderer via TriggerFontChange. + // This default fake font value is only used to check if the font is a raster font. + // Otherwise, the font is changed to a real value with the renderer via TriggerFontChange. FontInfo _fontInfo{ DEFAULT_FONT_FACE, TMPF_TRUETYPE, 10, { 0, DEFAULT_FONT_SIZE }, CP_UTF8, false }; #pragma region Text Selection // a selection is represented as a range between two COORDs (start and end) diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index b623916fae9..dd86878b89f 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -317,13 +317,16 @@ IFACEMETHODIMP UiaTextRangeBase::ExpandToEnclosingUnit(_In_ TextUnit unit) noexc } // Method Description: -// - Generate an attribute verification function for that attributeId and sub-type +// - Verify that the given attribute has the desired formatting saved in the attributeId and val // Arguments: // - attributeId - the UIA text attribute identifier we're looking for // - val - the attributeId's sub-type we're looking for +// - attr - the text attribute we're checking // Return Value: -// - a function that can be used to verify if a given TextAttribute meets the attributeId's sub-type -std::function UiaTextRangeBase::_getAttrVerificationFn(TEXTATTRIBUTEID attributeId, VARIANT val) const +// - true, if the given attribute has the desired formatting. +// - false, if the given attribute does not have the desired formatting. +// - nullopt, if checking for the desired formatting is not supported. +std::optional UiaTextRangeBase::_verifyAttr(TEXTATTRIBUTEID attributeId, VARIANT val, const TextAttribute& attr) const { // Most of the attributes we're looking for just require us to check TextAttribute. // So if we support it, we'll return a function to verify if the TextAttribute @@ -337,9 +340,7 @@ std::function UiaTextRangeBase::_getAttrVerification // The foreground color is stored as a COLORREF. const auto queryBackgroundColor{ val.lVal }; - return [this, queryBackgroundColor](const TextAttribute& attr) noexcept { - return _RemoveAlpha(_pData->GetAttributeColors(attr).second) == queryBackgroundColor; - }; + return _RemoveAlpha(_pData->GetAttributeColors(attr).second) == queryBackgroundColor; } case UIA_FontWeightAttributeId: { @@ -354,16 +355,12 @@ std::function UiaTextRangeBase::_getAttrVerification if (queryFontWeight > FW_NORMAL) { // we're looking for a bold font weight - return [](const TextAttribute& attr) noexcept { - return attr.IsBold(); - }; + return attr.IsBold(); } else { // we're looking for "normal" font weight - return [](const TextAttribute& attr) noexcept { - return !attr.IsBold(); - }; + return !attr.IsBold(); } } case UIA_ForegroundColorAttributeId: @@ -373,9 +370,7 @@ std::function UiaTextRangeBase::_getAttrVerification // The foreground color is stored as a COLORREF. const auto queryForegroundColor{ base::ClampedNumeric(val.lVal) }; - return [this, queryForegroundColor](const TextAttribute& attr) noexcept { - return _RemoveAlpha(_pData->GetAttributeColors(attr).first) == queryForegroundColor; - }; + return _RemoveAlpha(_pData->GetAttributeColors(attr).first) == queryForegroundColor; } case UIA_IsItalicAttributeId: { @@ -384,18 +379,7 @@ std::function UiaTextRangeBase::_getAttrVerification // The text is either italic or it isn't. const auto queryIsItalic{ val.boolVal }; - if (queryIsItalic) - { - return [](const TextAttribute& attr) noexcept { - return attr.IsItalic(); - }; - } - else - { - return [](const TextAttribute& attr) noexcept { - return !attr.IsItalic(); - }; - } + return queryIsItalic ? attr.IsItalic() : !attr.IsItalic(); } case UIA_StrikethroughStyleAttributeId: { @@ -408,15 +392,11 @@ std::function UiaTextRangeBase::_getAttrVerification switch (val.lVal) { case TextDecorationLineStyle_None: - return [](const TextAttribute& attr) noexcept { - return !attr.IsCrossedOut(); - }; + return !attr.IsCrossedOut(); case TextDecorationLineStyle_Single: - return [](const TextAttribute& attr) noexcept { - return attr.IsCrossedOut(); - }; + return attr.IsCrossedOut(); default: - return nullptr; + return std::nullopt; } } case UIA_UnderlineStyleAttributeId: @@ -430,23 +410,17 @@ std::function UiaTextRangeBase::_getAttrVerification switch (val.lVal) { case TextDecorationLineStyle_None: - return [](const TextAttribute& attr) noexcept { - return !attr.IsUnderlined() && !attr.IsDoublyUnderlined(); - }; + return !attr.IsUnderlined() && !attr.IsDoublyUnderlined(); case TextDecorationLineStyle_Double: - return [](const TextAttribute& attr) noexcept { - return attr.IsDoublyUnderlined(); - }; + return attr.IsDoublyUnderlined(); case TextDecorationLineStyle_Single: - return [](const TextAttribute& attr) noexcept { - return attr.IsUnderlined(); - }; + return attr.IsUnderlined(); default: - return nullptr; + return std::nullopt; } } default: - return nullptr; + return std::nullopt; } } @@ -465,6 +439,9 @@ try case UIA_FontNameAttributeId: { RETURN_HR_IF(E_INVALIDARG, val.vt != VT_BSTR); + + // Technically, we'll truncate early if there's an embedded null in the BSTR. + // But we're probably fine in this curcumstance. const std::wstring queryFontName{ val.bstrVal }; if (queryFontName == _pData->GetFontInfo().GetFaceName()) { @@ -488,11 +465,9 @@ try } // AttributeIDs that are exposed via TextAttribute - std::function checkIfAttrFound; try { - checkIfAttrFound = _getAttrVerificationFn(attributeId, val); - if (!checkIfAttrFound) + if (!_verifyAttr(attributeId, val, {}).has_value()) { // The AttributeID is not supported. UiaTracing::TextRange::FindAttribute(*this, attributeId, val, searchBackwards, static_cast(**ppRetVal), UiaTracing::AttributeType::Unsupported); @@ -519,8 +494,19 @@ try std::optional resultSecondAnchor; // Start/End for the direction to perform the search in + // We need searchEnd to be exclusive. This allows the for-loop below to + // iterate up until the exclusive searchEnd, and not attempt to read the + // data at that position. const auto searchStart{ searchBackwards ? inclusiveEnd : _start }; - const auto searchEnd{ searchBackwards ? _start : inclusiveEnd }; + auto searchEndExclusive{ searchBackwards ? _start : inclusiveEnd }; + if (searchBackwards) + { + bufferSize.DecrementInBounds(searchEndExclusive, true); + } + else + { + bufferSize.IncrementInBounds(searchEndExclusive, true); + } // Iterate from searchStart to searchEnd in the buffer. // If we find the attribute we're looking for, we update resultFirstAnchor/SecondAnchor appropriately. @@ -535,16 +521,16 @@ try } auto iter{ buffer.GetCellDataAt(searchStart, viewportRange) }; const auto iterStep{ searchBackwards ? -1 : 1 }; - for (; iter && iter.Pos() != searchEnd; iter += iterStep) + for (; iter && iter.Pos() != searchEndExclusive; iter += iterStep) { - const auto& attr{ iter->TextAttr() }; - if (checkIfAttrFound(attr)) + if (_verifyAttr(attributeId, val, iter->TextAttr()).value()) { // populate the first anchor if it's not populated. // otherwise, populate the second anchor. if (!resultFirstAnchor.has_value()) { resultFirstAnchor = iter.Pos(); + resultSecondAnchor = iter.Pos(); } else { @@ -558,6 +544,7 @@ try // - the anchors have been populated // This means that we've found a contiguous range where the text attribute was found. // No point in searching through the rest of the search space. + // TLDR: keep updating the second anchor and make the range wider until the attribute changes. break; } } @@ -640,24 +627,24 @@ CATCH_RETURN(); // Method Description: // - (1) Checks the current range for the attributeId's sub-type // - (2) Record the attributeId's sub-type -// - (3) Generate an attribute verification function for that attributeId sub-type // Arguments: // - attributeId - the UIA text attribute identifier we're looking for // - pRetVal - the attributeId's sub-type for the first cell in the range (i.e. foreground color) +// - attr - the text attribute we're checking // Return Value: -// - a function that can be used to verify if a given TextAttribute meets the attributeId's sub-type -std::function UiaTextRangeBase::_getAttrVerificationFnForFirstAttr(TEXTATTRIBUTEID attributeId, VARIANT* pRetVal) const +// - true, if the attributeId is supported. false, otherwise. +// - pRetVal is populated with the appropriate response relevant to the returned bool. +bool UiaTextRangeBase::_initializeAttrQuery(TEXTATTRIBUTEID attributeId, VARIANT* pRetVal, const TextAttribute& attr) const { THROW_HR_IF(E_INVALIDARG, pRetVal == nullptr); - const auto attr{ _pData->GetTextBuffer().GetCellDataAt(_start)->TextAttr() }; switch (attributeId) { case UIA_BackgroundColorAttributeId: { pRetVal->vt = VT_I4; pRetVal->lVal = _RemoveAlpha(_pData->GetAttributeColors(attr).second); - return _getAttrVerificationFn(attributeId, *pRetVal); + return true; } case UIA_FontWeightAttributeId: { @@ -667,25 +654,25 @@ std::function UiaTextRangeBase::_getAttrVerification // Source: https://docs.microsoft.com/en-us/windows/win32/winauto/uiauto-textattribute-ids pRetVal->vt = VT_I4; pRetVal->lVal = attr.IsBold() ? FW_BOLD : FW_NORMAL; - return _getAttrVerificationFn(attributeId, *pRetVal); + return true; } case UIA_ForegroundColorAttributeId: { pRetVal->vt = VT_I4; pRetVal->lVal = _RemoveAlpha(_pData->GetAttributeColors(attr).first); - return _getAttrVerificationFn(attributeId, *pRetVal); + return true; } case UIA_IsItalicAttributeId: { pRetVal->vt = VT_BOOL; pRetVal->boolVal = attr.IsItalic(); - return _getAttrVerificationFn(attributeId, *pRetVal); + return true; } case UIA_StrikethroughStyleAttributeId: { pRetVal->vt = VT_I4; pRetVal->lVal = attr.IsCrossedOut() ? TextDecorationLineStyle_Single : TextDecorationLineStyle_None; - return _getAttrVerificationFn(attributeId, *pRetVal); + return true; } case UIA_UnderlineStyleAttributeId: { @@ -702,13 +689,13 @@ std::function UiaTextRangeBase::_getAttrVerification { pRetVal->lVal = TextDecorationLineStyle_None; } - return _getAttrVerificationFn(attributeId, *pRetVal); + return true; } default: // This attribute is not supported. pRetVal->vt = VT_UNKNOWN; UiaGetReservedNotSupportedValue(&pRetVal->punkVal); - return nullptr; + return false; } } @@ -741,17 +728,27 @@ try } // AttributeIDs that are exposed via TextAttribute - std::function checkIfAttrFound; try { - checkIfAttrFound = _getAttrVerificationFnForFirstAttr(attributeId, pRetVal); - if (!checkIfAttrFound) + // Unlike a normal text editor, which applies formatting at the caret, + // we don't know what attributes are written at a degenerate range. + // So instead, we'll use GetCurrentAttributes to get an idea of the default + // text attributes used. And return a result based off of that. + const auto attr{ IsDegenerate() ? _pData->GetTextBuffer().GetCurrentAttributes() : + _pData->GetTextBuffer().GetCellDataAt(_start)->TextAttr() }; + if (!_initializeAttrQuery(attributeId, pRetVal, attr)) { // The AttributeID is not supported. pRetVal->vt = VT_UNKNOWN; UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal, UiaTracing::AttributeType::Unsupported); return UiaGetReservedNotSupportedValue(&pRetVal->punkVal); } + else if (IsDegenerate()) + { + // If we're a degenerate range, we have all the information we need. + UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal); + return S_OK; + } } catch (...) { @@ -760,17 +757,6 @@ try return E_INVALIDARG; } - if (IsDegenerate()) - { - // Unlike a normal text editor, which applies formatting at the caret, - // we don't know what attributes are written at a degenerate range. - // So let's return UiaGetReservedMixedAttributeValue. - // Source: https://docs.microsoft.com/en-us/windows/win32/api/uiautomationcore/nf-uiautomationcore-itextrangeprovider-getattributevalue - pRetVal->vt = VT_UNKNOWN; - UiaTracing::TextRange::GetAttributeValue(*this, attributeId, *pRetVal, UiaTracing::AttributeType::Mixed); - return UiaGetReservedMixedAttributeValue(&pRetVal->punkVal); - } - // Get some useful variables const auto& buffer{ _pData->GetTextBuffer() }; const auto bufferSize{ buffer.GetSize() }; @@ -789,8 +775,7 @@ try auto iter{ buffer.GetCellDataAt(_start, viewportRange) }; for (; iter && iter.Pos() != inclusiveEnd; ++iter) { - const auto& attr{ iter->TextAttr() }; - if (!checkIfAttrFound(attr)) + if (!_verifyAttr(attributeId, *pRetVal, iter->TextAttr()).value()) { // The value of the specified attribute varies over the text range // return UiaGetReservedMixedAttributeValue. diff --git a/src/types/UiaTextRangeBase.hpp b/src/types/UiaTextRangeBase.hpp index 15e27e269cc..6809478e8cc 100644 --- a/src/types/UiaTextRangeBase.hpp +++ b/src/types/UiaTextRangeBase.hpp @@ -177,8 +177,8 @@ namespace Microsoft::Console::Types gsl::not_null const pAmountMoved, _In_ const bool preventBufferEnd = false) noexcept; - std::function _getAttrVerificationFn(TEXTATTRIBUTEID attributeId, VARIANT val) const; - std::function _getAttrVerificationFnForFirstAttr(TEXTATTRIBUTEID attributeId, VARIANT* pRetVal) const; + std::optional _verifyAttr(TEXTATTRIBUTEID attributeId, VARIANT val, const TextAttribute& attr) const; + bool _initializeAttrQuery(TEXTATTRIBUTEID attributeId, VARIANT* pRetVal, const TextAttribute& attr) const; COORD _getInclusiveEnd() noexcept; From 74a645f8fc853c27bbcadf1e8c916989f0326ec1 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 7 Jul 2021 12:48:18 -0700 Subject: [PATCH 7/9] fix failing tests and audit mode --- .../UiaTextRangeTests.cpp | 2 + src/types/UiaTextRangeBase.cpp | 46 ++++++++++++------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp index 11d38fc4fba..6c441a48456 100644 --- a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp +++ b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp @@ -1617,6 +1617,8 @@ class UiaTextRangeTests THROW_IF_FAILED(result->Compare(resultBackwards.Get(), &isEqual)); VERIFY_IS_TRUE(isEqual); } + } + TEST_METHOD(BlockRange) { // This test replicates GH#7960. diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index 31cf6fe42f1..39a338dea4e 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -10,7 +10,7 @@ using namespace Microsoft::Console::Types; // Foreground/Background text color doesn't care about the alpha. -static long _RemoveAlpha(COLORREF color) +static constexpr long _RemoveAlpha(COLORREF color) noexcept { return color & 0x00ffffff; } @@ -493,13 +493,32 @@ try // We'll do some post-processing to fix this on the way out. std::optional resultFirstAnchor; std::optional resultSecondAnchor; + const auto attemptUpdateAnchors = [=, &resultFirstAnchor, &resultSecondAnchor](const TextBufferCellIterator iter) { + const auto attrFound{ _verifyAttr(attributeId, val, iter->TextAttr()).value() }; + if (attrFound) + { + // populate the first anchor if it's not populated. + // otherwise, populate the second anchor. + if (!resultFirstAnchor.has_value()) + { + resultFirstAnchor = iter.Pos(); + resultSecondAnchor = iter.Pos(); + } + else + { + resultSecondAnchor = iter.Pos(); + } + } + return attrFound; + }; // Start/End for the direction to perform the search in // We need searchEnd to be exclusive. This allows the for-loop below to // iterate up until the exclusive searchEnd, and not attempt to read the // data at that position. const auto searchStart{ searchBackwards ? inclusiveEnd : _start }; - auto searchEndExclusive{ searchBackwards ? _start : inclusiveEnd }; + const auto searchEndInclusive{ searchBackwards ? _start : inclusiveEnd }; + auto searchEndExclusive{ searchEndInclusive }; if (searchBackwards) { bufferSize.DecrementInBounds(searchEndExclusive, true); @@ -524,21 +543,7 @@ try const auto iterStep{ searchBackwards ? -1 : 1 }; for (; iter && iter.Pos() != searchEndExclusive; iter += iterStep) { - if (_verifyAttr(attributeId, val, iter->TextAttr()).value()) - { - // populate the first anchor if it's not populated. - // otherwise, populate the second anchor. - if (!resultFirstAnchor.has_value()) - { - resultFirstAnchor = iter.Pos(); - resultSecondAnchor = iter.Pos(); - } - else - { - resultSecondAnchor = iter.Pos(); - } - } - else if (resultFirstAnchor.has_value() && resultSecondAnchor.has_value()) + if (!attemptUpdateAnchors(iter) && resultFirstAnchor.has_value() && resultSecondAnchor.has_value()) { // Exit the loop early if... // - the cell we're looking at doesn't have the attr we're looking for @@ -550,6 +555,13 @@ try } } + // Corner case: we couldn't actually move the searchEnd to make it exclusive + // (i.e. DecrementInBounds on Origin doesn't move it) + if (searchEndInclusive == searchEndExclusive) + { + attemptUpdateAnchors(iter); + } + // If a result was found, populate ppRetVal with the UiaTextRange // representing the found selection anchors. if (resultFirstAnchor.has_value() && resultSecondAnchor.has_value()) From 2e7895c8456b4b9f02b0f1130f94b9710ea1ffac Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 7 Jul 2021 12:49:05 -0700 Subject: [PATCH 8/9] fix typo Co-authored-by: Michael Niksa --- src/types/UiaTextRangeBase.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index 39a338dea4e..032fefbd42e 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -442,7 +442,8 @@ try RETURN_HR_IF(E_INVALIDARG, val.vt != VT_BSTR); // Technically, we'll truncate early if there's an embedded null in the BSTR. - // But we're probably fine in this curcumstance. + // But we're probably fine in this circumstance. + const std::wstring queryFontName{ val.bstrVal }; if (queryFontName == _pData->GetFontInfo().GetFaceName()) { From 96a55511021905415c44dcbcfb4b705869416a1f Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 9 Jul 2021 15:56:28 -0700 Subject: [PATCH 9/9] apply Dustin's suggestions --- .../UiaTextRangeTests.cpp | 22 +++++++++++++++++++ src/types/UiaTextRangeBase.cpp | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp index 6c441a48456..5457b98537c 100644 --- a/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp +++ b/src/interactivity/win32/ut_interactivity_win32/UiaTextRangeTests.cpp @@ -1514,6 +1514,28 @@ class UiaTextRangeTests VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_IsReadOnlyAttributeId, &result)); VERIFY_IS_FALSE(result.boolVal); } + { + // "Mixed" is when the desired attribute value is inconsistent across the range. + // We'll make our life easier by setting an attribute on a character, + // but getting the attribute for the entire line. + Log::Comment(L"Test Mixed"); + VARIANT result; + THROW_IF_FAILED(utr->ExpandToEnclosingUnit(TextUnit_Line)); + + // set first cell as underlined, but second cell as not underlined + attr.SetUnderlined(true); + _pTextBuffer->Write({ attr }, { 0, 0 }); + attr.SetUnderlined(false); + _pTextBuffer->Write({ attr }, { 1, 0 }); + + VERIFY_SUCCEEDED(utr->GetAttributeValue(UIA_UnderlineStyleAttributeId, &result)); + + // Expected: mixed + Microsoft::WRL::ComPtr mixedVal; + THROW_IF_FAILED(UiaGetReservedMixedAttributeValue(&mixedVal)); + VERIFY_ARE_EQUAL(VT_UNKNOWN, result.vt); + VERIFY_ARE_EQUAL(mixedVal.Get(), result.punkVal); + } } TEST_METHOD(FindAttribute) diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index 032fefbd42e..69f531c09cc 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -370,7 +370,7 @@ std::optional UiaTextRangeBase::_verifyAttr(TEXTATTRIBUTEID attributeId, V THROW_HR_IF(E_INVALIDARG, val.vt != VT_I4); // The foreground color is stored as a COLORREF. - const auto queryForegroundColor{ base::ClampedNumeric(val.lVal) }; + const auto queryForegroundColor{ val.lVal }; return _RemoveAlpha(_pData->GetAttributeColors(attr).first) == queryForegroundColor; } case UIA_IsItalicAttributeId: