From ff18f729f0d2c6bf0a68a0987d9d7a27fc8b66ff Mon Sep 17 00:00:00 2001 From: sideshowbarker Date: Mon, 11 Nov 2024 16:21:05 +0900 Subject: [PATCH 1/2] LibWeb: Compute accessible names for hidden/hidden-but-referenced nodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change implements full support for the “A. Hidden Not Referenced” step at https://w3c.github.io/accname/#step2A in the “Accessible Name and Description Computation” spec — including handling all hidden nodes that must be ignored, as well as handling hidden nodes that, for the purposes of accessible-name computation, must not be ignored (due to having aria-labelledby/aria-describedby references from other nodes). Otherwise, without this change, not all cases of hidden nodes get ignored as expected, while cases of nodes that are hidden but that have aria-labelledby/aria-describedby references from other nodes get unexpectedly ignored. --- Libraries/LibWeb/DOM/Element.cpp | 48 ++++ Libraries/LibWeb/DOM/Element.h | 6 + Libraries/LibWeb/DOM/Node.cpp | 55 ++-- .../name/comp_hidden_not_referenced.txt | 15 ++ .../name/comp_labelledby_hidden_nodes.txt | 37 +++ .../name/comp_hidden_not_referenced.html | 92 +++++++ .../name/comp_labelledby_hidden_nodes.html | 245 ++++++++++++++++++ 7 files changed, 478 insertions(+), 20 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/wpt-import/accname/name/comp_hidden_not_referenced.txt create mode 100644 Tests/LibWeb/Text/expected/wpt-import/accname/name/comp_labelledby_hidden_nodes.txt create mode 100644 Tests/LibWeb/Text/input/wpt-import/accname/name/comp_hidden_not_referenced.html create mode 100644 Tests/LibWeb/Text/input/wpt-import/accname/name/comp_labelledby_hidden_nodes.html diff --git a/Libraries/LibWeb/DOM/Element.cpp b/Libraries/LibWeb/DOM/Element.cpp index fea2ff0449f8..395d687826af 100644 --- a/Libraries/LibWeb/DOM/Element.cpp +++ b/Libraries/LibWeb/DOM/Element.cpp @@ -1874,6 +1874,54 @@ void Element::invalidate_style_after_attribute_change(FlyString const& attribute invalidate_style(StyleInvalidationReason::ElementAttributeChange); } +bool Element::is_hidden() const +{ + if (layout_node() == nullptr) + return true; + if ((layout_node() != nullptr) && (layout_node()->computed_values().visibility() == CSS::Visibility::Hidden || layout_node()->computed_values().visibility() == CSS::Visibility::Collapse || layout_node()->computed_values().content_visibility() == CSS::ContentVisibility::Hidden)) + return true; + for (ParentNode const* self_or_ancestor = this; self_or_ancestor; self_or_ancestor = self_or_ancestor->parent_or_shadow_host()) { + if (self_or_ancestor->is_element() && static_cast(self_or_ancestor)->aria_hidden() == "true") + return true; + } + return false; +} + +bool Element::has_hidden_ancestor() const +{ + for (ParentNode const* self_or_ancestor = this; self_or_ancestor; self_or_ancestor = self_or_ancestor->parent_or_shadow_host()) { + if (self_or_ancestor->is_element() && static_cast(self_or_ancestor)->is_hidden()) + return true; + } + return false; +} + +bool Element::is_referenced() const +{ + bool is_referenced = false; + if (id().has_value()) { + this->root().for_each_in_inclusive_subtree_of_type([&](auto& element) { + auto aria_data = MUST(Web::ARIA::AriaData::build_data(element)); + if (aria_data->aria_labelled_by_or_default().contains_slow(id().value())) { + is_referenced = true; + return TraversalDecision::Break; + } + return TraversalDecision::Continue; + }); + } + return is_referenced; +} + +bool Element::has_referenced_and_hidden_ancestor() const +{ + for (auto const* ancestor = parent_or_shadow_host(); ancestor; ancestor = ancestor->parent_or_shadow_host()) { + if (ancestor->is_element()) + if (auto const* element = static_cast(ancestor); element->is_referenced() && element->is_hidden()) + return true; + } + return false; +} + // https://www.w3.org/TR/wai-aria-1.2/#tree_exclusion bool Element::exclude_from_accessibility_tree() const { diff --git a/Libraries/LibWeb/DOM/Element.h b/Libraries/LibWeb/DOM/Element.h index 94caa57ef42a..382e3b29393b 100644 --- a/Libraries/LibWeb/DOM/Element.h +++ b/Libraries/LibWeb/DOM/Element.h @@ -348,6 +348,12 @@ class Element virtual bool include_in_accessibility_tree() const override; + bool is_hidden() const; + bool has_hidden_ancestor() const; + + bool is_referenced() const; + bool has_referenced_and_hidden_ancestor() const; + void enqueue_a_custom_element_upgrade_reaction(HTML::CustomElementDefinition& custom_element_definition); void enqueue_a_custom_element_callback_reaction(FlyString const& callback_name, GC::MarkedVector arguments); diff --git a/Libraries/LibWeb/DOM/Node.cpp b/Libraries/LibWeb/DOM/Node.cpp index 051dd37c8b98..1895b5e41d55 100644 --- a/Libraries/LibWeb/DOM/Node.cpp +++ b/Libraries/LibWeb/DOM/Node.cpp @@ -2195,23 +2195,25 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con if (is_element()) { auto const* element = static_cast(this); auto role = element->role_or_default(); - bool is_referenced = false; - auto id = element->id(); - if (id.has_value()) { - this->root().for_each_in_inclusive_subtree_of_type([&](auto& element) { - auto aria_data = MUST(Web::ARIA::AriaData::build_data(element)); - if (aria_data->aria_labelled_by_or_default().contains_slow(id.value())) { - is_referenced = true; - return TraversalDecision::Break; - } - return TraversalDecision::Continue; - }); - } // 2. Compute the text alternative for the current node: - // A. If the current node is hidden and is not directly referenced by aria-labelledby or aria-describedby, nor directly referenced by a native host language text alternative element (e.g. label in HTML) or attribute, return the empty string. - // FIXME: Check for references - if (element->aria_hidden() == "true") - return String {}; + + // A. Hidden Not Referenced: If the current node is hidden and is: + // i. Not part of an aria-labelledby or aria-describedby traversal, where the node directly referenced by that + // relation was hidden. + // ii. Nor part of a native host language text alternative element (e.g. label in HTML) or attribute traversal, + // where the root of that traversal was hidden. + // Return the empty string. + // NOTE: Nodes with CSS properties display:none, visibility:hidden, visibility:collapse or + // content-visibility:hidden: They are considered hidden, as they match the guidelines "not perceivable" and + // "explicitly hidden". + // + // AD-HOC: We don’t implement this step here — because strictly implementing this would cause us to return early + // whenever encountering a node (element, actually) that “is hidden and is not directly referenced by + // aria-labelledby or aria-describedby”, without traversing down through that element’s subtree to see if it has + // (1) any descendant elements that are directly referenced and/or (2) any un-hidden nodes. So we instead (in + // substep G below) traverse upward through ancestor nodes of every text node, and check in that way to do the + // equivalent of what this step seems to have been intended to do. + // B. Otherwise: // - if computing a name, and the current node has an aria-labelledby attribute that contains at least one valid IDREF, and the current node is not already part of an aria-labelledby traversal, // process its IDREFs in the order they occur: @@ -2350,8 +2352,10 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con // with the spec requirements — and if not, then add handling for it here. } - // F. Otherwise, if the current node's role allows name from content, or if the current node is referenced by aria-labelledby, aria-describedby, or is a native host language text alternative element (e.g. label in HTML), or is a descendant of a native host language text alternative element: - if ((role.has_value() && ARIA::allows_name_from_content(role.value())) || is_referenced || is_descendant == IsDescendant::Yes) { + // F. Name From Content: Otherwise, if the current node's role allows name from content, or if the current node + // is referenced by aria-labelledby, aria-describedby, or is a native host language text alternative element + // (e.g. label in HTML), or is a descendant of a native host language text alternative element: + if ((role.has_value() && ARIA::allows_name_from_content(role.value())) || element->is_referenced() || is_descendant == IsDescendant::Yes) { // i. Set the accumulated text to the empty string. total_accumulated_text.clear(); // ii. Name From Generated Content: Check for CSS generated textual content associated with the current node and include @@ -2416,13 +2420,24 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con } // G. Text Node: Otherwise, if the current node is a Text Node, return its textual contents. - if (is_text()) { + // AD-HOC: The spec doesn’t require ascending through the parent node and ancestor nodes of every text node we + // reach — the way we’re doing there. But we implement it this way because the spec algorithm as written doesn’t + // appear to achieve what it seems to be intended to achieve. Specifically, the spec algorithm as written doesn’t + // cause traversal through element subtrees in way that’s necessary to check for descendants that are referenced by + // aria-labelledby or aria-describedby and/or un-hidden. See the comment for substep A above. + if (is_text() && (parent_element()->is_referenced() || !parent_element()->is_hidden() || !parent_element()->has_hidden_ancestor() || parent_element()->has_referenced_and_hidden_ancestor())) { if (layout_node() && layout_node()->is_text_node()) return verify_cast(layout_node())->text_for_rendering(); return text_content().value(); } - // TODO: H. Otherwise, if the current node is a descendant of an element whose Accessible Name or Accessible Description is being computed, and contains descendants, proceed to 2F.i. + // H. Otherwise, if the current node is a descendant of an element whose Accessible Name or Accessible Description + // is being computed, and contains descendants, proceed to 2F.i. + // AD-HOC: We don’t implement this step here — because is essentially unreachable code in the spec algorithm. + // We could never get here without descending through every subtree of an element whose Accessible Name or + // Accessible Description is being computed. And in our implementation of substep F about, we’re anyway already + // recursively descending through all the child nodes of every element whose Accessible Name or Accessible + // Description is being computed, in a way that never leads to this substep H every being hit. // I. Otherwise, if the current node has a Tooltip attribute, return its value. // https://www.w3.org/TR/accname-1.2/#dfn-tooltip-attribute diff --git a/Tests/LibWeb/Text/expected/wpt-import/accname/name/comp_hidden_not_referenced.txt b/Tests/LibWeb/Text/expected/wpt-import/accname/name/comp_hidden_not_referenced.txt new file mode 100644 index 000000000000..92dee6c38c54 --- /dev/null +++ b/Tests/LibWeb/Text/expected/wpt-import/accname/name/comp_hidden_not_referenced.txt @@ -0,0 +1,15 @@ +Summary + +Harness status: OK + +Rerun + +Found 5 tests + +5 Pass +Details +Result Test Name MessagePass button containing a rendered, unreferenced element that is aria-hidden=true, an unreferenced element with the hidden host language attribute, and an unreferenced element that is unconditionally rendered +Pass button labelled by element that is aria-hidden=true +Pass button labelled by element with the hidden host language attribute +Pass link labelled by elements with assorted visibility and a11y tree exposure +Pass heading with name from content, containing element that is visibility:hidden with nested content that is visibility:visible \ No newline at end of file diff --git a/Tests/LibWeb/Text/expected/wpt-import/accname/name/comp_labelledby_hidden_nodes.txt b/Tests/LibWeb/Text/expected/wpt-import/accname/name/comp_labelledby_hidden_nodes.txt new file mode 100644 index 000000000000..38a76614a0db --- /dev/null +++ b/Tests/LibWeb/Text/expected/wpt-import/accname/name/comp_labelledby_hidden_nodes.txt @@ -0,0 +1,37 @@ +Summary + +Harness status: OK + +Rerun + +Found 27 tests + +27 Pass +Details +Result Test Name MessagePass button with aria-labelledby using display:none hidden span (with nested span) +Pass button with aria-labelledby using display:none hidden span (with nested spans, depth 2) +Pass button with aria-labelledby using span without display:none (with nested display:none spans, depth 2) +Pass button with aria-labelledby using display:none hidden span (with nested sibling spans) +Pass button with aria-labelledby using span without display:none (with nested display:none sibling spans) +Pass button with aria-labelledby using span with display:none (with nested display:inline sibling spans) +Pass button with aria-labelledby using visibility:hidden span (with nested span) +Pass button with aria-labelledby using visibility:hidden span (with nested spans, depth 2) +Pass button with aria-labelledby using span without visibility:hidden (with nested visibility:hidden spans, depth 2) +Pass button with aria-labelledby using visibility:hidden hidden span (with nested sibling spans) +Pass button with aria-labelledby using span without visibility:hidden (with nested visibility:hidden sibling spans) +Pass button with aria-labelledby using span with visibility:hidden (with nested visibility:visible sibling spans) +Pass button with aria-labelledby using visibility:collapse span (with nested span) +Pass button with aria-labelledby using visibility:collapse span (with nested spans, depth 2) +Pass button with aria-labelledby using span without visibility:collapse (with nested visibility:visible spans, depth 2) +Pass button with aria-labelledby using visibility:collapse span (with nested sibling spans) +Pass button with aria-labelledby using span without visibility:collapse (with nested visibility:collapse sibling spans) +Pass button with aria-labelledby using span with visibility:collapse (with nested visible sibling spans) +Pass button with aria-labelledby using aria-hidden span (with nested span) +Pass button with aria-labelledby using aria-hidden span (with nested spans, depth 2) +Pass button with aria-labelledby using span without aria-hidden (with nested aria-hidden spans, depth 2) +Pass button with aria-labelledby using aria-hidden hidden span (with nested sibling spans) +Pass button with aria-labelledby using HTML5 hidden span (with nested span) +Pass button with aria-labelledby using HTML5 hidden span (with nested spans, depth 2) +Pass button with aria-labelledby using span without HTML5 hidden (with nested HTML5 hidden spans, depth 2) +Pass button with aria-labelledby using HTML5 hidden span (with nested hidden sibling spans) +Pass button with aria-labelledby using span without HTML5 hidden (with nested hidden sibling spans) diff --git a/Tests/LibWeb/Text/input/wpt-import/accname/name/comp_hidden_not_referenced.html b/Tests/LibWeb/Text/input/wpt-import/accname/name/comp_hidden_not_referenced.html new file mode 100644 index 000000000000..0637deb46ba9 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/accname/name/comp_hidden_not_referenced.html @@ -0,0 +1,92 @@ + + + + + Name Comp: Hidden Not Referenced + + + + + + + + + +

