From 173368bd5eaa640efc221f4ffca10dbac4cb9235 Mon Sep 17 00:00:00 2001 From: sideshowbarker Date: Thu, 19 Dec 2024 23:21:48 +0900 Subject: [PATCH] LibWeb: Allow accessible-name computation to skip role-attribute lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per https://w3c.github.io/aria/#document-handling_author-errors_roles, determining whether to ignore certain specified landmark roles requires first determining whether the element for which the role is specified has an accessible name. But if we then try to retrieve a role for such elements, we end up calling right back into the accessible-name computation code — which would cause the calls to loop infinitely. So to avoid that — and to have handling for any other future cases the spec may introduce of such recursive calls that will loop indefinitely — this change introduces a parameter that callers can pass to cause role-attribute lookup to be skipped during accessible-name computation. --- Libraries/LibWeb/DOM/Node.cpp | 18 +++++++++++++----- Libraries/LibWeb/DOM/Node.h | 9 +++++++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/Libraries/LibWeb/DOM/Node.cpp b/Libraries/LibWeb/DOM/Node.cpp index 40718469da4..49ce0afe3c8 100644 --- a/Libraries/LibWeb/DOM/Node.cpp +++ b/Libraries/LibWeb/DOM/Node.cpp @@ -2347,7 +2347,7 @@ void Node::build_accessibility_tree(AccessibilityTreeNode& parent) } // https://www.w3.org/TR/accname-1.2/#mapping_additional_nd_te -ErrorOr Node::name_or_description(NameOrDescription target, Document const& document, HashTable& visited_nodes, IsDescendant is_descendant) const +ErrorOr Node::name_or_description(NameOrDescription target, Document const& document, HashTable& visited_nodes, IsDescendant is_descendant, ShouldComputeRole should_compute_role) const { // The text alternative for a given element is computed as follows: // 1. Set the root node to the given element, the current node to the root node, and the total accumulated text to the @@ -2359,7 +2359,15 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con if (is_element()) { auto const* element = static_cast(this); - auto role = element->role_from_role_attribute_value(); + Optional role = OptionalNone {}; + // Per https://w3c.github.io/aria/#document-handling_author-errors_roles, determining whether to ignore certain + // specified landmark roles requires first determining, in the ARIAMixin code, whether the element for which the + // role is specified has an accessible name — that is, calling into this name_or_description code. But if we + // then try to retrieve a role for such elements here, that’d then end up calling right back into this + // name_or_description code — which would cause the calls to loop infinitely. So to avoid that, the caller + // in the ARIAMixin code can pass the shouldComputeRole parameter to indicate we must skip the role lookup. + if (should_compute_role == ShouldComputeRole::Yes) + role = element->role_from_role_attribute_value(); // Per https://w3c.github.io/html-aam/#el-aside and https://w3c.github.io/html-aam/#el-section, computing a // default role for an aside element or section element requires first computing its accessible name — that is, // calling into this name_or_description code. But if we then try to determine a default role for the aside @@ -2682,7 +2690,7 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con current_node = child_node; // b. Compute the text alternative of the current node beginning with step 2. Set the result to that text alternative. - auto result = MUST(current_node->name_or_description(target, document, visited_nodes, IsDescendant::Yes)); + auto result = MUST(current_node->name_or_description(target, document, visited_nodes, IsDescendant::Yes, should_compute_role)); // J. Append a space character and the result of each step above to the total accumulated text. // AD-HOC: Doing the space-adding here is in a different order from what the spec states. @@ -2752,11 +2760,11 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con } // https://www.w3.org/TR/accname-1.2/#mapping_additional_nd_name -ErrorOr Node::accessible_name(Document const& document) const +ErrorOr Node::accessible_name(Document const& document, ShouldComputeRole should_compute_role) const { HashTable visited_nodes; // User agents MUST compute an accessible name using the rules outlined below in the section titled Accessible Name and Description Computation. - return name_or_description(NameOrDescription::Name, document, visited_nodes); + return name_or_description(NameOrDescription::Name, document, visited_nodes, IsDescendant::No, should_compute_role); } // https://www.w3.org/TR/accname-1.2/#mapping_additional_nd_description diff --git a/Libraries/LibWeb/DOM/Node.h b/Libraries/LibWeb/DOM/Node.h index c2b6c796eca..e39943ca03d 100644 --- a/Libraries/LibWeb/DOM/Node.h +++ b/Libraries/LibWeb/DOM/Node.h @@ -58,6 +58,11 @@ enum class IsDescendant { Yes, }; +enum class ShouldComputeRole { + No, + Yes, +}; + #define ENUMERATE_STYLE_INVALIDATION_REASONS(X) \ X(ActiveElementChange) \ X(AdoptedStyleSheetsList) \ @@ -732,7 +737,7 @@ public: return false; } - ErrorOr accessible_name(Document const&) const; + ErrorOr accessible_name(Document const&, ShouldComputeRole = ShouldComputeRole::Yes) const; ErrorOr accessible_description(Document const&) const; Optional locate_a_namespace(Optional const& prefix) const; @@ -765,7 +770,7 @@ protected: void build_accessibility_tree(AccessibilityTreeNode& parent); - ErrorOr name_or_description(NameOrDescription, Document const&, HashTable&, IsDescendant = IsDescendant::No) const; + ErrorOr name_or_description(NameOrDescription, Document const&, HashTable&, IsDescendant = IsDescendant::No, ShouldComputeRole = ShouldComputeRole::Yes) const; private: void queue_tree_mutation_record(Vector> added_nodes, Vector> removed_nodes, Node* previous_sibling, Node* next_sibling);