[a11y] Ensure buffer is initialized before interacting with it (#11312)
Adds a check before every UIA function call to ensure the terminal (specifically the buffer) is initialized before doing work. Both the `ScreenInfoUiaProvider` and the `UiaTextRange` are now covered. ## References Closes #11135 #10971 & #11042 ## Detailed Description of the Pull Request / Additional comments Originally, I tried applying this heuristic to all the `RuntimeClassInitialize` on `UiaTextRangeBase` with the philosophy of "a range pointing to an invalid buffer is invalid itself", but that caused a regression on [MSFT 33353327](https://microsoft.visualstudio.com/OS/_workitems/edit/33353327). `IUiaData` also has `GetTextBuffer()` return a `TextBuffer&`, which cannot be checked for nullness. Instead, I decided to add a function to `IUiaData` that checks if we have a valid state. Since this is shared with Conhost and Conhost doesn't have this issue, I simply make that function say that it's always in a valid state. ## Validation Steps Performed - [X] Narrator can detect newly created terminals - [X] (On Windows Server 2022) Windows Terminal does not hang on launch
This commit is contained in:
parent
c070be12d3
commit
0f122ca290
|
@ -12,6 +12,7 @@
|
|||
<EventProvider Id="EventProvider_TerminalWin32Host" Name="56c06166-2e2e-5f4d-7ff3-74f4b78c87d6" />
|
||||
<EventProvider Id="EventProvider_TerminalRemoting" Name="d6f04aad-629f-539a-77c1-73f5c3e4aa7b" />
|
||||
<EventProvider Id="EventProvider_TerminalDirectX" Name="c93e739e-ae50-5a14-78e7-f171e947535d" />
|
||||
<EventProvider Id="EventProvider_TerminalUIA" Name="e7ebce59-2161-572d-b263-2f16a6afb9e5"/>
|
||||
<Profile Id="Terminal.Verbose.File" Name="Terminal" Description="Terminal" LoggingMode="File" DetailLevel="Verbose">
|
||||
<Collectors>
|
||||
<EventCollectorId Value="EventCollector_Terminal">
|
||||
|
@ -23,6 +24,7 @@
|
|||
<EventProviderId Value="EventProvider_TerminalWin32Host" />
|
||||
<EventProviderId Value="EventProvider_TerminalRemoting" />
|
||||
<EventProviderId Value="EventProvider_TerminalDirectX" />
|
||||
<EventProviderId Value="EventProvider_TerminalUIA" />
|
||||
</EventProviders>
|
||||
</EventCollectorId>
|
||||
</Collectors>
|
||||
|
|
|
@ -199,6 +199,7 @@ public:
|
|||
const COORD GetSelectionEnd() const noexcept override;
|
||||
const std::wstring_view GetConsoleTitle() const noexcept override;
|
||||
void ColorSelection(const COORD coordSelectionStart, const COORD coordSelectionEnd, const TextAttribute) override;
|
||||
const bool IsUiaDataInitialized() const noexcept override;
|
||||
#pragma endregion
|
||||
|
||||
void SetWriteInputCallback(std::function<void(std::wstring&)> pfn) noexcept;
|
||||
|
|
|
@ -250,3 +250,12 @@ bool Terminal::IsScreenReversed() const noexcept
|
|||
{
|
||||
return _screenReversed;
|
||||
}
|
||||
|
||||
const bool Terminal::IsUiaDataInitialized() const noexcept
|
||||
{
|
||||
// GH#11135: Windows Terminal needs to create and return an automation peer
|
||||
// when a screen reader requests it. However, the terminal might not be fully
|
||||
// initialized yet. So we use this to check if any crucial components of
|
||||
// UiaData are not yet initialized.
|
||||
return !!_buffer;
|
||||
}
|
||||
|
|
|
@ -69,5 +69,6 @@ public:
|
|||
const COORD GetSelectionAnchor() const noexcept;
|
||||
const COORD GetSelectionEnd() const noexcept;
|
||||
void ColorSelection(const COORD coordSelectionStart, const COORD coordSelectionEnd, const TextAttribute attr);
|
||||
const bool IsUiaDataInitialized() const noexcept override { return true; }
|
||||
#pragma endregion
|
||||
};
|
||||
|
|
|
@ -392,6 +392,11 @@ public:
|
|||
{
|
||||
}
|
||||
|
||||
const bool IsUiaDataInitialized() const noexcept
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
const std::wstring GetHyperlinkUri(uint16_t /*id*/) const noexcept
|
||||
{
|
||||
return {};
|
||||
|
|
|
@ -40,6 +40,7 @@ namespace Microsoft::Console::Types
|
|||
virtual const COORD GetSelectionAnchor() const noexcept = 0;
|
||||
virtual const COORD GetSelectionEnd() const noexcept = 0;
|
||||
virtual void ColorSelection(const COORD coordSelectionStart, const COORD coordSelectionEnd, const TextAttribute attr) = 0;
|
||||
virtual const bool IsUiaDataInitialized() const noexcept = 0;
|
||||
};
|
||||
|
||||
// See docs/virtual-dtors.md for an explanation of why this is weird.
|
||||
|
|
|
@ -220,21 +220,19 @@ IFACEMETHODIMP ScreenInfoUiaProviderBase::SetFocus()
|
|||
|
||||
IFACEMETHODIMP ScreenInfoUiaProviderBase::GetSelection(_Outptr_result_maybenull_ SAFEARRAY** ppRetVal)
|
||||
{
|
||||
RETURN_HR_IF_NULL(E_INVALIDARG, ppRetVal);
|
||||
*ppRetVal = nullptr;
|
||||
|
||||
_LockConsole();
|
||||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_UnlockConsole();
|
||||
});
|
||||
|
||||
RETURN_HR_IF_NULL(E_INVALIDARG, ppRetVal);
|
||||
*ppRetVal = nullptr;
|
||||
HRESULT hr = S_OK;
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
// make a safe array
|
||||
HRESULT hr = S_OK;
|
||||
*ppRetVal = SafeArrayCreateVector(VT_UNKNOWN, 0, 1);
|
||||
if (*ppRetVal == nullptr)
|
||||
{
|
||||
return E_OUTOFMEMORY;
|
||||
}
|
||||
RETURN_HR_IF_NULL(E_OUTOFMEMORY, *ppRetVal);
|
||||
|
||||
WRL::ComPtr<UiaTextRangeBase> range;
|
||||
if (!_pData->IsSelectionActive())
|
||||
|
@ -272,19 +270,18 @@ IFACEMETHODIMP ScreenInfoUiaProviderBase::GetSelection(_Outptr_result_maybenull_
|
|||
|
||||
IFACEMETHODIMP ScreenInfoUiaProviderBase::GetVisibleRanges(_Outptr_result_maybenull_ SAFEARRAY** ppRetVal)
|
||||
{
|
||||
RETURN_HR_IF_NULL(E_INVALIDARG, ppRetVal);
|
||||
*ppRetVal = nullptr;
|
||||
|
||||
_LockConsole();
|
||||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_UnlockConsole();
|
||||
});
|
||||
|
||||
RETURN_HR_IF_NULL(E_INVALIDARG, ppRetVal);
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
// make a safe array
|
||||
*ppRetVal = SafeArrayCreateVector(VT_UNKNOWN, 0, 1);
|
||||
if (*ppRetVal == nullptr)
|
||||
{
|
||||
return E_OUTOFMEMORY;
|
||||
}
|
||||
RETURN_HR_IF_NULL(E_OUTOFMEMORY, *ppRetVal);
|
||||
|
||||
WRL::ComPtr<UiaTextRangeBase> range;
|
||||
const auto bufferSize = _pData->GetTextBuffer().GetSize();
|
||||
|
|
|
@ -16,9 +16,6 @@ HRESULT TermControlUiaProvider::RuntimeClassInitialize(_In_ ::Microsoft::Console
|
|||
RETURN_IF_FAILED(ScreenInfoUiaProviderBase::RuntimeClassInitialize(uiaData));
|
||||
|
||||
_controlInfo = controlInfo;
|
||||
|
||||
// TODO GitHub #1914: Re-attach Tracing to UIA Tree
|
||||
//Tracing::s_TraceUia(nullptr, ApiCall::Constructor, nullptr);
|
||||
return S_OK;
|
||||
}
|
||||
|
||||
|
@ -26,11 +23,6 @@ IFACEMETHODIMP TermControlUiaProvider::Navigate(_In_ NavigateDirection direction
|
|||
_COM_Outptr_result_maybenull_ IRawElementProviderFragment** ppProvider) noexcept
|
||||
{
|
||||
RETURN_HR_IF_NULL(E_INVALIDARG, ppProvider);
|
||||
|
||||
// TODO GitHub #1914: Re-attach Tracing to UIA Tree
|
||||
/*ApiMsgNavigate apiMsg;
|
||||
apiMsg.Direction = direction;
|
||||
Tracing::s_TraceUia(this, ApiCall::Navigate, &apiMsg);*/
|
||||
*ppProvider = nullptr;
|
||||
|
||||
if (direction == NavigateDirection_Parent)
|
||||
|
@ -122,6 +114,12 @@ HRESULT TermControlUiaProvider::GetSelectionRange(_In_ IRawElementProviderSimple
|
|||
RETURN_HR_IF_NULL(E_INVALIDARG, ppUtr);
|
||||
*ppUtr = nullptr;
|
||||
|
||||
_pData->LockConsole();
|
||||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized() || !_pData->IsSelectionActive());
|
||||
|
||||
const auto start = _pData->GetSelectionAnchor();
|
||||
|
||||
// we need to make end exclusive
|
||||
|
|
|
@ -63,20 +63,6 @@ IFACEMETHODIMP TermControlUiaTextRange::Clone(_Outptr_result_maybenull_ ITextRan
|
|||
return hr;
|
||||
}
|
||||
|
||||
#if defined(_DEBUG) && defined(UiaTextRangeBase_DEBUG_MSGS)
|
||||
OutputDebugString(L"Clone\n");
|
||||
std::wstringstream ss;
|
||||
ss << _id << L" cloned to " << (static_cast<UiaTextRangeBase*>(*ppRetVal))->_id;
|
||||
std::wstring str = ss.str();
|
||||
OutputDebugString(str.c_str());
|
||||
OutputDebugString(L"\n");
|
||||
#endif
|
||||
// TODO GitHub #1914: Re-attach Tracing to UIA Tree
|
||||
// tracing
|
||||
/*ApiMsgClone apiMsg;
|
||||
apiMsg.CloneId = static_cast<UiaTextRangeBase*>(*ppRetVal)->GetId();
|
||||
Tracing::s_TraceUia(this, ApiCall::Clone, &apiMsg);*/
|
||||
|
||||
return S_OK;
|
||||
}
|
||||
|
||||
|
|
|
@ -220,15 +220,18 @@ IFACEMETHODIMP UiaTextRangeBase::CompareEndpoints(_In_ TextPatternRangeEndpoint
|
|||
_Out_ int* pRetVal) noexcept
|
||||
try
|
||||
{
|
||||
RETURN_HR_IF(E_INVALIDARG, pRetVal == nullptr);
|
||||
RETURN_HR_IF_NULL(E_INVALIDARG, pRetVal);
|
||||
*pRetVal = 0;
|
||||
|
||||
_pData->LockConsole();
|
||||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
// get the text range that we're comparing to
|
||||
const UiaTextRangeBase* range = static_cast<UiaTextRangeBase*>(pTargetRange);
|
||||
if (range == nullptr)
|
||||
{
|
||||
return E_INVALIDARG;
|
||||
}
|
||||
RETURN_HR_IF_NULL(E_INVALIDARG, range);
|
||||
|
||||
// get endpoint value that we're comparing to
|
||||
const auto other = range->GetEndpoint(targetEndpoint);
|
||||
|
@ -240,10 +243,7 @@ try
|
|||
// This is a temporary solution to comparing two UTRs from different TextBuffers
|
||||
// Ensure both endpoints fit in the current buffer.
|
||||
const auto bufferSize = _pData->GetTextBuffer().GetSize();
|
||||
if (!bufferSize.IsInBounds(mine, true) || !bufferSize.IsInBounds(other, true))
|
||||
{
|
||||
return E_FAIL;
|
||||
}
|
||||
RETURN_HR_IF(E_FAIL, !bufferSize.IsInBounds(mine, true) || !bufferSize.IsInBounds(other, true));
|
||||
|
||||
// compare them
|
||||
*pRetVal = bufferSize.CompareInBounds(mine, other, true);
|
||||
|
@ -259,6 +259,7 @@ IFACEMETHODIMP UiaTextRangeBase::ExpandToEnclosingUnit(_In_ TextUnit unit) noexc
|
|||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
try
|
||||
{
|
||||
|
@ -444,6 +445,12 @@ try
|
|||
RETURN_HR_IF(E_INVALIDARG, ppRetVal == nullptr);
|
||||
*ppRetVal = nullptr;
|
||||
|
||||
_pData->LockConsole();
|
||||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
// AttributeIDs that require special handling
|
||||
switch (attributeId)
|
||||
{
|
||||
|
@ -605,6 +612,12 @@ try
|
|||
RETURN_HR_IF(E_INVALIDARG, ppRetVal == nullptr);
|
||||
*ppRetVal = nullptr;
|
||||
|
||||
_pData->LockConsole();
|
||||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
const std::wstring queryText{ text, SysStringLen(text) };
|
||||
const auto bufferSize = _getOptimizedBufferSize();
|
||||
const auto sensitivity = ignoreCase ? Search::Sensitivity::CaseInsensitive : Search::Sensitivity::CaseSensitive;
|
||||
|
@ -730,6 +743,12 @@ try
|
|||
RETURN_HR_IF(E_INVALIDARG, pRetVal == nullptr);
|
||||
VariantInit(pRetVal);
|
||||
|
||||
_pData->LockConsole();
|
||||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
// AttributeIDs that require special handling
|
||||
switch (attributeId)
|
||||
{
|
||||
|
@ -817,13 +836,14 @@ CATCH_RETURN();
|
|||
|
||||
IFACEMETHODIMP UiaTextRangeBase::GetBoundingRectangles(_Outptr_result_maybenull_ SAFEARRAY** ppRetVal) noexcept
|
||||
{
|
||||
RETURN_HR_IF(E_INVALIDARG, ppRetVal == nullptr);
|
||||
*ppRetVal = nullptr;
|
||||
|
||||
_pData->LockConsole();
|
||||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
|
||||
RETURN_HR_IF(E_INVALIDARG, ppRetVal == nullptr);
|
||||
*ppRetVal = nullptr;
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
try
|
||||
{
|
||||
|
@ -925,21 +945,19 @@ CATCH_RETURN();
|
|||
IFACEMETHODIMP UiaTextRangeBase::GetText(_In_ int maxLength, _Out_ BSTR* pRetVal) noexcept
|
||||
try
|
||||
{
|
||||
RETURN_HR_IF(E_INVALIDARG, pRetVal == nullptr);
|
||||
RETURN_HR_IF_NULL(E_INVALIDARG, pRetVal);
|
||||
RETURN_HR_IF(E_INVALIDARG, maxLength < -1);
|
||||
*pRetVal = nullptr;
|
||||
|
||||
if (maxLength < -1)
|
||||
{
|
||||
return E_INVALIDARG;
|
||||
}
|
||||
_pData->LockConsole();
|
||||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
const auto maxLengthOpt = (maxLength == -1) ?
|
||||
std::nullopt :
|
||||
std::optional<unsigned int>{ maxLength };
|
||||
_pData->LockConsole();
|
||||
auto Unlock = wil::scope_exit([this]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
const auto text = _getTextValue(maxLengthOpt);
|
||||
Unlock.reset();
|
||||
|
||||
|
@ -1013,6 +1031,7 @@ try
|
|||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
// We can abstract this movement by moving _start
|
||||
// GH#7342: check if we're past the documentEnd
|
||||
|
@ -1075,15 +1094,13 @@ IFACEMETHODIMP UiaTextRangeBase::MoveEndpointByUnit(_In_ TextPatternRangeEndpoin
|
|||
{
|
||||
RETURN_HR_IF(E_INVALIDARG, pRetVal == nullptr);
|
||||
*pRetVal = 0;
|
||||
if (count == 0)
|
||||
{
|
||||
return S_OK;
|
||||
}
|
||||
|
||||
_pData->LockConsole();
|
||||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
RETURN_HR_IF(S_OK, count == 0);
|
||||
|
||||
// GH#7342: check if we're past the documentEnd
|
||||
// If so, clamp each endpoint to the end of the document.
|
||||
|
@ -1141,10 +1158,8 @@ try
|
|||
});
|
||||
|
||||
const UiaTextRangeBase* range = static_cast<UiaTextRangeBase*>(pTargetRange);
|
||||
if (range == nullptr)
|
||||
{
|
||||
return E_INVALIDARG;
|
||||
}
|
||||
RETURN_HR_IF_NULL(E_INVALIDARG, range);
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
// TODO GH#5406: create a different UIA parent object for each TextBuffer
|
||||
// This is a temporary solution to comparing two UTRs from different TextBuffers
|
||||
|
@ -1152,10 +1167,7 @@ try
|
|||
const auto bufferSize = _pData->GetTextBuffer().GetSize();
|
||||
const auto mine = GetEndpoint(endpoint);
|
||||
const auto other = range->GetEndpoint(targetEndpoint);
|
||||
if (!bufferSize.IsInBounds(mine, true) || !bufferSize.IsInBounds(other, true))
|
||||
{
|
||||
return E_FAIL;
|
||||
}
|
||||
RETURN_HR_IF(E_FAIL, !bufferSize.IsInBounds(mine, true) || !bufferSize.IsInBounds(other, true));
|
||||
|
||||
SetEndpoint(endpoint, range->GetEndpoint(targetEndpoint));
|
||||
|
||||
|
@ -1171,6 +1183,7 @@ try
|
|||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
if (IsDegenerate())
|
||||
{
|
||||
|
@ -1215,6 +1228,7 @@ try
|
|||
auto Unlock = wil::scope_exit([&]() noexcept {
|
||||
_pData->UnlockConsole();
|
||||
});
|
||||
RETURN_HR_IF(E_FAIL, !_pData->IsUiaDataInitialized());
|
||||
|
||||
const auto oldViewport = _pData->GetViewport().ToInclusive();
|
||||
const auto viewportHeight = _getViewportHeight(oldViewport);
|
||||
|
|
Loading…
Reference in a new issue