Tests the #comp_hidden_not_referenced portions of the AccName Name Computation algorithm.

+ + + + + + + + + + visible to all users, + + + + + + +

+ visible to all users, + + hidden from all users, + un-hidden for all users + +

+ + + + + + + + + + + + + + + diff --git a/Tests/LibWeb/Text/input/wpt-import/accname/name/comp_labelledby_hidden_nodes.html b/Tests/LibWeb/Text/input/wpt-import/accname/name/comp_labelledby_hidden_nodes.html new file mode 100644 index 000000000000..6a070fd3abf3 --- /dev/null +++ b/Tests/LibWeb/Text/input/wpt-import/accname/name/comp_labelledby_hidden_nodes.html @@ -0,0 +1,245 @@ + + + + Name Comp: Labelledby & Hidden Nodes + + + + + + + + + +

Tests hidden node name computation as part of the #comp_labelledby portion of the AccName Name Computation algorithm.

+ + + +

Testing with display:none

+ + + + + + + + + + foo + + + + + + + + + foo + + + + + + + +

Testing with visibility:hidden

+ + + + + + + + + + foo + + + + + + + + + foo + + + + + + + +

Testing with visibility:collapse

+ + + + foo + bar + + + + + foo + + bar + baz + + + + + + foo + + bar + baz + + + + + + foo + bar + baz + + + + + foo + bar + baz + + + + + foo + bar + baz + + +

