From 20c67974782cd5cf25ed6d8c84f7d3520e3e80c5 Mon Sep 17 00:00:00 2001
From: James Teh <jteh@mozilla.com>
Date: Fri, 4 Nov 2022 02:31:10 +0000
Subject: [PATCH] Bug 1798098: Prevent a11y tree walks by the Suggested Actions
feature in Windows 11 22H2. r=nlapre, a=RyanVM
The patches in bug 1774285 prevent Suggested Actions a11y tree walking in most cases.
However, when a11y is already enabled, we can still get into a tree walk that triggers a hang when using Copy Phone Number on a tel: link.
This is because there is no selected text in this case, so Suggested Actions falls back to walking the tree.
This patch prevents these walks by returning 1 from get_nSelections within a timeout period after setting the clipboard, indicating that there is a selection when there isn't really.
Unfortunately, even though we lie about the selection count, the selection reported by get_selection still isn't valid.
Fixing that for a selection deep in the tree is difficult; we don't have enough information in the parent process to fake the selection properly.
Thus, Suggested Actions might then do a normal tree walk from the document, so we also return a 0 child count within this timeout period.
With the cache disabled, in-process Windows clients access content process Accessibles using COM proxies.
We must therefore hack get_nSelections and get_accChildCount in AccessibleHandler, which wraps these COM proxies for in-process clients.
This means that Firefox needs to be installed in order for this to work, since AccessibleHandler can only be used with an installed copy.
A11y performance without the cache is very poor without AccessibleHandler anyway.
Because AccessibleHandler is an entirely separate dll, we need to duplicate the suppression logic in AccessibleHandlerControl, which can be accessed by both Gecko and AccessibleHandler.
After all these attempts on the document, Suggested Actions falls back to walking from the root.
To prevent that walk, we return 0 for the child count on the root MsaaAccessible within the timeout period.
Differential Revision: https://phabricator.services.mozilla.com/D160746
---
.../ipc/win/handler/AccessibleHandler.cpp | 44 ++++++++++++++++++-
.../win/handler/AccessibleHandlerControl.cpp | 16 +++++++
.../win/handler/AccessibleHandlerControl.h | 6 +++
accessible/ipc/win/handler/HandlerData.idl | 1 +
accessible/windows/msaa/AccessibleWrap.cpp | 17 +++++++
accessible/windows/msaa/AccessibleWrap.h | 2 +
accessible/windows/msaa/Compatibility.cpp | 3 +-
accessible/windows/msaa/MsaaAccessible.cpp | 10 +++++
8 files changed, 97 insertions(+), 2 deletions(-)
diff --git a/accessible/ipc/win/handler/AccessibleHandler.cpp b/accessible/ipc/win/handler/AccessibleHandler.cpp
index 673bf8b3568e8..bf70af4f35a15 100644
--- a/accessible/ipc/win/handler/AccessibleHandler.cpp
+++ b/accessible/ipc/win/handler/AccessibleHandler.cpp
@@ -687,6 +687,25 @@ AccessibleHandler::get_accChildCount(long* pcountChildren) {
}
BEGIN_CACHE_ACCESS;
+ if (mCachedData.mDynamicData.mIA2Role == ROLE_SYSTEM_DOCUMENT) {
+ RefPtr<AccessibleHandlerControl> ctl(
+ gControlFactory.GetOrCreateSingleton());
+ if (!ctl) {
+ return E_OUTOFMEMORY;
+ }
+ if (ctl->IsA11ySuppressedForClipboardCopy()) {
+ // Bug 1798098: Windows Suggested Actions (introduced in Windows 11
+ // 22H2) might walk the document a11y tree using UIA whenever anything
+ // is copied to the clipboard. This causes an unacceptable hang,
+ // particularly when the cache is disabled. Even though we lie about the
+ // selection in nSelections, it falls back to a normal tree walk on the
+ // document if it doesn't get a proper text selection. Prevent that by
+ // returning a 0 child count on the document.
+ *pcountChildren = 0;
+ return S_OK;
+ }
+ }
+
GET_FIELD(mChildCount, *pcountChildren);
return S_OK;
}
@@ -1824,7 +1843,30 @@ AccessibleHandler::get_nSelections(long* nSelections) {
return hr;
}
- return mIAHypertextPassThru->get_nSelections(nSelections);
+ hr = mIAHypertextPassThru->get_nSelections(nSelections);
+ if (SUCCEEDED(hr) && *nSelections == 0 && HasPayload()) {
+ BEGIN_CACHE_ACCESS;
+ if (mCachedData.mDynamicData.mIA2Role == ROLE_SYSTEM_DOCUMENT) {
+ RefPtr<AccessibleHandlerControl> ctl(
+ gControlFactory.GetOrCreateSingleton());
+ if (!ctl) {
+ return E_OUTOFMEMORY;
+ }
+ if (ctl->IsA11ySuppressedForClipboardCopy()) {
+ // Bug 1798098: Windows Suggested Actions (introduced in Windows 11
+ // 22H2) might walk the document a11y tree using UIA whenever anything
+ // is copied to the clipboard. This causes an unacceptable hang,
+ // particularly when the cache is disabled. It walks using
+ // IAccessibleText/IAccessibleHyperText if the document reports no
+ // selection, so we lie here and say that there is a selection even
+ // though there isn't. It will subsequently call get_selection, which
+ // will fail, but this hack here seems to be enough to avoid further
+ // text calls.
+ *nSelections = 1;
+ }
+ }
+ }
+ return hr;
}
HRESULT
diff --git a/accessible/ipc/win/handler/AccessibleHandlerControl.cpp b/accessible/ipc/win/handler/AccessibleHandlerControl.cpp
index 24324c1b5e3d9..0d0bd4d71d94e 100644
--- a/accessible/ipc/win/handler/AccessibleHandlerControl.cpp
+++ b/accessible/ipc/win/handler/AccessibleHandlerControl.cpp
@@ -201,5 +201,21 @@ HRESULT AccessibleHandlerControl::GetCachedAccessible(
return S_OK;
}
+HRESULT AccessibleHandlerControl::SuppressA11yForClipboardCopy() {
+ mA11yClipboardCopySuppressionStartTime = ::GetTickCount();
+ return S_OK;
+}
+
+bool AccessibleHandlerControl::IsA11ySuppressedForClipboardCopy() {
+ // Must be kept in sync with kSuppressTimeout in
+ // accessible/windows/msaa/Compatibility.cpp.
+ constexpr DWORD kSuppressTimeout = 1500; // ms
+ if (!mA11yClipboardCopySuppressionStartTime) {
+ return false;
+ }
+ return ::GetTickCount() - mA11yClipboardCopySuppressionStartTime <
+ kSuppressTimeout;
+}
+
} // namespace a11y
} // namespace mozilla
diff --git a/accessible/ipc/win/handler/AccessibleHandlerControl.h b/accessible/ipc/win/handler/AccessibleHandlerControl.h
index d016ecb98b0ed..9a94ff9fe8185 100644
--- a/accessible/ipc/win/handler/AccessibleHandlerControl.h
+++ b/accessible/ipc/win/handler/AccessibleHandlerControl.h
@@ -62,6 +62,7 @@ class AccessibleHandlerControl final : public IHandlerControl {
STDMETHODIMP OnTextChange(long aHwnd, long aIA2UniqueId,
VARIANT_BOOL aIsInsert,
IA2TextSegment* aText) override;
+ STDMETHODIMP SuppressA11yForClipboardCopy() override;
uint32_t GetCacheGen() const { return mCacheGen; }
@@ -75,6 +76,8 @@ class AccessibleHandlerControl final : public IHandlerControl {
void CacheAccessible(long aUniqueId, AccessibleHandler* aAccessible);
HRESULT GetCachedAccessible(long aUniqueId, AccessibleHandler** aAccessible);
+ bool IsA11ySuppressedForClipboardCopy();
+
private:
AccessibleHandlerControl();
~AccessibleHandlerControl() = default;
@@ -87,6 +90,9 @@ class AccessibleHandlerControl final : public IHandlerControl {
// We can't use Gecko APIs in this dll, hence the use of std::unordered_map.
typedef std::unordered_map<long, RefPtr<AccessibleHandler>> AccessibleCache;
AccessibleCache mAccessibleCache;
+ // Time when SuppressA11yForClipboardCopy() was called, as returned by
+ // ::GetTickCount().
+ DWORD mA11yClipboardCopySuppressionStartTime = 0;
};
extern mscom::SingletonFactory<AccessibleHandlerControl> gControlFactory;
diff --git a/accessible/ipc/win/handler/HandlerData.idl b/accessible/ipc/win/handler/HandlerData.idl
index 0cf51d9468dae..e449a8826344d 100644
--- a/accessible/ipc/win/handler/HandlerData.idl
+++ b/accessible/ipc/win/handler/HandlerData.idl
@@ -87,6 +87,7 @@ interface IHandlerControl : IUnknown
HRESULT OnTextChange([in] long aHwnd, [in] long aIA2UniqueId,
[in] VARIANT_BOOL aIsInsert,
[in] IA2TextSegment* aText);
+ HRESULT SuppressA11yForClipboardCopy();
}
typedef struct _IARelationData
diff --git a/accessible/windows/msaa/AccessibleWrap.cpp b/accessible/windows/msaa/AccessibleWrap.cpp
index e299f04b33d46..d10ca81d7dc19 100644
--- a/accessible/windows/msaa/AccessibleWrap.cpp
+++ b/accessible/windows/msaa/AccessibleWrap.cpp
@@ -267,3 +267,20 @@ bool AccessibleWrap::DispatchTextChangeToHandler(Accessible* aAcc,
return SUCCEEDED(hr);
}
+
+/* static */
+void AccessibleWrap::SuppressHandlerA11yForClipboardCopy() {
+ if (!sHandlerControllers || sHandlerControllers->IsEmpty()) {
+ return;
+ }
+ // The original intent was that AccessibleHandler would be used in any
+ // process that wanted to access Gecko a11y. That didn't work out for various
+ // reasons. In practice, there is only a single AccessibleHandlerControl which
+ // is for our own parent process, used for in-process client calls. That also
+ // means we don't need to worry about async invokation here.
+ auto& controller = sHandlerControllers->ElementAt(0);
+ MOZ_ASSERT(controller.mPid == ::GetCurrentProcessId() &&
+ !controller.mIsProxy);
+ DebugOnly<HRESULT> hr = controller.mCtrl->SuppressA11yForClipboardCopy();
+ MOZ_ASSERT(SUCCEEDED(hr));
+}
diff --git a/accessible/windows/msaa/AccessibleWrap.h b/accessible/windows/msaa/AccessibleWrap.h
index a511e577ac546..dc089dfba41ab 100644
--- a/accessible/windows/msaa/AccessibleWrap.h
+++ b/accessible/windows/msaa/AccessibleWrap.h
@@ -67,6 +67,8 @@ class AccessibleWrap : public LocalAccessible {
const nsString& aText, int32_t aStart,
uint32_t aLen);
+ static void SuppressHandlerA11yForClipboardCopy();
+
protected:
virtual ~AccessibleWrap() = default;
diff --git a/accessible/windows/msaa/Compatibility.cpp b/accessible/windows/msaa/Compatibility.cpp
index 3fdda8f866c62..ee00f39caae0d 100644
--- a/accessible/windows/msaa/Compatibility.cpp
+++ b/accessible/windows/msaa/Compatibility.cpp
@@ -268,12 +268,13 @@ void Compatibility::SuppressA11yForClipboardCopy() {
if (doSuppress) {
sA11yClipboardCopySuppressionStartTime = ::GetTickCount();
+ AccessibleWrap::SuppressHandlerA11yForClipboardCopy();
}
}
/* static */
bool Compatibility::IsA11ySuppressedForClipboardCopy() {
- constexpr DWORD kSuppressTimeout = 1000; // ms
+ constexpr DWORD kSuppressTimeout = 1500; // ms
if (!sA11yClipboardCopySuppressionStartTime) {
return false;
}
diff --git a/accessible/windows/msaa/MsaaAccessible.cpp b/accessible/windows/msaa/MsaaAccessible.cpp
index 71d6c3bfa1b29..60635968f9398 100644
--- a/accessible/windows/msaa/MsaaAccessible.cpp
+++ b/accessible/windows/msaa/MsaaAccessible.cpp
@@ -11,6 +11,7 @@
#include "ia2AccessibleTable.h"
#include "ia2AccessibleTableCell.h"
#include "mozilla/a11y/AccessibleWrap.h"
+#include "mozilla/a11y/Compatibility.h"
#include "mozilla/a11y/DocAccessibleParent.h"
#include "mozilla/dom/BrowserBridgeChild.h"
#include "mozilla/dom/BrowserBridgeParent.h"
@@ -920,6 +921,15 @@ MsaaAccessible::get_accChildCount(long __RPC_FAR* pcountChildren) {
if (!mAcc) return CO_E_OBJNOTCONNECTED;
+ if (Compatibility::IsA11ySuppressedForClipboardCopy() && mAcc->IsRoot()) {
+ // Bug 1798098: Windows Suggested Actions (introduced in Windows 11 22H2)
+ // might walk the entire a11y tree using UIA whenever anything is copied to
+ // the clipboard. This causes an unacceptable hang, particularly when the
+ // cache is disabled. We prevent this tree walk by returning a 0 child count
+ // for the root window, from which Windows might walk.
+ return S_OK;
+ }
+
if (nsAccUtils::MustPrune(mAcc)) return S_OK;
*pcountChildren = mAcc->ChildCount();
--
GitLab