From 41c0dbb5ad1fb4e7dab4d400952916c318be0647 Mon Sep 17 00:00:00 2001 From: Eitan Isaacson Date: Tue, 12 Mar 2024 20:32:16 +0000 Subject: [PATCH] Bug 1883996 - P2: Add ariaActiveDescendantElement to ARIAMixin and implement. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D203850 --- dom/base/Element.cpp | 13 ++++++ dom/base/Element.h | 11 +++++ dom/html/ElementInternals.cpp | 44 +++++++++++++++++++ dom/html/ElementInternals.h | 26 +++++++++++ dom/webidl/ARIAMixin.webidl | 3 ++ dom/xul/nsXULElement.cpp | 4 ++ .../ElementInternals-accessibility.html.ini | 3 -- .../AriaMixin-element-attributes.html.ini | 6 --- ...a-element-reflection-disconnected.html.ini | 3 -- .../html/dom/aria-element-reflection.html.ini | 33 -------------- .../meta/wai-aria/idlharness.window.js.ini | 6 --- 11 files changed, 101 insertions(+), 51 deletions(-) diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 2f7b9930acaf..b6735d919d34 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -2787,6 +2787,12 @@ bool Element::ParseAttribute(int32_t aNamespaceID, nsAtom* aAttribute, return true; } + if (aAttribute == nsGkAtoms::aria_activedescendant) { + // String in aria-activedescendant is an id, so store as an atom. + aResult.ParseAtom(aValue); + return true; + } + if (aAttribute == nsGkAtoms::id) { // Store id as an atom. id="" means that the element has no id, // not that it has an emptystring as the id. @@ -2841,6 +2847,8 @@ void Element::AfterSetAttr(int32_t aNamespaceID, nsAtom* aName, if (ShadowRoot* shadow = GetParent()->GetShadowRoot()) { shadow->MaybeReassignContent(*this); } + } else if (aName == nsGkAtoms::aria_activedescendant) { + ClearExplicitlySetAttrElement(aName); } } } @@ -2892,6 +2900,11 @@ void Element::OnAttrSetButNotChanged(int32_t aNamespaceID, nsAtom* aName, ElementCallbackType::eAttributeChanged, this, args, definition); } } + + if (aNamespaceID == kNameSpaceID_None && + aName == nsGkAtoms::aria_activedescendant) { + ClearExplicitlySetAttrElement(aName); + } } EventListenerManager* Element::GetEventListenerManagerForAttr(nsAtom* aAttrName, diff --git a/dom/base/Element.h b/dom/base/Element.h index 53515b3225af..40a8052aef75 100644 --- a/dom/base/Element.h +++ b/dom/base/Element.h @@ -243,6 +243,15 @@ class Grid; SetOrRemoveNullableStringAttr(nsGkAtoms::attr, aValue, aRv); \ } +#define REFLECT_NULLABLE_ELEMENT_ATTR(method, attr) \ + Element* Get##method() const { \ + return GetAttrAssociatedElement(nsGkAtoms::attr); \ + } \ + \ + void Set##method(Element* aElement) { \ + ExplicitlySetAttrElement(nsGkAtoms::attr, aElement); \ + } + class Element : public FragmentOrElement { public: #ifdef MOZILLA_INTERNAL_API @@ -648,6 +657,8 @@ class Element : public FragmentOrElement { REFLECT_NULLABLE_DOMSTRING_ATTR(Role, role) // AriaAttributes + REFLECT_NULLABLE_ELEMENT_ATTR(AriaActiveDescendantElement, + aria_activedescendant) REFLECT_NULLABLE_DOMSTRING_ATTR(AriaAtomic, aria_atomic) REFLECT_NULLABLE_DOMSTRING_ATTR(AriaAutoComplete, aria_autocomplete) REFLECT_NULLABLE_DOMSTRING_ATTR(AriaBrailleLabel, aria_braillelabel) diff --git a/dom/html/ElementInternals.cpp b/dom/html/ElementInternals.cpp index aaf58f818ecc..9a19744603b6 100644 --- a/dom/html/ElementInternals.cpp +++ b/dom/html/ElementInternals.cpp @@ -22,6 +22,10 @@ #include "nsDebug.h" #include "nsGenericHTMLElement.h" +#ifdef ACCESSIBILITY +# include "nsAccessibilityService.h" +#endif + namespace mozilla::dom { NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_CLASS(ElementInternals) @@ -482,4 +486,44 @@ void ElementInternals::InitializeControlNumber() { mControlNumber = mTarget->OwnerDoc()->GetNextControlNumber(); } +void ElementInternals::SetAttrElement(nsAtom* aAttr, Element* aElement) { + // Accessibility requires that no other attribute changes occur between + // AttrElementWillChange and AttrElementChanged. Scripts could cause + // this, so don't let them run here. We do this even if accessibility isn't + // running so that the JS behavior is consistent regardless of accessibility. + // Otherwise, JS might be able to use this difference to determine whether + // accessibility is running, which would be a privacy concern. + nsAutoScriptBlocker scriptBlocker; + +#ifdef ACCESSIBILITY + // If the target has this attribute defined then it overrides the defaults + // defined here in the Internals instance. In that case we don't need to + // notify the change to a11y since the attribute hasn't changed, just the + // underlying default. We can set accService to null and not notify. + nsAccessibilityService* accService = + !mTarget->HasAttr(aAttr) ? GetAccService() : nullptr; + if (accService) { + accService->NotifyAttrElementWillChange(mTarget, aAttr); + } +#endif + + if (aElement) { + mAttrElements.InsertOrUpdate(aAttr, do_GetWeakReference(aElement)); + } else { + mAttrElements.Remove(aAttr); + } + +#ifdef ACCESSIBILITY + if (accService) { + accService->NotifyAttrElementChanged(mTarget, aAttr); + } +#endif +} + +Element* ElementInternals::GetAttrElement(nsAtom* aAttr) const { + nsWeakPtr weakAttrEl = mAttrElements.Get(aAttr); + nsCOMPtr attrEl = do_QueryReferent(weakAttrEl); + return attrEl; +} + } // namespace mozilla::dom diff --git a/dom/html/ElementInternals.h b/dom/html/ElementInternals.h index ca34167ae69e..4bbf5c96f383 100644 --- a/dom/html/ElementInternals.h +++ b/dom/html/ElementInternals.h @@ -27,6 +27,13 @@ aResult = ErrorResult(SetAttr(nsGkAtoms::attr, aValue)); \ } +#define ARIA_REFLECT_ATTR_ELEMENT(method, attr) \ + Element* Get##method() const { return GetAttrElement(nsGkAtoms::attr); } \ + \ + void Set##method(Element* aElement) { \ + SetAttrElement(nsGkAtoms::attr, aElement); \ + } + class nsINodeList; class nsGenericHTMLElement; @@ -119,6 +126,7 @@ class ElementInternals final : public nsIFormControl, ARIA_REFLECT_ATTR(Role, role) // AriaAttributes + ARIA_REFLECT_ATTR_ELEMENT(AriaActiveDescendantElement, aria_activedescendant) ARIA_REFLECT_ATTR(AriaAtomic, aria_atomic) ARIA_REFLECT_ATTR(AriaAutoComplete, aria_autocomplete) ARIA_REFLECT_ATTR(AriaBusy, aria_busy) @@ -174,6 +182,18 @@ class ElementInternals final : public nsIFormControl, private: ~ElementInternals() = default; + /** + * Gets the attribute element for the given attribute. + * https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#explicitly-set-attr-element + */ + Element* GetAttrElement(nsAtom* aAttr) const; + + /** + * Sets an attribute element for the given attribute. + * https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#explicitly-set-attr-element + */ + void SetAttrElement(nsAtom* aAttr, Element* aElement); + // It's a target element which is a custom element. RefPtr mTarget; @@ -213,6 +233,12 @@ class ElementInternals final : public nsIFormControl, // owner document. This is only set to a number for elements inserted into the // document by the parser from the network. Otherwise, it is -1. int32_t mControlNumber; + + /** + * Explicitly set attr-elements, see + * https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#explicitly-set-attr-element + */ + nsTHashMap, nsWeakPtr> mAttrElements; }; } // namespace mozilla::dom diff --git a/dom/webidl/ARIAMixin.webidl b/dom/webidl/ARIAMixin.webidl index 057be8510e90..30d6b7505169 100644 --- a/dom/webidl/ARIAMixin.webidl +++ b/dom/webidl/ARIAMixin.webidl @@ -11,6 +11,9 @@ */ interface mixin ARIAMixin { + [Pref="accessibility.ARIAReflection.enabled", CEReactions] + attribute Element? ariaActiveDescendantElement; + [Pref="accessibility.ARIAReflection.enabled", CEReactions, SetterThrows] attribute DOMString? role; diff --git a/dom/xul/nsXULElement.cpp b/dom/xul/nsXULElement.cpp index ab1d7dc03d54..5bccc15f6985 100644 --- a/dom/xul/nsXULElement.cpp +++ b/dom/xul/nsXULElement.cpp @@ -1425,6 +1425,10 @@ nsresult nsXULPrototypeElement::SetAttrAt(uint32_t aPos, // emptystring as id. mAttributes[aPos].mValue.ParseAtom(aValue); + return NS_OK; + } else if (mAttributes[aPos].mName.Equals(nsGkAtoms::aria_activedescendant)) { + mAttributes[aPos].mValue.ParseAtom(aValue); + return NS_OK; } else if (mAttributes[aPos].mName.Equals(nsGkAtoms::is)) { // Store is as atom. diff --git a/testing/web-platform/meta/custom-elements/ElementInternals-accessibility.html.ini b/testing/web-platform/meta/custom-elements/ElementInternals-accessibility.html.ini index 07ae15106a2c..86b10ffb5c88 100644 --- a/testing/web-platform/meta/custom-elements/ElementInternals-accessibility.html.ini +++ b/testing/web-platform/meta/custom-elements/ElementInternals-accessibility.html.ini @@ -1,7 +1,4 @@ [ElementInternals-accessibility.html] - [ariaActiveDescendantElement is defined in ElementInternals] - expected: FAIL - [ariaControlsElements is defined in ElementInternals] expected: FAIL diff --git a/testing/web-platform/meta/custom-elements/reactions/AriaMixin-element-attributes.html.ini b/testing/web-platform/meta/custom-elements/reactions/AriaMixin-element-attributes.html.ini index 6dbcc4164965..5ade4d563d7a 100644 --- a/testing/web-platform/meta/custom-elements/reactions/AriaMixin-element-attributes.html.ini +++ b/testing/web-platform/meta/custom-elements/reactions/AriaMixin-element-attributes.html.ini @@ -1,10 +1,4 @@ [AriaMixin-element-attributes.html] - [ariaActiveDescendantElement in Element must enqueue an attributeChanged reaction when adding aria-activedescendant content attribute] - expected: FAIL - - [ariaActiveDescendantElement in Element must enqueue an attributeChanged reaction when replacing an existing attribute] - expected: FAIL - [ariaControlsElements in Element must enqueue an attributeChanged reaction when adding aria-controls content attribute] expected: FAIL diff --git a/testing/web-platform/meta/html/dom/aria-element-reflection-disconnected.html.ini b/testing/web-platform/meta/html/dom/aria-element-reflection-disconnected.html.ini index d8f8b83412be..c7ae76500eeb 100644 --- a/testing/web-platform/meta/html/dom/aria-element-reflection-disconnected.html.ini +++ b/testing/web-platform/meta/html/dom/aria-element-reflection-disconnected.html.ini @@ -1,6 +1,3 @@ [aria-element-reflection-disconnected.html] - [Element references should stay valid when content is disconnected (single element)] - expected: FAIL - [Element references should stay valid when content is disconnected (element array)] expected: FAIL diff --git a/testing/web-platform/meta/html/dom/aria-element-reflection.html.ini b/testing/web-platform/meta/html/dom/aria-element-reflection.html.ini index e59f2ea93889..95a9b5623191 100644 --- a/testing/web-platform/meta/html/dom/aria-element-reflection.html.ini +++ b/testing/web-platform/meta/html/dom/aria-element-reflection.html.ini @@ -1,31 +1,10 @@ [aria-element-reflection.html] - [aria-activedescendant element reflection] - expected: FAIL - - [If the content attribute is set directly, the IDL attribute getter always returns the first element whose ID matches the content attribute.] - expected: FAIL - - [Setting the IDL attribute to an element which is not the first element in DOM order with its ID causes the content attribute to be an empty string] - expected: FAIL - - [Setting an element reference that crosses into a shadow tree is disallowed, but setting one that is in a shadow inclusive ancestor is allowed.] - expected: FAIL - [aria-errormessage] expected: FAIL [aria-details] expected: FAIL - [Reparenting an element into a descendant shadow scope hides the element reference.] - expected: FAIL - - [Reparenting referenced element cannot cause retargeting of reference.] - expected: FAIL - - [Element reference set in invalid scope remains intact throughout move to valid scope.] - expected: FAIL - [aria-labelledby.] expected: FAIL @@ -50,17 +29,5 @@ [Moving explicitly set elements around within the same scope, and removing from the DOM.] expected: FAIL - [Attaching element reference before it's inserted into the DOM.] - expected: FAIL - - [Cross-document references and moves.] - expected: FAIL - - [Deleting a reflected element should return null for the IDL attribute and the content attribute will be empty.] - expected: FAIL - - [Changing the ID of an element doesn't lose the reference.] - expected: FAIL - [Passing values of the wrong type should throw a TypeError] expected: FAIL diff --git a/testing/web-platform/meta/wai-aria/idlharness.window.js.ini b/testing/web-platform/meta/wai-aria/idlharness.window.js.ini index 22e3076f038c..b3c83497854f 100644 --- a/testing/web-platform/meta/wai-aria/idlharness.window.js.ini +++ b/testing/web-platform/meta/wai-aria/idlharness.window.js.ini @@ -1,7 +1,4 @@ [idlharness.window.html] - [Element interface: attribute ariaActiveDescendantElement] - expected: FAIL - [Element interface: attribute ariaControlsElements] expected: FAIL @@ -20,9 +17,6 @@ [Element interface: attribute ariaOwnsElements] expected: FAIL - [Element interface: element must inherit property "ariaActiveDescendantElement" with the proper type] - expected: FAIL - [Element interface: element must inherit property "ariaControlsElements" with the proper type] expected: FAIL