1379098 - ARIA combobox should map to AXComboBox with role description of 'combo box'

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement)

Description

[Joanmarie Diggs]

According to the Core AAM [1], the ARIA combobox role should map to 

AXRole: AXComboBox
AXSubrole: <nil>
AXRoleDescription: 'combo box'

Gecko is currently exposing this role as an AXPopUpButton with roledescription 'pop up button'.

[1] https://rawgit.com/w3c/aria/master/core-aam/core-aam.html#role-map-combobox

Comment 1

[Joanmarie Diggs]

Attachment: proposed patch

Comment 2

[Marco Zehe (:MarcoZ)]

Comment on attachment 8884236 [details] [diff] [review]
proposed patch

This will also affect the mapping for the native <select> element with size=1, which is indeed designed to behave like a popup button menu like in Safari. We're mimicking the native widget behavior as closely as possible with native HTML widgets. So if we change this mapping, we'll break every <select> element with size=1, or rather, its accessibility.

So if ARIA comboboxes indeed require a different handling on MacOS than <select size="1">, we'll have to have a specially cased ARIA combobox for this. r- for now.

Comment 3

[Joanmarie Diggs]

Attachment: proposed patch

(In reply to Marco Zehe (:MarcoZ) from comment #2)

> So if ARIA comboboxes indeed require a different handling on MacOS than
> <select size="1">, we'll have to have a specially cased ARIA combobox for
> this. r- for now.

Thanks for the review! And sorry about not catching the select-related mapping. This patch creates a new internal role. I went with EDITCOMBOBOX because the ARIA role combobox assumes there will be a text input, whereas the HTML select with size of 1 does not. And I verified (via accessibility inspector) that HTML select is not turning into a AXComboBox. Please let me know if I missed anything else.

Comment 4

[Marco Zehe (:MarcoZ)]

Comment on attachment 8884797 [details] [diff] [review]
proposed patch

Thanks, this looks correct now! And don't worry, this is not the first time Apple's popup button menus have caused a bit of confusion for people who normally use Windows or Linux. ;) r=me.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed18b5a361f9
ARIA combobox should map to AXComboBox. r=marcoz

Comment 6

[Carsten Book [:Tomcat]]

backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=113017764&repo=mozilla-inbound

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for timing out in a11y's accessible/tests/mochitest/events/test_valuechange.html and browser-chrome's accessible/tests/browser/e10s/browser_caching_value.js:

https://hg.mozilla.org/integration/mozilla-inbound/rev/582c6d6f8b04fc9774f2ac8cbdc673282bdd2e19

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=ed18b5a361f9ee5f0656eb98e2fce54adbd93373&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable

