commit 6cd265bc1128577ba1786a441b3c0fc7f1daa555
parent 8e7405337dbf0b0bf945420746b2afc6efb566ee
Author: Eitan Isaacson <eitan@monotonous.org>
Date: Wed, 3 Dec 2025 19:02:56 +0000
Bug 2002339 - Only store text attributes when they differ from parent. r=Jamie
Differential Revision: https://phabricator.services.mozilla.com/D274209
Diffstat:
6 files changed, 84 insertions(+), 36 deletions(-)
diff --git a/accessible/base/AccAttributes.cpp b/accessible/base/AccAttributes.cpp
@@ -133,6 +133,29 @@ void AccAttributes::Update(AccAttributes* aOther) {
}
}
+void AccAttributes::RemoveIdentical(const AccAttributes* aOther) {
+ for (auto iter = mData.Iter(); !iter.Done(); iter.Next()) {
+ if (const auto otherEntry = aOther->mData.Lookup(iter.Key())) {
+ if (iter.Data().is<UniquePtr<nsString>>()) {
+ // Because we store nsString in a UniquePtr, we must handle it specially
+ // so we compare the string and not the pointer.
+ if (!otherEntry->is<UniquePtr<nsString>>()) {
+ continue;
+ }
+ const auto& thisStr = iter.Data().as<UniquePtr<nsString>>();
+ const auto& otherStr = otherEntry->as<UniquePtr<nsString>>();
+ if (*thisStr != *otherStr) {
+ continue;
+ }
+ } else if (iter.Data() != otherEntry.Data()) {
+ continue;
+ }
+
+ iter.Remove();
+ }
+ }
+}
+
bool AccAttributes::Equal(const AccAttributes* aOther) const {
if (Count() != aOther->Count()) {
return false;
@@ -160,8 +183,12 @@ bool AccAttributes::Equal(const AccAttributes* aOther) const {
return true;
}
-void AccAttributes::CopyTo(AccAttributes* aDest) const {
+void AccAttributes::CopyTo(AccAttributes* aDest, bool aOnlyMissing) const {
for (auto iter = mData.ConstIter(); !iter.Done(); iter.Next()) {
+ if (aOnlyMissing && aDest->HasAttribute(iter.Key())) {
+ continue;
+ }
+
iter.Data().match(
[&iter, &aDest](const bool& val) {
aDest->mData.InsertOrUpdate(iter.Key(), AsVariant(val));
diff --git a/accessible/base/AccAttributes.h b/accessible/base/AccAttributes.h
@@ -223,6 +223,9 @@ class AccAttributes {
// will be emptied.
void Update(AccAttributes* aOther);
+ // Remove all entries that are identical to the supplied AccAttributes.
+ void RemoveIdentical(const AccAttributes* aOther);
+
/**
* Return true if all the attributes in this instance are equal to all the
* attributes in another instance.
@@ -235,8 +238,10 @@ class AccAttributes {
* cached attributes without modifying the cache. It can only copy simple
* value types; e.g. it can't copy array values. Attempting to copy an
* AccAttributes with uncopyable values will cause an assertion.
+ * If aOnlyMissing is true, don't copy entries if destination already has
+ * a given key.
*/
- void CopyTo(AccAttributes* aDest) const;
+ void CopyTo(AccAttributes* aDest, bool aOnlyMissing = false) const;
// An entry class for our iterator.
class Entry {
diff --git a/accessible/base/TextLeafRange.cpp b/accessible/base/TextLeafRange.cpp
@@ -2014,14 +2014,10 @@ already_AddRefed<AccAttributes> TextLeafPoint::GetTextAttributes(
if (mAcc->IsLocal()) {
attrs = GetTextAttributesLocalAcc(aIncludeDefaults);
} else {
- attrs = new AccAttributes();
if (aIncludeDefaults) {
- Accessible* parent = mAcc->Parent();
- if (parent && parent->IsRemote() && parent->IsHyperText()) {
- if (auto defAttrs = parent->AsRemote()->GetCachedTextAttributes()) {
- defAttrs->CopyTo(attrs);
- }
- }
+ attrs = mAcc->AsRemote()->DefaultTextAttributes();
+ } else {
+ attrs = new AccAttributes();
}
if (auto thisAttrs = mAcc->AsRemote()->GetCachedTextAttributes()) {
thisAttrs->CopyTo(attrs);
@@ -2036,11 +2032,9 @@ TextLeafPoint TextLeafPoint::FindTextAttrsStart(nsDirection aDirection,
if (mIsEndOfLineInsertionPoint) {
return AdjustEndOfLine().FindTextAttrsStart(aDirection, aIncludeOrigin);
}
- const bool isRemote = mAcc->IsRemote();
RefPtr<const AccAttributes> lastAttrs;
if (mAcc->IsText()) {
- lastAttrs = isRemote ? mAcc->AsRemote()->GetCachedTextAttributes()
- : GetTextAttributesLocalAcc();
+ lastAttrs = GetTextAttributes();
}
if (aIncludeOrigin && aDirection == eDirNext && mOffset == 0) {
if (!mAcc->IsText()) {
@@ -2054,11 +2048,7 @@ TextLeafPoint TextLeafPoint::FindTextAttrsStart(nsDirection aDirection,
if (!point.mAcc || !point.mAcc->IsText()) {
return *this;
}
- // For RemoteAccessible, we can get attributes from the cache without any
- // calculation or copying.
- RefPtr<const AccAttributes> attrs =
- isRemote ? point.mAcc->AsRemote()->GetCachedTextAttributes()
- : point.GetTextAttributesLocalAcc();
+ RefPtr<const AccAttributes> attrs = point.GetTextAttributes();
if (attrs && lastAttrs && !attrs->Equal(lastAttrs)) {
return *this;
}
@@ -2100,10 +2090,9 @@ TextLeafPoint TextLeafPoint::FindTextAttrsStart(nsDirection aDirection,
if (!point.mAcc || !point.mAcc->IsText()) {
break;
}
- RefPtr<const AccAttributes> attrs =
- isRemote ? point.mAcc->AsRemote()->GetCachedTextAttributes()
- : point.GetTextAttributesLocalAcc();
- if (!lastAttrs || (attrs && !attrs->Equal(lastAttrs))) {
+ RefPtr<const AccAttributes> attrs = point.GetTextAttributes();
+ if (((!lastAttrs || !attrs) && attrs != lastAttrs) ||
+ (attrs && !attrs->Equal(lastAttrs))) {
// The attributes change here. If we're moving forward, we want to return
// this point.
if (aDirection == eDirNext) {
diff --git a/accessible/generic/LocalAccessible.cpp b/accessible/generic/LocalAccessible.cpp
@@ -3707,12 +3707,29 @@ already_AddRefed<AccAttributes> LocalAccessible::BundleFieldsForCache(
TextLeafPoint point(this, 0);
RefPtr<AccAttributes> attrs = point.GetTextAttributesLocalAcc(
/* aIncludeDefaults */ false);
- fields->SetAttribute(CacheKey::TextAttributes, std::move(attrs));
+ if (attrs->Count()) {
+ fields->SetAttribute(CacheKey::TextAttributes, std::move(attrs));
+ } else if (IsUpdatePush(CacheDomain::Text)) {
+ fields->SetAttribute(CacheKey::TextAttributes, DeleteEntry());
+ }
}
}
if (HyperTextAccessible* ht = AsHyperText()) {
RefPtr<AccAttributes> attrs = ht->DefaultTextAttributes();
- fields->SetAttribute(CacheKey::TextAttributes, std::move(attrs));
+ LocalAccessible* parent = LocalParent();
+ if (HyperTextAccessible* htParent =
+ parent ? parent->AsHyperText() : nullptr) {
+ if (RefPtr<AccAttributes> parentAttrs =
+ htParent->DefaultTextAttributes()) {
+ // Discard any entries that our parent already has.
+ attrs->RemoveIdentical(parentAttrs);
+ }
+ }
+ if (attrs->Count()) {
+ fields->SetAttribute(CacheKey::TextAttributes, std::move(attrs));
+ } else if (IsUpdatePush(CacheDomain::Text)) {
+ fields->SetAttribute(CacheKey::TextAttributes, DeleteEntry());
+ }
} else if (!IsText()) {
// Language is normally cached in text attributes, but Accessibles that
// aren't HyperText or Text (e.g. <img>, <input type="radio">) don't have
diff --git a/accessible/ipc/RemoteAccessible.cpp b/accessible/ipc/RemoteAccessible.cpp
@@ -1713,11 +1713,23 @@ already_AddRefed<AccAttributes> RemoteAccessible::DefaultTextAttributes() {
if (RequestDomainsIfInactive(CacheDomain::Text)) {
return nullptr;
}
- RefPtr<const AccAttributes> attrs = GetCachedTextAttributes();
+
RefPtr<AccAttributes> result = new AccAttributes();
- if (attrs) {
- attrs->CopyTo(result);
+ for (RemoteAccessible* parent = this; parent;
+ parent = parent->RemoteParent()) {
+ if (!parent->IsHyperText()) {
+ // We are only interested in hypertext nodes for defaults, not in text
+ // leafs or non hypertext nodes.
+ continue;
+ }
+
+ if (RefPtr<const AccAttributes> parentAttrs =
+ parent->GetCachedTextAttributes()) {
+ // Update our text attributes with any parent entries we don't have.
+ parentAttrs->CopyTo(result, true);
+ }
}
+
return result.forget();
}
@@ -2622,15 +2634,13 @@ void RemoteAccessible::Language(nsAString& aLocale) {
}
if (IsHyperText() || IsText()) {
- if (auto attrs = GetCachedTextAttributes()) {
- attrs->GetAttribute(nsGkAtoms::language, aLocale);
- }
- if (IsText() && aLocale.IsEmpty()) {
- // If a leaf has the same language as its parent HyperTextAccessible, it
- // won't be cached in the leaf's text attributes. Check the parent.
- if (RemoteAccessible* parent = RemoteParent()) {
- if (auto attrs = parent->GetCachedTextAttributes()) {
- attrs->GetAttribute(nsGkAtoms::language, aLocale);
+ for (RemoteAccessible* parent = this; parent;
+ parent = parent->RemoteParent()) {
+ // Climb up the tree to find where the nearest language attribute is.
+ if (RefPtr<const AccAttributes> attrs =
+ parent->GetCachedTextAttributes()) {
+ if (attrs->GetAttribute(nsGkAtoms::language, aLocale)) {
+ return;
}
}
}
diff --git a/accessible/tests/browser/caching_granularity/browser_text_domain.js b/accessible/tests/browser/caching_granularity/browser_text_domain.js
@@ -41,7 +41,7 @@ addAccessibleTask(
// CacheKey::TextAttributes, CacheDomain::Text
addAccessibleTask(
- `<div id="test">test</div>`,
+ `<div id="test"><strong>test</strong> me</div>`,
async function (browser, docAcc) {
let acc = findAccessibleChildByID(docAcc, "test").firstChild;
await testCachingPerPlatform(acc, "style", () => {
