Closed Bug 1864576 Opened 8 months ago Closed 7 months ago

Investigate how 2 browser_UsageTelemetry_interaction tests in Tabbed Browser need to be updated to pass a11y_checks

Categories

(Firefox :: Tabbed Browser, task, P3)

task

Tracking

()

RESOLVED FIXED
122 Branch
Tracking Status
firefox122 --- fixed

People

(Reporter: ayeddi, Assigned: ayeddi)

References

Details

Attachments

(1 file)

There are 2 test cases within the same test file which are failing Tier 2 a11y-checks, because the click events are sent to elements that are disabled in Accessibility API when the checks are running.

Failing cases are the following:

FAIL	browser/modules/test/browser/browser_UsageTelemetry_interaction.js	Node is enabled but disabled via the accessibility API	reload-button	toolbarbutton	toolbarbutton-1	"
chrome://mochikit/content/browser-test.js:test_ok:1583
chrome://mochikit/content/tests/SimpleTest/AccessibilityUtils.js:a11yFail:339
chrome://mochikit/content/tests/SimpleTest/AccessibilityUtils.js:assertEnabled:363
chrome://mochikit/content/tests/SimpleTest/AccessibilityUtils.js:assertCanBeClicked:669
chrome://mochikit/content/tests/SimpleTest/AccessibilityUtils.js:runIfA11YChecks/this.AccessibilityUtils:635
chrome://mochikit/content/tests/SimpleTest/AccessibilityUtils.js:handleClick:751
chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeMouseAtPoint:652
chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeMouse:555
chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeMouseAtCenter:740
chrome://mochitests/content/browser/browser/modules/test/browser/browser_UsageTelemetry_interaction.js:click:70
chrome://mochitests/content/browser/browser/modules/test/browser/browser_UsageTelemetry_interaction.js:toolbarButtons/<:97
resource://testing-common/BrowserTestUtils.sys.mjs:withNewTab:146
chrome://mochitests/content/browser/browser/modules/test/browser/browser_UsageTelemetry_interaction.js:toolbarButtons:74
"

FAIL	browser/modules/test/browser/browser_UsageTelemetry_interaction.js	Node is enabled but disabled via the accessibility API	back-button	toolbarbutton	toolbarbutton-1 chromeclass-toolbar-additional	"
chrome://mochikit/content/browser-test.js:test_ok:1583
chrome://mochikit/content/tests/SimpleTest/AccessibilityUtils.js:a11yFail:339
chrome://mochikit/content/tests/SimpleTest/AccessibilityUtils.js:assertEnabled:363
chrome://mochikit/content/tests/SimpleTest/AccessibilityUtils.js:assertCanBeClicked:669
chrome://mochikit/content/tests/SimpleTest/AccessibilityUtils.js:runIfA11YChecks/this.AccessibilityUtils:635
chrome://mochikit/content/tests/SimpleTest/AccessibilityUtils.js:handleClick:751
chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeMouseAtPoint:652
chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeMouse:555
chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeMouseAtCenter:740
chrome://mochitests/content/browser/browser/modules/test/browser/browser_UsageTelemetry_interaction.js:click:70
chrome://mochitests/content/browser/browser/modules/test/browser/browser_UsageTelemetry_interaction.js:toolbarButtons/<:98
resource://testing-common/BrowserTestUtils.sys.mjs:withNewTab:146
chrome://mochitests/content/browser/browser/modules/test/browser/browser_UsageTelemetry_interaction.js:toolbarButtons:74
Component: Frontend → Tabbed Browser
Product: WebExtensions → Firefox
Summary: Investigate if 2 tests in Tabbed Browser need to be updated to pass a11y_checks → Investigate if 2 UsageTelemetry_interaction tests in Tabbed Browser need to be updated to pass a11y_checks
See Also: → 1864577
Summary: Investigate if 2 UsageTelemetry_interaction tests in Tabbed Browser need to be updated to pass a11y_checks → Investigate how 2 UsageTelemetry_interaction tests in Tabbed Browser need to be updated to pass a11y_checks
Summary: Investigate how 2 UsageTelemetry_interaction tests in Tabbed Browser need to be updated to pass a11y_checks → Investigate how 2 browser_UsageTelemetry_interaction tests in Tabbed Browser need to be updated to pass a11y_checks

Dave, do you know if we are sending clicks to disabled toolbarbuttons in this test intentionally or is it a test bug?

Flags: needinfo?(dtownsend)

(In reply to Anna Yeddi [:ayeddi] from comment #1)

Dave, do you know if we are sending clicks to disabled toolbarbuttons in this test intentionally or is it a test bug?

After much digging I still can't actually answer this. I don't know if we failed to consider disabled buttons when building the telemetry or we did and didn't document what we wanted or implemented it wrong.

We don't want to stop recording telemetry from disabled buttons, from bug 1854999 comment 4 it sounds like the preference would be to instead record clicks on disabled items differently rather than removing the telemetry entirely. I don't think you're proposing changing the telemetry anyway, but if we don't want to change the telemetry I'd rather we kept these tests as-is to ensure that it doesn't change by accident.

So I guess why is it a problem that we're clicking on disabled buttons?

Flags: needinfo?(dtownsend) → needinfo?(ayeddi)

We intentionally turn off a11y_checks for these click events, because the test is checking the telemetry functionality and the following 3 clicks are targeting disabled controls to test the changes in scalars (for more refer to the bug 1864576 comment 2 and bug 1854999 comment 4).

Also, we are adding the test label to the custom widget 12foo to satisfy the a11y_check expecting to click on a control with an accessible name.

Since the test file is now passing, we are removing the fail-if notation that was added by the bug 1854506 before the investigation in the meta bug 1854507.

(In reply to Dave Townsend [:mossop] from comment #2)

(In reply to Anna Yeddi [:ayeddi] from comment #1)

Dave, do you know if we are sending clicks to disabled toolbarbuttons in this test intentionally or is it a test bug?

After much digging I still can't actually answer this. I don't know if we failed to consider disabled buttons when building the telemetry or we did and didn't document what we wanted or implemented it wrong.

We don't want to stop recording telemetry from disabled buttons, from bug 1854999 comment 4 it sounds like the preference would be to instead record clicks on disabled items differently rather than removing the telemetry entirely. I don't think you're proposing changing the telemetry anyway, but if we don't want to change the telemetry I'd rather we kept these tests as-is to ensure that it doesn't change by accident.

So I guess why is it a problem that we're clicking on disabled buttons?

Thank you for your investigation and feedback, Dave!

In general, we should not be clicking on a control that is not enabled for the Accessibility API since this would mean that it would be hidden for any assistive technology and the user won't be able to activate it. This is why we check for it in the AccessibilityUtils (a11y-checks jobs).

But since in our unit tests we do sometimes want to test that the disabled controls are, in fact, disabled and nothing happens, thus we added an environment variable mustBeEnabled that we could manually set for a specific click to tell the harness that we do, indeed, expect this target to be disabled.

It looks that this would be the case here as well, so I sent a patch for your review where we'd just ask a11y-checks to ignore the disabled state of these controls, so we are not touching anything with the telemetry and allow the test to proceed as usual. Just while passing the a11y-checks.

I also added a test label to the test custom widget in the same patch, since it affects the same test and it is not included in the telemetry checks, as far as I could tell. So we could un-fail-if this test file.

Flags: needinfo?(ayeddi)
Pushed by ayeddi@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1472fea43fc9
Add exceptions from a11y_checks for 3 clicks on disabled controls in browser_UsageTelemetry_interaction.js r=mossop
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 122 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: