From 349592fb9170594e0b0d0d8560e142309eec504f Mon Sep 17 00:00:00 2001 From: Eitan Isaacson Date: Thu, 10 Jun 2021 23:07:07 +0000 Subject: [PATCH] Bug 1714390 - P6: Make more attribute keys static atoms. r=Jamie Keys should be static atoms whenever possible. Differential Revision: https://phabricator.services.mozilla.com/D116787 --- accessible/android/AccessibleWrap.cpp | 7 +++---- accessible/base/AccAttributes.h | 14 -------------- accessible/generic/ARIAGridAccessible.cpp | 5 +++-- accessible/generic/HyperTextAccessible.cpp | 2 +- accessible/generic/LocalAccessible.cpp | 8 ++++---- accessible/html/HTMLTableAccessible.cpp | 5 +++-- accessible/mac/MOXMathAccessibles.mm | 4 ++-- accessible/tests/mochitest/attributes.js | 4 ++-- .../tests/mochitest/attributes/test_obj_group.html | 4 ++-- .../windows/msaa/ApplicationAccessibleWrap.cpp | 3 ++- accessible/xpcom/xpcAccessible.cpp | 4 ++-- xpcom/ds/StaticAtoms.py | 4 ++++ 12 files changed, 28 insertions(+), 36 deletions(-) diff --git a/accessible/android/AccessibleWrap.cpp b/accessible/android/AccessibleWrap.cpp index 61b33fff145a..9997166933b4 100644 --- a/accessible/android/AccessibleWrap.cpp +++ b/accessible/android/AccessibleWrap.cpp @@ -811,8 +811,8 @@ mozilla::java::GeckoBundle::LocalRef AccessibleWrap::ToBundle( GECKOBUNDLE_PUT(nodeInfo, "collectionItemInfo", collectionItemInfo); } - RefPtr attrAtom = NS_Atomize("child-item-count"_ns); - Maybe rowCount = aAttributes->GetAttribute(attrAtom); + Maybe rowCount = + aAttributes->GetAttribute(nsGkAtoms::child_item_count); if (rowCount) { GECKOBUNDLE_START(collectionInfo); GECKOBUNDLE_PUT(collectionInfo, "rowCount", @@ -820,8 +820,7 @@ mozilla::java::GeckoBundle::LocalRef AccessibleWrap::ToBundle( GECKOBUNDLE_PUT(collectionInfo, "columnCount", java::sdk::Integer::ValueOf(1)); - attrAtom = NS_Atomize("hierarchical"_ns); - if (aAttributes->HasAttribute(attrAtom)) { + if (aAttributes->HasAttribute(nsGkAtoms::tree)) { GECKOBUNDLE_PUT(collectionInfo, "isHierarchical", java::sdk::Boolean::TRUE()); } diff --git a/accessible/base/AccAttributes.h b/accessible/base/AccAttributes.h index 2ac0da72df53..3f3bd5567541 100644 --- a/accessible/base/AccAttributes.h +++ b/accessible/base/AccAttributes.h @@ -66,20 +66,6 @@ class AccAttributes { } } - // XXX: This will be removed in a later patch in the stack. This is - // just a stop-gap before we transtion to atom-only keys. - template - void SetAttribute(const nsAString& aAttrName, const T& aAttrValue) { - RefPtr attrAtom = NS_Atomize(aAttrName); - if constexpr (std::is_base_of_v>) { - mData.InsertOrUpdate(attrAtom, AsVariant(RefPtr(aAttrValue))); - } else if constexpr (std::is_base_of_v) { - mData.InsertOrUpdate(attrAtom, AsVariant(nsString(aAttrValue))); - } else { - mData.InsertOrUpdate(attrAtom, AsVariant(aAttrValue)); - } - } - template Maybe GetAttribute(nsAtom* aAttrName) { if (auto value = mData.Lookup(aAttrName)) { diff --git a/accessible/generic/ARIAGridAccessible.cpp b/accessible/generic/ARIAGridAccessible.cpp index d2021e50b45f..2e0845376d39 100644 --- a/accessible/generic/ARIAGridAccessible.cpp +++ b/accessible/generic/ARIAGridAccessible.cpp @@ -40,7 +40,7 @@ already_AddRefed ARIAGridAccessible::NativeAttributes() { RefPtr attributes = AccessibleWrap::NativeAttributes(); if (IsProbablyLayoutTable()) { - attributes->SetAttribute(u"layout-guess"_ns, true); + attributes->SetAttribute(nsGkAtoms::layout_guess, true); } return attributes.forget(); @@ -608,7 +608,8 @@ already_AddRefed ARIAGridCellAccessible::NativeAttributes() { rowIdx * colCount + colIdx); #ifdef DEBUG - attributes->SetAttribute(u"cppclass"_ns, u"ARIAGridCellAccessible"_ns); + RefPtr cppClass = NS_Atomize(u"cppclass"_ns); + attributes->SetAttribute(cppClass, u"ARIAGridCellAccessible"_ns); #endif return attributes.forget(); diff --git a/accessible/generic/HyperTextAccessible.cpp b/accessible/generic/HyperTextAccessible.cpp index 3ea0249b46f1..2294b04c1004 100644 --- a/accessible/generic/HyperTextAccessible.cpp +++ b/accessible/generic/HyperTextAccessible.cpp @@ -1411,7 +1411,7 @@ already_AddRefed HyperTextAccessible::NativeAttributes() { // instead. nsIFrame* frame = GetFrame(); if (frame && frame->IsBlockFrame()) { - attributes->SetAttribute(u"formatting"_ns, nsGkAtoms::block); + attributes->SetAttribute(nsGkAtoms::formatting, nsGkAtoms::block); } if (FocusMgr()->IsFocused(this)) { diff --git a/accessible/generic/LocalAccessible.cpp b/accessible/generic/LocalAccessible.cpp index d387692ff0b8..30a55d12e53e 100644 --- a/accessible/generic/LocalAccessible.cpp +++ b/accessible/generic/LocalAccessible.cpp @@ -1019,7 +1019,7 @@ already_AddRefed LocalAccessible::NativeAttributes() { if (HasNumericValue()) { nsAutoString valuetext; Value(valuetext); - attributes->SetAttribute(u"valuetext"_ns, valuetext); + attributes->SetAttribute(nsGkAtoms::aria_valuetext, valuetext); } // Expose checkable object attribute if the accessible has checkable state @@ -1030,7 +1030,7 @@ already_AddRefed LocalAccessible::NativeAttributes() { // Expose 'explicit-name' attribute. nsAutoString name; if (Name(name) != eNameFromSubtree && !name.IsVoid()) { - attributes->SetAttribute(u"explicit-name"_ns, true); + attributes->SetAttribute(nsGkAtoms::explicit_name, true); } // Group attributes (level/setsize/posinset) @@ -1041,12 +1041,12 @@ already_AddRefed LocalAccessible::NativeAttributes() { bool hierarchical = false; uint32_t itemCount = AccGroupInfo::TotalItemCount(this, &hierarchical); if (itemCount) { - attributes->SetAttribute(u"child-item-count"_ns, + attributes->SetAttribute(nsGkAtoms::child_item_count, static_cast(itemCount)); } if (hierarchical) { - attributes->SetAttribute(u"hierarchical"_ns, true); + attributes->SetAttribute(nsGkAtoms::tree, true); } // If the accessible doesn't have own content (such as list item bullet or diff --git a/accessible/html/HTMLTableAccessible.cpp b/accessible/html/HTMLTableAccessible.cpp index ae3dc7c8cbb2..fbf7009a8fbb 100644 --- a/accessible/html/HTMLTableAccessible.cpp +++ b/accessible/html/HTMLTableAccessible.cpp @@ -118,7 +118,8 @@ already_AddRefed HTMLTableCellAccessible::NativeAttributes() { } #ifdef DEBUG - attributes->SetAttribute(u"cppclass"_ns, u"HTMLTableCellAccessible"_ns); + RefPtr cppClass = NS_Atomize(u"cppclass"_ns); + attributes->SetAttribute(cppClass, u"HTMLTableCellAccessible"_ns); #endif return attributes.forget(); @@ -398,7 +399,7 @@ already_AddRefed HTMLTableAccessible::NativeAttributes() { } if (IsProbablyLayoutTable()) { - attributes->SetAttribute(u"layout-guess"_ns, true); + attributes->SetAttribute(nsGkAtoms::layout_guess, true); } return attributes.forget(); diff --git a/accessible/mac/MOXMathAccessibles.mm b/accessible/mac/MOXMathAccessibles.mm index 919586b28e7a..7bfe2e3e05a0 100644 --- a/accessible/mac/MOXMathAccessibles.mm +++ b/accessible/mac/MOXMathAccessibles.mm @@ -58,8 +58,8 @@ using namespace mozilla::a11y; // Per the MathML 3 spec, the latter happens iff the linethickness // attribute is of the form [zero-float][optional-unit]. In that case we // set line thickness to zero and in the other cases we set it to one. - RefPtr attrName = NS_Atomize("thickness"_ns); - if (NSString* thickness = utils::GetAccAttr(self, attrName)) { + if (NSString* thickness = + utils::GetAccAttr(self, nsGkAtoms::linethickness_)) { NSNumberFormatter* formatter = [[[NSNumberFormatter alloc] init] autorelease]; NSNumber* value = [formatter numberFromString:thickness]; diff --git a/accessible/tests/mochitest/attributes.js b/accessible/tests/mochitest/attributes.js index 2ae88b3ebf6d..f491e2820a71 100644 --- a/accessible/tests/mochitest/attributes.js +++ b/accessible/tests/mochitest/attributes.js @@ -138,9 +138,9 @@ function testGroupParentAttrs(aAccOrElmOrID, aChildItemCount, aIsHierarchical) { ); if (aIsHierarchical) { - testAttrs(aAccOrElmOrID, { hierarchical: "true" }, true); + testAttrs(aAccOrElmOrID, { tree: "true" }, true); } else { - testAbsentAttrs(aAccOrElmOrID, { hierarchical: "true" }); + testAbsentAttrs(aAccOrElmOrID, { tree: "true" }); } } diff --git a/accessible/tests/mochitest/attributes/test_obj_group.html b/accessible/tests/mochitest/attributes/test_obj_group.html index 6bfaaf7285f4..f495fa982ea2 100644 --- a/accessible/tests/mochitest/attributes/test_obj_group.html +++ b/accessible/tests/mochitest/attributes/test_obj_group.html @@ -212,8 +212,8 @@ testGroupAttrs("h5", 0, 0, 5); testGroupAttrs("h6", 0, 0, 6); testGroupAttrs("ariaHeadingNoLevel", 0, 0, 2); - // No child item counts or "hierarchical" flag for parent of headings - testAbsentAttrs("headings", {"child-item-count": "", "hierarchical": ""}); + // No child item counts or "tree" flag for parent of headings + testAbsentAttrs("headings", {"child-item-count": "", "tree": ""}); // //////////////////////////////////////////////////////////////////////// // ARIA combobox diff --git a/accessible/windows/msaa/ApplicationAccessibleWrap.cpp b/accessible/windows/msaa/ApplicationAccessibleWrap.cpp index 8a3273f98424..6a10a8bddcde 100644 --- a/accessible/windows/msaa/ApplicationAccessibleWrap.cpp +++ b/accessible/windows/msaa/ApplicationAccessibleWrap.cpp @@ -26,7 +26,8 @@ already_AddRefed ApplicationAccessibleWrap::NativeAttributes() { if (gfxInfo) { bool isD2DEnabled = false; gfxInfo->GetD2DEnabled(&isD2DEnabled); - attributes->SetAttribute(u"D2D"_ns, isD2DEnabled); + RefPtr attrName = NS_Atomize(u"D2D"_ns); + attributes->SetAttribute(attrName, isD2DEnabled); } return attributes.forget(); diff --git a/accessible/xpcom/xpcAccessible.cpp b/accessible/xpcom/xpcAccessible.cpp index 2f5802978547..98f7c3ee0af0 100644 --- a/accessible/xpcom/xpcAccessible.cpp +++ b/accessible/xpcom/xpcAccessible.cpp @@ -378,8 +378,8 @@ xpcAccessible::GetAttributes(nsIPersistentProperties** aAttributes) { proxy->Attributes(&attrs); uint32_t attrCount = attrs.Length(); for (uint32_t i = 0; i < attrCount; i++) { - attributes->SetAttribute(NS_ConvertUTF8toUTF16(attrs[i].Name()), - attrs[i].Value()); + RefPtr nameAttr = NS_Atomize(attrs[i].Name()); + attributes->SetAttribute(nameAttr, attrs[i].Value()); } } diff --git a/xpcom/ds/StaticAtoms.py b/xpcom/ds/StaticAtoms.py index 746516f1f069..50991b06d9bd 100644 --- a/xpcom/ds/StaticAtoms.py +++ b/xpcom/ds/StaticAtoms.py @@ -224,6 +224,7 @@ STATIC_ATOMS = [ Atom("child", "child"), Atom("children", "children"), Atom("childList", "childList"), + Atom("child_item_count", "child-item-count"), Atom("choose", "choose"), Atom("chromemargin", "chromemargin"), Atom("exposeToUntrustedContent", "exposeToUntrustedContent"), @@ -422,6 +423,7 @@ STATIC_ATOMS = [ Atom("events", "events"), Atom("excludeResultPrefixes", "exclude-result-prefixes"), Atom("exportparts", "exportparts"), + Atom("explicit_name", "explicit-name"), Atom("extends", "extends"), Atom("extensionElementPrefixes", "extension-element-prefixes"), Atom("face", "face"), @@ -583,6 +585,7 @@ STATIC_ATOMS = [ Atom("last", "last"), Atom("layer", "layer"), Atom("LayerActivity", "LayerActivity"), + Atom("layout_guess", "layout-guess"), Atom("leading", "leading"), Atom("leaf", "leaf"), Atom("left", "left"), @@ -1398,6 +1401,7 @@ STATIC_ATOMS = [ Atom("font_stretch", "font-stretch"), Atom("font_style", "font-style"), Atom("font_variant", "font-variant"), + Atom("formatting", "formatting"), Atom("foreignObject", "foreignObject"), Atom("fractalNoise", "fractalNoise"), Atom("fr", "fr"),