From fdcb27749de4ffc16aee74f4fe51b98da35ec311 Mon Sep 17 00:00:00 2001 From: sideshowbarker Date: Tue, 25 Mar 2025 18:59:18 +0900 Subject: [PATCH] LibWeb: Update ARIA default-role handling for img, section, and aside This change updates our handling of computation for default ARIA roles for the img, section, and aside elements to match the expectations in the updated WPT test at http://wpt.live/html-aam/roles-contextual.html. --- Libraries/LibWeb/DOM/Node.cpp | 18 ++++---- Libraries/LibWeb/HTML/HTMLElement.cpp | 4 +- Libraries/LibWeb/HTML/HTMLImageElement.cpp | 13 +++--- .../wpt-import/html-aam/roles-contextual.txt | 36 ++++++++++++++-- .../wpt-import/html-aam/roles-contextual.html | 41 ++++++++++++++++++- 5 files changed, 90 insertions(+), 22 deletions(-) diff --git a/Libraries/LibWeb/DOM/Node.cpp b/Libraries/LibWeb/DOM/Node.cpp index 62d020cea44..f236ae9c10a 100644 --- a/Libraries/LibWeb/DOM/Node.cpp +++ b/Libraries/LibWeb/DOM/Node.cpp @@ -2470,16 +2470,14 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con // 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. + // https://github.com/w3c/aria/issues/2404 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 - // element or section element 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, we only compute a default role here if this - // isn’t an aside element or section element. + // Per-spec, at https://w3c.github.io/html-aam/#el-aside and elsewhere, computing a default role for certain + // elements (img, aside, and section) requires first computing its accessible name. So to avoid getting into an + // infinite loop here, the callers in our code for those cases pass in ShouldComputeRole::Yes. // https://github.com/w3c/aria/issues/2391 - if (!role.has_value() && element->local_name() != HTML::TagNames::aside && element->local_name() != HTML::TagNames::section) + if (should_compute_role == ShouldComputeRole::Yes && !role.has_value()) role = element->default_role(); // 2. Compute the text alternative for the current node: @@ -2550,7 +2548,7 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con total_accumulated_text.append(result); } - // iii. Return the accumulated text. + // iii. Return the accumulated text if it is not the empty string (""). // 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 @@ -2675,7 +2673,9 @@ ErrorOr Node::name_or_description(NameOrDescription target, Document con // // https://w3c.github.io/html-aam/#img-element-accessible-name-computation // use alt attribute, even if its value is the empty string. - // See also https://wpt.fyi/results/accname/name/comp_tooltip.tentative.html. + // See also https://wpt.fyi/results/accname/name/comp_tooltip.tentative.html — but also see + // https://github.com/w3c/aria/issues/2491 and the el-img-empty-alt-title subtest in the test at + // https://wpt.fyi/results/html-aam/roles-contextual.html. if (is(*element) && element->has_attribute(HTML::AttributeNames::alt)) return element->get_attribute(HTML::AttributeNames::alt).value(); diff --git a/Libraries/LibWeb/HTML/HTMLElement.cpp b/Libraries/LibWeb/HTML/HTMLElement.cpp index a19927021f4..252e0f3ad8f 100644 --- a/Libraries/LibWeb/HTML/HTMLElement.cpp +++ b/Libraries/LibWeb/HTML/HTMLElement.cpp @@ -845,7 +845,7 @@ Optional HTMLElement::default_role() const // https://w3c.github.io/html-aam/#el-aside for (auto const* ancestor = parent_element(); ancestor; ancestor = ancestor->parent_element()) { if (ancestor->local_name().is_one_of(TagNames::article, TagNames::aside, TagNames::nav, TagNames::section) - && accessible_name(document()).value().is_empty()) + && accessible_name(document(), DOM::ShouldComputeRole::No).value().trim_whitespace(TrimMode::Both).value().is_empty()) return ARIA::Role::generic; } // https://w3c.github.io/html-aam/#el-aside-ancestorbodymain @@ -932,7 +932,7 @@ Optional HTMLElement::default_role() const // https://www.w3.org/TR/html-aria/#el-section if (local_name() == TagNames::section) { // role=region if the section element has an accessible name - if (!accessible_name(document()).value().is_empty()) + if (!accessible_name(document(), DOM::ShouldComputeRole::No).value().trim_whitespace(TrimMode::Both).value().is_empty()) return ARIA::Role::region; // Otherwise, role=generic return ARIA::Role::generic; diff --git a/Libraries/LibWeb/HTML/HTMLImageElement.cpp b/Libraries/LibWeb/HTML/HTMLImageElement.cpp index 7477190293f..0090c9a30a0 100644 --- a/Libraries/LibWeb/HTML/HTMLImageElement.cpp +++ b/Libraries/LibWeb/HTML/HTMLImageElement.cpp @@ -418,17 +418,18 @@ WebIDL::ExceptionOr> HTMLImageElement::decode() const Optional HTMLImageElement::default_role() const { + // https://www.w3.org/TR/html-aria/#el-img-empty-alt + // NOTE: The "none" role value is a synonym for the older "presentation" role value; however, the el-img-alt-no-value + // test in https://wpt.fyi/results/html-aam/roles.html expects the value to be "none" (not "presentation"). + if (has_attribute(HTML::AttributeNames::alt) && alt().is_empty() + && accessible_name(document(), DOM::ShouldComputeRole::No).value().trim_whitespace(TrimMode::Both).value().is_empty()) + return ARIA::Role::none; // https://www.w3.org/TR/html-aria/#el-img // https://www.w3.org/TR/html-aria/#el-img-no-alt // https://w3c.github.io/aria/#image // NOTE: The "image" role value is a synonym for the older "img" role value; however, the el-img test in // https://wpt.fyi/results/html-aam/roles.html expects the value to be "image" (not "img"). - if (!alt().is_empty()) - return ARIA::Role::image; - // https://www.w3.org/TR/html-aria/#el-img-empty-alt - // NOTE: The "none" role value is a synonym for the older "presentation" role value; however, the el-img-alt-no-value - // test in https://wpt.fyi/results/html-aam/roles.html expects the value to be "none" (not "presentation"). - return ARIA::Role::none; + return ARIA::Role::image; } // https://html.spec.whatwg.org/multipage/images.html#use-srcset-or-picture diff --git a/Tests/LibWeb/Text/expected/wpt-import/html-aam/roles-contextual.txt b/Tests/LibWeb/Text/expected/wpt-import/html-aam/roles-contextual.txt index d272dff7495..270aff05594 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/html-aam/roles-contextual.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/html-aam/roles-contextual.txt @@ -1,8 +1,9 @@ Harness status: OK -Found 19 tests +Found 48 tests -19 Pass +47 Pass +1 Fail Pass el-a Pass el-aside Pass el-aside-in-main @@ -12,13 +13,42 @@ Pass el-aside-in-aside-with-name Pass el-aside-in-nav-with-name Pass el-aside-in-nav-with-role Pass el-aside-in-section-with-name +Pass el-aside-in-section-aria-labelledby +Pass el-aside-in-section-title Pass el-footer-ancestorbody Pass el-header-ancestorbody +Pass el-img-no-name +Pass el-img-empty-alt-aria-label +Pass el-img-empty-alt-aria-labelledby +Fail el-img-empty-alt-title Pass el-section +Pass el-section-aria-labelledby +Pass el-section-title Pass el-a-no-href Pass el-aside-in-article-in-main Pass el-aside-in-article Pass el-aside-in-aside Pass el-aside-in-nav Pass el-aside-in-section -Pass el-section-no-name \ No newline at end of file +Pass el-aside-in-section-aria-label-empty +Pass el-aside-in-section-aria-label-whitespace +Pass el-aside-in-section-aria-labelledby-non-existing +Pass el-aside-in-section-aria-labelledby-empty +Pass el-aside-in-section-aria-labelledby-whitespace +Pass el-aside-in-section-title-empty +Pass el-aside-in-section-title-whitespace +Pass el-img-empty-alt-aria-label-empty +Pass el-img-empty-alt-aria-label-whitespace +Pass el-img-empty-alt-aria-labelledby-non-existing +Pass el-img-empty-alt-aria-labelledby-empty +Pass el-img-empty-alt-aria-labelledby-whitespace +Pass el-img-empty-alt-title-empty +Pass el-img-empty-alt-title-whitespace +Pass el-section-no-name +Pass el-section-aria-label-empty +Pass el-section-aria-label-whitespace +Pass el-section-aria-labelledby-non-existing +Pass el-section-aria-labelledby-empty +Pass el-section-aria-labelledby-whitespace +Pass el-section-title-empty +Pass el-section-title-whitespace \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/wpt-import/html-aam/roles-contextual.html b/Tests/LibWeb/Text/input/wpt-import/html-aam/roles-contextual.html index 5fd897c1e62..298f44d954a 100644 --- a/Tests/LibWeb/Text/input/wpt-import/html-aam/roles-contextual.html +++ b/Tests/LibWeb/Text/input/wpt-import/html-aam/roles-contextual.html @@ -12,7 +12,7 @@ -

Tests contextual computedrole mappings defined in HTML-AAM, where the returned computed role is expected to change based on the context. Most test names correspond to a unique ID defined in the spec.

+

Tests contextual computed role mappings defined in HTML-AAM, where the returned computed role is expected to change based on the context. Most test names correspond to a unique ID defined in the spec.

These should remain in alphabetical order.

@@ -50,6 +50,15 @@
+ + + + + + + + +
@@ -62,10 +71,38 @@
x
+ + + + + + + + + + + + + + +
x
x
+
x
+
x
+
x
+
x
+
x
+
x
+
x
+
x
+
x
+ +
labelledby
+
+
- \ No newline at end of file +