tor-browser

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

commit 4684fdbfa0261bef0c87078ca7af772a921f4fc6
parent 15919fad2fd9f25803177ede846a1ddc129f1a95
Author: Eitan Isaacson <eitan@monotonous.org>
Date:   Thu, 20 Nov 2025 20:05:23 +0000

Bug 1998255 - P4: Calculate labeled names remotely. r=morgan

This patch does several things:
1. Only sends direct names to cache, no more relation calculated ones.
2. Stop sending name flag, we now can do that remotely.
3. Remotely calculate names using label relations if there is no
   explicit name stored in the cache.

Depends on D272727

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

Diffstat:
Maccessible/base/CacheConstants.h | 3---
Maccessible/base/nsTextEquivUtils.cpp | 39+++++++++++++++++++++++++++++++++++++++
Maccessible/base/nsTextEquivUtils.h | 20+++++++++++++-------
Maccessible/generic/LocalAccessible.cpp | 19+++++++------------
Maccessible/ipc/RemoteAccessible.cpp | 86+++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------
Maccessible/tests/browser/name_and_description/browser_name_general.js | 21++++++++++++++-------
6 files changed, 131 insertions(+), 57 deletions(-)

diff --git a/accessible/base/CacheConstants.h b/accessible/base/CacheConstants.h @@ -209,9 +209,6 @@ class CacheKey { static constexpr nsStaticAtom* MinValue = nsGkAtoms::min; // nsString, CacheDomain::NameAndDescription static constexpr nsStaticAtom* Name = nsGkAtoms::name; - // ENameValueFlag, CacheDomain::NameAndDescription - // Returned by Accessible::Name. - static constexpr nsStaticAtom* NameValueFlag = nsGkAtoms::explicit_name; // double, CacheDomain::Value // The numeric value returned by Accessible::CurValue. static constexpr nsStaticAtom* NumericValue = nsGkAtoms::value; diff --git a/accessible/base/nsTextEquivUtils.cpp b/accessible/base/nsTextEquivUtils.cpp @@ -11,6 +11,7 @@ #include "AccIterator.h" #include "CssAltContent.h" #include "nsCoreUtils.h" +#include "Relation.h" #include "mozilla/dom/ChildIterator.h" #include "mozilla/dom/Text.h" @@ -96,6 +97,44 @@ nsresult nsTextEquivUtils::GetNameFromSubtree(const Accessible* aAccessible, return NS_OK; } +void nsTextEquivUtils::GetTextEquivFromAccIterable( + const Accessible* aAccessible, AccIterable* aIter, nsAString& aTextEquiv) { + if (GetReferencedAccs().Contains(aAccessible)) { + // We got here when trying to resolve a dependant label or description, + // early out. + return; + } + // Remember the initiating accessible so we know when we've returned to it. + if (GetReferencedAccs().IsEmpty()) { + sInitiatorAcc = aAccessible; + } + // This method doesn't allow self-referencing accessibles, so add the + // initiator to the referenced accs hash. + GetReferencedAccs().Insert(aAccessible); + + aTextEquiv.Truncate(); + + while (Accessible* acc = aIter->Next()) { + if (!aTextEquiv.IsEmpty()) aTextEquiv += ' '; + + if (GetReferencedAccs().Contains(acc)) { + continue; + } + GetReferencedAccs().Insert(acc); + + AppendFromAccessible(acc, &aTextEquiv); + } + + if (aAccessible == sInitiatorAcc) { + // This is the top-level initiator, clear the hash. + GetReferencedAccs().Clear(); + sInitiatorAcc = nullptr; + } else { + // This is not the top-level initiator, just remove the calling accessible. + GetReferencedAccs().Remove(aAccessible); + } +} + bool nsTextEquivUtils::GetTextEquivFromIDRefs( const LocalAccessible* aAccessible, nsAtom* aIDRefsAttr, nsAString& aTextEquiv) { diff --git a/accessible/base/nsTextEquivUtils.h b/accessible/base/nsTextEquivUtils.h @@ -16,6 +16,7 @@ class nsIContent; namespace mozilla { namespace a11y { class LocalAccessible; +class AccIterable; } } // namespace mozilla @@ -51,6 +52,7 @@ class nsTextEquivUtils { public: typedef mozilla::a11y::LocalAccessible LocalAccessible; typedef mozilla::a11y::Accessible Accessible; + typedef mozilla::a11y::AccIterable AccIterable; /** * Determines if the accessible has a given name rule. @@ -98,6 +100,10 @@ class nsTextEquivUtils { nsAtom* aIDRefsAttr, nsAString& aTextEquiv); + static void GetTextEquivFromAccIterable(const Accessible* aAccessible, + AccIterable* aIter, + nsAString& aTextEquiv); + /** * Calculates the text equivalent from the given content - and its subtree, if * allowed - and appends it to the given string. @@ -135,6 +141,13 @@ class nsTextEquivUtils { static nsresult AppendFromDOMChildren(nsIContent* aContent, nsAString* aString); + /** + * Calculates the text equivalent from the given accessible - and its subtree, + * if allowed. Then, appends the calculated text to the given string. + */ + static nsresult AppendFromAccessible(Accessible* aAccessible, + nsAString* aString); + private: /** * Iterates the given accessible's children and calculates the text equivalent @@ -144,13 +157,6 @@ class nsTextEquivUtils { nsAString* aString); /** - * Calculates the text equivalent from the given accessible - and its subtree, - * if allowed. Then, appends the calculated text to the given string. - */ - static nsresult AppendFromAccessible(Accessible* aAccessible, - nsAString* aString); - - /** * Calculates the text equivalent from the value of the given accessible. * Then, appends the calculated text to the given string. This function * implements the "Embedded Control" section of the AccName spec. diff --git a/accessible/generic/LocalAccessible.cpp b/accessible/generic/LocalAccessible.cpp @@ -2596,12 +2596,12 @@ ENameValueFlag LocalAccessible::ARIAName(nsString& aName) const { return eNameOK; } // aria-labelledby now takes precedence over aria-label - bool usedHiddenContent = nsTextEquivUtils::GetTextEquivFromIDRefs( + bool notSimpleRelation = nsTextEquivUtils::GetTextEquivFromIDRefs( this, nsGkAtoms::aria_labelledby, aName); aName.CompressWhitespace(); if (!aName.IsEmpty()) { - return usedHiddenContent ? eNameOK : eNameFromRelations; + return notSimpleRelation ? eNameOK : eNameFromRelations; } if (mContent->IsElement() && @@ -2663,15 +2663,15 @@ ENameValueFlag LocalAccessible::NativeName(nsString& aName) const { if (mContent->IsHTMLElement()) { LocalAccessible* label = nullptr; HTMLLabelIterator iter(Document(), this); - bool usedHiddenContent = false; + bool notSimpleRelation = false; while ((label = iter.Next())) { - usedHiddenContent |= nsTextEquivUtils::AppendTextEquivFromContent( + notSimpleRelation |= nsTextEquivUtils::AppendTextEquivFromContent( this, label->GetContent(), &aName); aName.CompressWhitespace(); } if (!aName.IsEmpty()) { - return usedHiddenContent ? eNameOK : eNameFromRelations; + return notSimpleRelation ? eNameOK : eNameFromRelations; } NameFromAssociatedXULLabel(mDoc, mContent, aName); @@ -3389,12 +3389,7 @@ already_AddRefed<AccAttributes> LocalAccessible::BundleFieldsForCache( // always their text. Text gets handled below. if (aCacheDomain & CacheDomain::NameAndDescription && !IsText()) { nsString name; - int32_t nameFlag = DirectName(name); - if (nameFlag != eNameOK) { - fields->SetAttribute(CacheKey::NameValueFlag, nameFlag); - } else if (IsUpdatePush(CacheDomain::NameAndDescription)) { - fields->SetAttribute(CacheKey::NameValueFlag, DeleteEntry()); - } + ENameValueFlag nameFlag = DirectName(name); if (IsTextField()) { MOZ_ASSERT(mContent); @@ -3408,7 +3403,7 @@ already_AddRefed<AccAttributes> LocalAccessible::BundleFieldsForCache( } } - if (!name.IsEmpty()) { + if (nameFlag != eNameFromRelations && !name.IsEmpty()) { fields->SetAttribute(CacheKey::Name, std::move(name)); } else if (IsUpdatePush(CacheDomain::NameAndDescription)) { fields->SetAttribute(CacheKey::Name, DeleteEntry()); diff --git a/accessible/ipc/RemoteAccessible.cpp b/accessible/ipc/RemoteAccessible.cpp @@ -226,44 +226,74 @@ void RemoteAccessible::ApplyCache(CacheUpdateType aUpdateType, ENameValueFlag RemoteAccessible::Name(nsString& aName) const { if (RequestDomainsIfInactive(CacheDomain::NameAndDescription | - CacheDomain::Text)) { + CacheDomain::Text | CacheDomain::Relations) || + !mCachedFields) { aName.SetIsVoid(true); return eNameOK; } - if (mCachedFields) { - if (IsText()) { - mCachedFields->GetAttribute(CacheKey::Text, aName); - return eNameOK; - } + if (IsText()) { + mCachedFields->GetAttribute(CacheKey::Text, aName); + return eNameOK; + } - 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; + if (mCachedFields->GetAttribute(CacheKey::Name, aName)) { + VERIFY_CACHE(CacheDomain::NameAndDescription); + return eNameOK; + } - VERIFY_CACHE(CacheDomain::NameAndDescription); - return nameFlag; - } + if (auto maybeAriaLabelIds = mCachedFields->GetAttribute<nsTArray<uint64_t>>( + nsGkAtoms::aria_labelledby)) { + RemoteAccIterator iter(*maybeAriaLabelIds, Document()); + nsTextEquivUtils::GetTextEquivFromAccIterable(this, &iter, aName); + aName.CompressWhitespace(); + } - nsTextEquivUtils::GetNameFromSubtree(this, aName); - if (!aName.IsEmpty()) { - return eNameFromSubtree; - } + if (!aName.IsEmpty()) { + return eNameFromRelations; + } - if (mCachedFields->GetAttribute(CacheKey::Tooltip, aName)) { - VERIFY_CACHE(CacheDomain::NameAndDescription); - return eNameFromTooltip; - } + if (auto accRelMapEntry = mDoc->mReverseRelations.Lookup(ID())) { + nsTArray<uint64_t> relationCandidateIds; + for (const auto& data : kRelationTypeAtoms) { + if (data.mAtom != nsGkAtoms::_for || data.mValidTag != nsGkAtoms::label) { + continue; + } - if (mCachedFields->GetAttribute(CacheKey::CssAltContent, aName)) { - VERIFY_CACHE(CacheDomain::NameAndDescription); - return eNameOK; + if (auto labelIds = accRelMapEntry.Data().Lookup(&data)) { + RemoteAccIterator iter(*labelIds, Document()); + nsTextEquivUtils::GetTextEquivFromAccIterable(this, &iter, aName); + aName.CompressWhitespace(); + } } + aName.CompressWhitespace(); + } + + if (!aName.IsEmpty()) { + return eNameFromRelations; + } + + ArrayAccIterator iter(LegendsOrCaptions()); + nsTextEquivUtils::GetTextEquivFromAccIterable(this, &iter, aName); + aName.CompressWhitespace(); + + if (!aName.IsEmpty()) { + return eNameFromRelations; + } + + 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()); diff --git a/accessible/tests/browser/name_and_description/browser_name_general.js b/accessible/tests/browser/name_and_description/browser_name_general.js @@ -225,31 +225,38 @@ addAccessibleTask( </label> `, async function testARIACoreExamples(browser, docAcc) { - function testName_(id, expected) { + function testName_(id, expected, cached) { const acc = findAccessibleChildByID(docAcc, id); + if (browser.isRemoteBrowser) { + is( + acc.cache.has("name"), + cached, + `Name should ${cached ? "" : "not "}be in cache for '${id}'` + ); + } testName(acc, expected); } // Example 1 from section 4.3.1 under 2.B. // Element1 should get its name from the text in element3. // Element2 should not get its name from element1 because that already // gets its name from another element. - testName_("el1", "hello"); - testName_("el2", null); + testName_("el1", "hello", false); + testName_("el2", null, false); // Example 2 from section 4.3.1 under 2.C. // The buttons should get their name from their labels and the links. - testName_("del_row1", "Delete Documentation.pdf"); - testName_("del_row2", "Delete HolidayLetter.pdf"); + testName_("del_row1", "Delete Documentation.pdf", true); + testName_("del_row2", "Delete HolidayLetter.pdf", true); // Example 3 from section 4.3.1 under 2.F. // Name should be own content text plus the value of the input plus // more own inner text, separated by 1 space. - testName_("chkbx", "Flash the screen 5 times"); + testName_("chkbx", "Flash the screen 5 times", false); // Example 4 from section 4.3.1 under 2.F. // Name from content should include all the child nodes, including // table cells. - testName_("input_with_html_label", "foo bar baz"); + testName_("input_with_html_label", "foo bar baz", false); }, { topLevel: true, chrome: true } );