Closed Bug 1854521 Opened 1 year ago Closed 1 year ago

Investigate div#fd element that is failing Tier 2 a11y_checks for Core: Layout Engine

Categories

(Core :: Layout, task, P3)

task

Tracking

()

RESOLVED FIXED
120 Branch
Tracking Status
firefox120 --- fixed

People

(Reporter: ayeddi, Assigned: ayeddi)

References

Details

(Keywords: access)

Attachments

(1 file)

We are working on enabling Tier 2 a11y-checks to ensure our products have basic accessibility built-in (bug 1692110) and before that, we need to prepare the existing code base. While we triage and investigate the existent test that would fail once the a11y-checks are enabled in the CI, the failing test was failed-if by the bug 1854520.

This task is to track the investigation into the reasons this test failed and to be able to backtrack this test once it's resolved.

If separate bugs are needed, they are to block the following bugs:

  1. (all) this bug
  2. (Node is not accessible via accessibility API / Node is not focusable via the accessibility API / Node is enabled but disabled via the accessibility API) Bug 1848394 (a11y_checks_focusable)

Test affected:

FAIL	layout/base/tests/browser_bug1701027-1.js	Node is not accessible via accessibility API	fd	DIV

Jobs affected:

  1. test-linux1804-64-qr/opt-mochitest-browser-chrome-swr-a11y-checks-${1-7} (Try run)

Some Tier 2 accessibility checks for click events fired on controls that should be keyboard accessible and have valid labels were failing for this component. They were captured by testing/mochitest/tests/SimpleTest/AccessibilityUtils.js via bug 1692110. These failing tests should be temporarily skipped in the directory's browser.ini file while we investigate these failures. For all confirmed bugs individual defects should be filed.

When the test failure is resolved and its associated bug is closed, remove the fail-if condition for a11y_checks from the appropriate file/section to ensure better test coverage and to avoid regressions in a11y of this component.

Jobs affected:

  1. test-linux1804-64-qr/opt-mochitest-browser-chrome-swr-a11y-checks-${1-7} (Try run)

The task with the failing test there is bc5 -- here's the direct link to the failure in the log:
https://treeherder.mozilla.org/logviewer?job_id=428881828&repo=try&lineNumber=70294

Based on the backtrace (handleClick calling into assertCanBeClicked), it looks like the tool is concerned that we're sending a click[1] to an element that's not keyboard-focusable[2], and our accessibility tooling is alerting us that this clickable-element cannot be interacted with by keyboard users.

However, this isn't really an issue, since this isn't content we're showing to any users; it's essentially a crashtest (written as a browser-chrome test), trying to set up a set of conditions that used to trigger a crash. We probably don't want to change it to make the accessibility-checker approve of it, since it's conceivable that we might inadvertently nerf the crashtest so that it stops triggering the proper set of conditions that caused the crash in the first place. (And it's hard to validate whether we inadvertently do that, since we don't crash anymore.)

So, this is a WONTFIX, I think, unless there's a more graceful way for this test to opt out of this tooling (besides the fail-if annotation on https://phabricator.services.mozilla.com/D188920 ). Anna, can we just close this, or do you have any other recommendations for better annotations/opting-out here?

[1] The click is synthesized here
[2] The click hits this empty fixed-position div that fills the viewport in the test's helper html document.

Flags: needinfo?(ayeddi)

Thank you for investigating it, Daniel! This is really helpful!

My next step (after we silence/fail-if the 269 existent failures across the 24 components and activate new a11y_checks in CI) is to go test-by-test and triage the failures. Some would be real bugs, some may be false positives that would prompt Accessibiility Utils fine-tuning. But some would be a special cases, like this one appears to be - in this case I'll setEnv to exclude this specific click from the a11y_checks test case.

I will be happy to start with this test - I'll prep the patch for you and include the removal of the fail-if in it too (I'll keep my NI for now, so it's on top of the stack). It'd be expected that the a11y_checks won't fail after that. Once again, thank you for the information - it is super helpful.

That sounds great, thanks!

This isn't content we're showing to any users; it's essentially a crashtest (written as a browser-chrome test), trying to set up a set of conditions that used to trigger a crash.

Thus, we are exluding this click event from an accessibility checks and removing the fail-if annotation that was added before the investigation by the bug 1854520.

Attachment #9357182 - Attachment description: Bug 1854521 - Disable a11y_check for a non-user-facing chashtest in Core Layout Engine. r=dholbert,Jamie → Bug 1854521 - Disable a11y_check for a non-user-facing crashtest in Core Layout Engine. r=dholbert,Jamie
Pushed by ayeddi@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e05d90bdc7c3 Disable a11y_check for a non-user-facing crashtest in Core Layout Engine. r=dholbert
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 120 Branch
Flags: needinfo?(ayeddi)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: