1277791 - Do not activate accessibility support by default in Marionette

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Yesterday we noticed over on bug 1271911 that Marionette enables accessibility support by default. We should try to avoid that due to possible side-effects and most likely slowdowns in test execution. Tests which actually would need a11y turned on would most likely be the accessibility tests only.

As David mentioned we use it to somewhat interact with elements, and there might be some refactoring work necessary to get it working without a11y.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Deactivating accessibility features will be rather difficult in Marionette because our event input system relies heavily on it.

See these two files:

https://github.com/mozilla/gecko-dev/blob/master/testing/marionette/interaction.js
https://github.com/mozilla/gecko-dev/blob/master/testing/marionette/accessibility.js

yzen knows the dependency chain here better than I do, so I will let him speak to the feasibility of making it optional.

Comment 2

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

I think there are 2 issues that you list here:

* slowdowns in test execution - this has been addressed previously since we were polling for accessible objects even if we did not report accessibility errors. With strict mode off (non-accessibility test default) this is not the case. Once a11y is initiated, I believe performance will not be the issue compared, for example, with communication to marionette client.

* initiating a11y is a separate issue, which we can consider not doing if we are not reporting errors (e.g. non-strict mode). I'm alright with that since I do not think it is that useful anyways. I could take that bug if that's not a super urgent thing?

As a side note: we are making improvements to a11y shutdown so very soon in the future it will do so automatically if there are no references held to it from JS. This could be a better option perhaps ?

Comment 3

[Andreas Tolfsen ❲:ato❳]

I don’t really have any strong opinions here.  whimboo, how severe are the side-effects you speak of?

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So this is not too important for us right now given that we found a workaround. But not sure if other issues exists because of a11y is enabled. Yura, do we have any data on shutdown times? Given that our tests restart often it would be good to know if there is any impact.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Enabling a11y partly or completely is actually causing a problem those days for Mn-e10s tests on Windows XP. I run into this problem with my patch on bug 1299216 which improves crash handling. Sadly I see tons of failures which are a11y related and most likely cannot land this patch.

Andreas or Yura, if there is a bit of time in your schedule I would appreciate if you could have a look if we really need a11y in Marionette.

Comment 6

[Andreas Tolfsen ❲:ato❳]

I’ll defer to yzenevich.  From the perspective of WebDriver conformance, we don’t require accessibility checks.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Anreas, please keep in mind that he is not around the next 3 weeks. :/ Maybe I will discuss with David tomorrow too.

Comment 8

[Andreas Tolfsen ❲:ato❳]

I don’t have any insight into the testing requirements of Firefox, but I assume accessibility checks were added to fulfil some testing need.  If enabling it is causing problems on Win XP, we can choose to ignore XP if possible, or go down a long road of writing a Windows XP-specific patch for not enabling it.

Also it’s not clear to me why the accessibility features are failing on Windows XP in the first place, so I’m going to guess that others have made similar shortcuts in the past.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Please disregard the latest comments. As I have just noticed on bug 1309556 we do NOT run e10s tests for WinXP on integration branches. I simply ticked that option in trychooser. So the problem I have above can simply ignored. Hurray!

Comment 10

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

resolved with bug 1312816

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks Yura!