[Accessibility] Improve accessibility coverage for marionette accessibility checks.

RESOLVED FIXED in Firefox 43

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: yzen, Assigned: yzen)

Tracking

({access})

Trunk
mozilla43
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox42 affected, firefox43 fixed)

Details

Attachments

(1 attachment)

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 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
Attachment #8644349 - Flags: review?(dburns) → review+
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 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.
(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
(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!
Depends on: 1193018
https://hg.mozilla.org/mozilla-central/rev/d8159e66db6c
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Backed out in https://hg.mozilla.org/mozilla-central/rev/8a6045d14d6b
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Relanded in https://hg.mozilla.org/mozilla-central/rev/ba43a48d3c52
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
My apologies but this seems to be causing issues still. I'm backing it out for now. :/
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(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 ?
Flags: needinfo?(aus)
AFAIK, it was landed back, closing.
Flags: needinfo?(aus)
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.