Failure log a11y: https://treeherder.mozilla.org/logviewer.html#?job_id=113019935&repo=mozilla-inbound
[task 2017-07-10T13:47:30.863163Z] 13:47:30     INFO - TEST-PASS | accessible/tests/mochitest/events/test_valuechange.html | Wrong value of  'splitter'  
[task 2017-07-10T13:47:30.863897Z] 13:47:30     INFO - TEST-PASS | accessible/tests/mochitest/events/test_valuechange.html | Test with ID = ' 'splitter'  value changed' succeed.  Event 'value change' was handled. 
[task 2017-07-10T13:47:30.864437Z] 13:47:30     INFO - Invoke the ' 'combobox'  value changed' test { scenario #0: expected 'text value change' event;  }
[task 2017-07-10T13:47:30.865107Z] 13:47:30     INFO - Buffered messages finished
[task 2017-07-10T13:47:30.865791Z] 13:47:30     INFO - TEST-UNEXPECTED-FAIL | accessible/tests/mochitest/events/test_valuechange.html | Test timed out. 
[task 2017-07-10T13:47:30.866285Z] 13:47:30     INFO - reportError@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:121:7

Failure log browser-chrome: https://treeherder.mozilla.org/logviewer.html#?job_id=113019946&repo=mozilla-inbound
[task 2017-07-10T13:33:19.106117Z] 13:33:19     INFO - Value should change to @aria-valuetext when @aria-valuenow is removed
[task 2017-07-10T13:33:19.114514Z] 13:33:19     INFO - TEST-PASS | accessible/tests/browser/e10s/browser_caching_value.js | Wrong value of ['div@id="slider" node', address: [object HTMLDivElement], role: slider, address: [xpconnect wrapped (nsISupports, nsIAccessible, nsIAccessibleValue)]] - 
[task 2017-07-10T13:33:19.116470Z] 13:33:19     INFO - TEST-PASS | accessible/tests/browser/e10s/browser_caching_value.js | Wrong current value of ['div@id="slider" node', address: [object HTMLDivElement], role: slider, address: [xpconnect wrapped (nsISupports, nsIAccessible, nsIAccessibleValue)]] - 
[task 2017-07-10T13:33:19.118357Z] 13:33:19     INFO - TEST-PASS | accessible/tests/browser/e10s/browser_caching_value.js | Wrong minimum value of ['div@id="slider" node', address: [object HTMLDivElement], role: slider, address: [xpconnect wrapped (nsISupports, nsIAccessible, nsIAccessibleValue)]] - 
[task 2017-07-10T13:33:19.129275Z] 13:33:19     INFO - TEST-PASS | accessible/tests/browser/e10s/browser_caching_value.js | Wrong maximum value of ['div@id="slider" node', address: [object HTMLDivElement], role: slider, address: [xpconnect wrapped (nsISupports, nsIAccessible, nsIAccessibleValue)]] - 
[task 2017-07-10T13:33:19.134153Z] 13:33:19     INFO - TEST-PASS | accessible/tests/browser/e10s/browser_caching_value.js | Wrong minimum increment value of ['div@id="slider" node', address: [object HTMLDivElement], role: slider, address: [xpconnect wrapped (nsISupports, nsIAccessible, nsIAccessibleValue)]] - 
[task 2017-07-10T13:33:19.143291Z] 13:33:19     INFO - Initially value is not set for combobox
[task 2017-07-10T13:33:19.145112Z] 13:33:19     INFO - TEST-PASS | accessible/tests/browser/e10s/browser_caching_value.js | Correct value for ['input@id="combobox" node', address: [object HTMLInputElement], role: editcombobox, address: [xpconnect wrapped nsIAccessible]] - 
[task 2017-07-10T13:33:19.148178Z] 13:33:19     INFO - Value should change when @value attribute is updated
[task 2017-07-10T13:33:19.150094Z] 13:33:19     INFO - Buffered messages finished
[task 2017-07-10T13:33:19.159117Z] 13:33:19     INFO - TEST-UNEXPECTED-FAIL | accessible/tests/browser/e10s/browser_caching_value.js | Test timed out -

Comment 8

[Joanmarie Diggs]

Attachment: Patch for try server prior to re-review

The reason for the failure(s) appears to be my not changing a couple of role-based comparisons to IsCombobox() calls in the code related to accessible-event emission. This patch fixes that, and makes the main test failure (which I could reproduce) go away (at least locally).

That said, I'm seeing other failures in both Linux and macOS that are combobox/select related but appear to be independent of my patch. Given the environment differences, before I ask for re-review or commit, I'd like to throw this patch at the official test servers to see if I've addressed all failures.

Comment 9

[Mark Banner (:standard8)]

Pushed to try:

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

Comment 10

[Joanmarie Diggs]

Attachment: Patch for try server prior to re-review

The previous failure in test_expandable.xul was the result of a stupid mistake I made. This patch fixes that.

Comment 11

[Mark Banner (:standard8)]

Pushed to try:

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

Comment 12

[Marco Zehe (:MarcoZ)]

Comment on attachment 8885390 [details] [diff] [review]
Patch for try server prior to re-review

Giving this another r+ since there were some more code changes, and the try run looks OK, with a11y tests now passing onn all desktop platforms that I could see.

Comment 13

[Joanmarie Diggs]

Thanks Marco. I reviewed the failures shown in the try build / treeherder and they all have bug numbers and seem to be known failures, including those marked "unexpected." So I'll set the checkin-needed keyword and hope the commit sticks this time. :)

Thanks again to Mark for pushing my patches to try!

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/53823f1fb9a1
ARIA combobox should map to AXComboBox. r=marcoz

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/53823f1fb9a1