tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 0629edfc297d29a276231f96618eee0c1511b4f7
parent 24e6864b25e87f383ecf373dd5af51f5d7b6df50
Author: Eitan Isaacson <eitan@monotonous.org>
Date:   Thu, 20 Nov 2025 20:05:22 +0000

Bug 1998242 - P5: Perform subtree name calculation in RemoteAccessible::Name. r=morgan

Also make nsTextEquivUtils::GetNameFromSubtree remote friendly.

Depends on D271312

Differential Revision: https://phabricator.services.mozilla.com/D271313

Diffstat:
Maccessible/atk/AccessibleWrap.cpp | 2++
Maccessible/base/CacheConstants.h | 4++++
Maccessible/base/nsTextEquivUtils.cpp | 46+++++++++++++++++++++++++++++-----------------
Maccessible/base/nsTextEquivUtils.h | 2+-
Maccessible/generic/LocalAccessible.cpp | 17++++++++++++++++-
Maccessible/ipc/RemoteAccessible.cpp | 32+++++++++++++++++++++++++-------
Maccessible/mac/mozAccessible.mm | 3+++
Maccessible/tests/browser/caching_granularity/browser_name_and_description_domain.js | 4++--
8 files changed, 82 insertions(+), 28 deletions(-)

diff --git a/accessible/atk/AccessibleWrap.cpp b/accessible/atk/AccessibleWrap.cpp @@ -964,6 +964,8 @@ void a11y::PlatformEvent(Accessible* aTarget, uint32_t aEventType) { g_signal_emit_by_name(wrapper, "text-attributes-changed"); break; case nsIAccessibleEvent::EVENT_NAME_CHANGE: { + // Don't want to passively activate the cache because a name changed. + CacheDomainActivationBlocker cacheBlocker; nsAutoString newName; aTarget->Name(newName); MaybeFireNameChange(wrapper, newName); diff --git a/accessible/base/CacheConstants.h b/accessible/base/CacheConstants.h @@ -160,6 +160,8 @@ class CacheKey { // CSS position; e.g. fixed. static constexpr nsStaticAtom* CssPosition = nsGkAtoms::position; // nsString, CacheDomain::NameAndDescription + static constexpr nsStaticAtom* CssAltContent = nsGkAtoms::content; + // nsString, CacheDomain::NameAndDescription static constexpr nsStaticAtom* Description = nsGkAtoms::description; // EDescriptionValueFlag, CacheDomain::NameAndDescription // Returned by Accessible::Description. @@ -268,6 +270,8 @@ class CacheKey { // The textual value returned by Accessible::Value (as opposed to // the numeric value returned by Accessible::CurValue). static constexpr nsStaticAtom* TextValue = nsGkAtoms::aria_valuetext; + // nsString, CacheDomain::NameAndDescription + static constexpr nsStaticAtom* Tooltip = nsGkAtoms::tooltip; // gfx::Matrix4x4, CacheDomain::TransformMatrix static constexpr nsStaticAtom* TransformMatrix = nsGkAtoms::transform; // int32_t, CacheDomain::Value diff --git a/accessible/base/nsTextEquivUtils.cpp b/accessible/base/nsTextEquivUtils.cpp @@ -57,8 +57,8 @@ bool nsTextEquivUtils::HasNameRule(const Accessible* aAccessible, return (rule & aRule) == aRule; } -nsresult nsTextEquivUtils::GetNameFromSubtree( - const LocalAccessible* aAccessible, nsAString& aName) { +nsresult nsTextEquivUtils::GetNameFromSubtree(const Accessible* aAccessible, + nsAString& aName) { aName.Truncate(); if (GetReferencedAccs().Contains(aAccessible)) { @@ -71,24 +71,18 @@ nsresult nsTextEquivUtils::GetNameFromSubtree( } GetReferencedAccs().Insert(aAccessible); - if (nsIContent* content = aAccessible->GetContent()) { - AssociatedElementsIterator iter(aAccessible->Document(), content, - nsGkAtoms::aria_actions); - while (Accessible* actionTarget = iter.Next()) { - // aria-action targets are excluded from name calculation, so consider any - // of these targets as "referenced" for our purposes. - GetReferencedAccs().Insert(actionTarget); - } + Relation customActions(aAccessible->RelationByType(RelationType::ACTION)); + while (Accessible* target = customActions.Next()) { + // aria-action targets are excluded from name calculation, so consider any + // of these targets as "referenced" for our purposes. + GetReferencedAccs().Insert(target); } if (HasNameRule(aAccessible, eNameFromSubtreeRule)) { - // XXX: is it necessary to care the accessible is not a document? - if (aAccessible->IsContent()) { - nsAutoString name; - AppendFromAccessibleChildren(aAccessible, &name); - name.CompressWhitespace(); - if (!nsCoreUtils::IsWhitespaceString(name)) aName = name; - } + nsAutoString name; + AppendFromAccessibleChildren(aAccessible, &name); + name.CompressWhitespace(); + if (!nsCoreUtils::IsWhitespaceString(name)) aName = name; } // Once the text alternative computation is complete (i.e., once we've @@ -280,6 +274,24 @@ nsresult nsTextEquivUtils::AppendFromAccessible(Accessible* aAccessible, } } } + } else if (aAccessible->IsRemote()) { + if (aAccessible->IsText()) { + // Leafs should have their text appended with no spacing. + nsAutoString name; + aAccessible->Name(name); + aString->Append(name); + return NS_OK; + } + if (RefPtr<nsAtom>(aAccessible->DisplayStyle()) == nsGkAtoms::block || + aAccessible->IsHTMLListItem() || aAccessible->IsTableRow() || + aAccessible->IsTableCell()) { + // Similar to local case above, we need to add spaces around block level + // accessibles. + isHTMLBlock = true; + if (!aString->IsEmpty()) { + aString->Append(char16_t(' ')); + } + } } bool isEmptyTextEquiv = true; diff --git a/accessible/base/nsTextEquivUtils.h b/accessible/base/nsTextEquivUtils.h @@ -67,7 +67,7 @@ class nsTextEquivUtils { * @param aAccessible [in] the given accessible * @param aName [out] accessible name */ - static nsresult GetNameFromSubtree(const LocalAccessible* aAccessible, + static nsresult GetNameFromSubtree(const Accessible* aAccessible, nsAString& aName); /** diff --git a/accessible/generic/LocalAccessible.cpp b/accessible/generic/LocalAccessible.cpp @@ -3392,7 +3392,7 @@ already_AddRefed<AccAttributes> LocalAccessible::BundleFieldsForCache( // always their text. Text gets handled below. if (aCacheDomain & CacheDomain::NameAndDescription && !IsText()) { nsString name; - int32_t nameFlag = Name(name); + int32_t nameFlag = DirectName(name); if (nameFlag != eNameOK) { fields->SetAttribute(CacheKey::NameValueFlag, nameFlag); } else if (IsUpdatePush(CacheDomain::NameAndDescription)) { @@ -3417,6 +3417,21 @@ already_AddRefed<AccAttributes> LocalAccessible::BundleFieldsForCache( fields->SetAttribute(CacheKey::Name, DeleteEntry()); } + nsString tooltip; + if (Tooltip(tooltip)) { + fields->SetAttribute(CacheKey::Tooltip, std::move(tooltip)); + } else if (IsUpdatePush(CacheDomain::NameAndDescription)) { + fields->SetAttribute(CacheKey::Tooltip, DeleteEntry()); + } + + nsString cssAltContent; + if (auto cssAlt = CssAltContent(mContent)) { + cssAlt.AppendToString(cssAltContent); + fields->SetAttribute(CacheKey::CssAltContent, std::move(cssAltContent)); + } else if (IsUpdatePush(CacheDomain::NameAndDescription)) { + fields->SetAttribute(CacheKey::CssAltContent, DeleteEntry()); + } + nsString description; int32_t descFlag = Description(description); if (!description.IsEmpty()) { diff --git a/accessible/ipc/RemoteAccessible.cpp b/accessible/ipc/RemoteAccessible.cpp @@ -231,26 +231,44 @@ ENameValueFlag RemoteAccessible::Name(nsString& aName) const { return eNameOK; } - ENameValueFlag nameFlag = eNameOK; if (mCachedFields) { if (IsText()) { mCachedFields->GetAttribute(CacheKey::Text, aName); return eNameOK; } - auto cachedNameFlag = - mCachedFields->GetAttribute<int32_t>(CacheKey::NameValueFlag); - if (cachedNameFlag) { - nameFlag = static_cast<ENameValueFlag>(*cachedNameFlag); - } + if (mCachedFields->GetAttribute(CacheKey::Name, aName)) { + // Bug 1998255 - Once we tackle computing label calculation in the parent + // process, we can remove the cached name flag. + auto cachedNameFlag = + mCachedFields->GetAttribute<int32_t>(CacheKey::NameValueFlag); + ENameValueFlag nameFlag = + cachedNameFlag ? static_cast<ENameValueFlag>(*cachedNameFlag) + : eNameOK; + VERIFY_CACHE(CacheDomain::NameAndDescription); return nameFlag; } + + nsTextEquivUtils::GetNameFromSubtree(this, aName); + if (!aName.IsEmpty()) { + return eNameFromSubtree; + } + + if (mCachedFields->GetAttribute(CacheKey::Tooltip, aName)) { + VERIFY_CACHE(CacheDomain::NameAndDescription); + return eNameFromTooltip; + } + + if (mCachedFields->GetAttribute(CacheKey::CssAltContent, aName)) { + VERIFY_CACHE(CacheDomain::NameAndDescription); + return eNameOK; + } } MOZ_ASSERT(aName.IsEmpty()); aName.SetIsVoid(true); - return nameFlag; + return eNameOK; } EDescriptionValueFlag RemoteAccessible::Description( diff --git a/accessible/mac/mozAccessible.mm b/accessible/mac/mozAccessible.mm @@ -493,6 +493,7 @@ static bool ProvidesTitle(const Accessible* aAccessible, nsString& aName) { switch (aAccessible->Role()) { case roles::PAGETAB: case roles::COMBOBOX_OPTION: + case roles::OPTION: case roles::PARENT_MENUITEM: case roles::MENUITEM: // These roles always supply a title. @@ -1130,6 +1131,8 @@ static bool ProvidesTitle(const Accessible* aAccessible, nsString& aName) { mIsLiveRegion = false; break; case nsIAccessibleEvent::EVENT_NAME_CHANGE: { + // Don't want to passively activate the cache because a name changed. + CacheDomainActivationBlocker cacheBlocker; nsAutoString nameNotUsed; if (ProvidesTitle(mGeckoAccessible, nameNotUsed)) { [self moxPostNotification:NSAccessibilityTitleChangedNotification]; diff --git a/accessible/tests/browser/caching_granularity/browser_name_and_description_domain.js b/accessible/tests/browser/caching_granularity/browser_name_and_description_domain.js @@ -55,10 +55,10 @@ addAccessibleTask( // CacheKey::NameValueFlag, CacheDomain::NameAndDescription addAccessibleTask( - `<h3 id="test"><p>test</p></h3>`, + `<h3 id="test" aria-label="test me"><p>test</p></h3>`, async function (browser, docAcc) { let acc = findAccessibleChildByID(docAcc, "test"); - verifyAttributeCachedNoRetry(acc, "explicit-name"); + verifyAttributeCachedNoRetry(acc, "name"); }, { topLevel: true,