1191432 - [Accessibility] Improve accessibility coverage for marionette accessibility checks.

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Yura Zenevich [:yzen] (please use "needinfo")]

Marionette already has a number of accessibility checks that can trigger test failures if 'raisesAccessibilityExceptions' capability is enabled. The coverage however can be improved on the marionette server side. Here are some of the items that should be implemented:

* Add missing actionable elements to the list (including ARIA 1.1) - switch and rowheader. Switch is already used by gaia-switch web component.

* Check for keyboard accessibility using STATE_FOCUSABLE verification. All elements that are actionable should also be focusable.

* Elements that the user can send keys to should also be actionable. It means that they should have an appropriate accessible role, name, actions count and be focusable.

* Improve testing for elements enabled state from the accessibility standpoint. At the moment we only test if the accessible's STATE_UNAVAILABLE is in sync with the DOM element's disabled state. We only do it when is_enabled is called on the element. We should also check if the above states are in sync with element's computed pointer-events style. We should also if element is enabled to accessibility API on click (since we already do that in general case).

* Ensure we check SELECTED accessibility state when we check if element is selected.

Comment 1

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: 1191432 patch v1

https://treeherder.mozilla.org/#/jobs?repo=try&revision=177710cb405f

Comment 2

[David Burns :automatedtester]

Comment on attachment 8644349 [details] [diff] [review]
1191432 patch v1

Review of attachment 8644349 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good!

In future could I ask that marionette patches use MozReview? http://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview.html

Comment 3

[Yura Zenevich [:yzen] (please use "needinfo")]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9eeddf4d34ea

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Backed out for various Gij failures all over the place. Please verify that this is green on Try before re-pushing.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8e1e749c21bf

Comment 5

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8644349 [details] [diff] [review]
1191432 patch v1

Review of attachment 8644349 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/marionette/listener.js
@@ +1608,5 @@
> +    } else {
> +      utils.sendKeysToElement(curFrame, el, val, sendOk, sendError, command_id);
> +    }
> +  } catch (e) {
> +    sendError(e, command_id);

Why did you remove the try…catch here?

This will silently discard errors thrown by the code in this function because listener.js does not (yet) have automatically wrap command implementations.

Comment 6

[Andreas Tolfsen ❲:ato❳]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #4)
> Backed out for various Gij failures all over the place. Please verify that
> this is green on Try before re-pushing.

For future reference, a useful try syntax to use when testing Marionette changes is this:

% echo $trymarionette
-b o -p linux,linux64_gecko -u marionette,marionette-e10s,web-platform-tests-1,marionette-webapi,gaia-ui-test-functional,gaia-integration -t none

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Andreas Tolfsen (:ato) from comment #6)
> (In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #4)
> > Backed out for various Gij failures all over the place. Please verify that
> > this is green on Try before re-pushing.
> 
> For future reference, a useful try syntax to use when testing Marionette
> changes is this:
> 
> % echo $trymarionette
> -b o -p linux,linux64_gecko -u
> marionette,marionette-e10s,web-platform-tests-1,marionette-webapi,gaia-ui-
> test-functional,gaia-integration -t none

Ah, thanks, I did not get the gaia stuff included!

Comment 8

[Yura Zenevich [:yzen] (please use "needinfo")]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8bac8b79301

Comment 9

[Yura Zenevich [:yzen] (please use "needinfo")]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4792c7882ab0

Comment 10

[Yura Zenevich [:yzen] (please use "needinfo")]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3a9af4abb4d

Comment 11

[Yura Zenevich [:yzen] (please use "needinfo")]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d8159e66db6c

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d8159e66db6c

Comment 13

[Nigel Babu [:nigelb]]

Backed out in https://hg.mozilla.org/mozilla-central/rev/8a6045d14d6b

Comment 14

[Pulsebot]

https://hg.mozilla.org/mozilla-central/rev/ba43a48d3c52

Comment 15

[Nigel Babu [:nigelb]]

Relanded in https://hg.mozilla.org/mozilla-central/rev/ba43a48d3c52

Comment 16

[Aus Lacroix [:aus]]

My apologies but this seems to be causing issues still. I'm backing it out for now. :/

Comment 17

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Aus Lacroix [:aus] from comment #16)
> My apologies but this seems to be causing issues still. I'm backing it out
> for now. :/

Are there some details about the issues by chance? Are they on b2g-ibound or gaia-master ?

Comment 18

[Yura Zenevich [:yzen] (please use "needinfo")]

AFAIK, it was landed back, closing.