Testing with aria-hidden

+ + + + + + + + + + foo + + + + + + +

Testing with hidden attribute

+ + + + + + + + + + foo + + + + + + + + + foo + + + + + + + \ No newline at end of file From 4c06624a44eb2704584033bda3725d43a17a8f23 Mon Sep 17 00:00:00 2001 From: sideshowbarker Date: Mon, 18 Nov 2024 04:20:24 +0900 Subject: [PATCH 2/2] LibWeb: Fix aria-label precedence in accessible-name computation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change makes Ladybird give the value of the aria-label attribute the correct precedence for accessible-name computation required by the “Accessible Name and Description Computation” and HTML-AAM specs and by the corresponding WPT tests. Otherwise, without this change, Ladybird fails some of the WPT subtests of the test at https://wpt.fyi/results/accname/name/comp_label.html. --- Libraries/LibWeb/DOM/Node.cpp | 37 ++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/Libraries/LibWeb/DOM/Node.cpp b/Libraries/LibWeb/DOM/Node.cpp index 1895b5e41d55..f9181fd6de21 100644 --- a/Libraries/LibWeb/DOM/Node.cpp +++ b/Libraries/LibWeb/DOM/Node.cpp @@ -2256,13 +2256,31 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con total_accumulated_text.append(result); } // iii. Return the accumulated text. + // AD-HOC: This substep in the spec doesn’t seem to explicitly require the following check for an aria-label + // value; but the “button's hidden referenced name (visibility:hidden) with hidden aria-labelledby traversal + // falls back to aria-label” subtest at https://wpt.fyi/results/accname/name/comp_labelledby.html won’t pass + // unless we do this check. + if (total_accumulated_text.to_string().value().bytes_as_string_view().is_whitespace() && target == NameOrDescription::Name && element->aria_label().has_value() && !element->aria_label()->is_empty() && !element->aria_label()->bytes_as_string_view().is_whitespace()) + return element->aria_label().value(); return total_accumulated_text.to_string(); } - // C. Embedded Control: Otherwise, if the current node is a control embedded - // within the label (e.g. any element directly referenced by aria-labelledby) for - // another widget, where the user can adjust the embedded control's value, then - // return the embedded control as part of the text alternative in the following - // manner: + + // D. AriaLabel: Otherwise, if the current node has an aria-label attribute whose value is not undefined, not + // the empty string, nor, when trimmed of whitespace, is not the empty string: + // AD-HOC: We’ve reordered substeps C and D from https://w3c.github.io/accname/#step2 — because + // the more-specific per-HTML-element requirements at https://w3c.github.io/html-aam/#accname-computation + // necessitate doing so, and the “input with label for association is superceded by aria-label” subtest at + // https://wpt.fyi/results/accname/name/comp_label.html won’t pass unless we do this reordering. + // Spec PR: https://github.com/w3c/aria/pull/2377 + if (target == NameOrDescription::Name && element->aria_label().has_value() && !element->aria_label()->is_empty() && !element->aria_label()->bytes_as_string_view().is_whitespace()) { + // TODO: - If traversal of the current node is due to recursion and the current node is an embedded control as defined in step 2E, ignore aria-label and skip to rule 2E. + // - Otherwise, return the value of aria-label. + return element->aria_label().value(); + } + + // C. Embedded Control: Otherwise, if the current node is a control embedded within the label (e.g. any element + // directly referenced by aria-labelledby) for another widget, where the user can adjust the embedded control's + // value, then return the embedded control as part of the text alternative in the following manner: GC::Ptr labels; if (is(this)) labels = (const_cast(static_cast(*current_node))).labels(); @@ -2332,15 +2350,6 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con return builder.to_string(); } - // D. AriaLabel: Otherwise, if the current node has an aria-label attribute whose - // value is not undefined, not the empty string, nor, when trimmed of whitespace, - // is not the empty string: - if (target == NameOrDescription::Name && element->aria_label().has_value() && !element->aria_label()->is_empty() && !element->aria_label()->bytes_as_string_view().is_whitespace()) { - // TODO: - If traversal of the current node is due to recursion and the current node is an embedded control as defined in step 2E, ignore aria-label and skip to rule 2E. - // - Otherwise, return the value of aria-label. - return element->aria_label().value(); - } - // E. Host Language Label: Otherwise, if the current node's native markup provides an attribute (e.g. alt) or // element (e.g. HTML label or SVG title) that defines a text alternative, return that alternative in the form // of a flat string as defined by the host language, unless the element is marked as presentational