Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#15885 closed defect (fixed)

Color popup at test_Button example doesn't use aria-owns

Reported by: apinheiro Owned by: bill
Priority: undecided Milestone: 1.8.4
Component: Accessibility Version: 1.8.0
Keywords: Cc:
Blocked By: Blocking:

Description

At the example page [1] there is a button to select the Color (label "Color"), that it is used to show a popup, in order to select the colors. Looking at the html code, and at the DOM generated in Webkit HTML engine, the popup is not a descendant of that button. That button is using the aria-haspopup property. From the documentation of that property [2]:

"Value Description

true: Indicates the object has a popup, either as a descendant or pointed to by aria-owns."

But in this case, aria-owns is not used. Using that attribute could be useful to relate the button with his popup, as mentioned in a recent WebKit bug [3].

FWIW, I found some examples using both aria-haspopup and aria-owns [4]

[1] http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/tests/form/test_Button.html

[2] http://www.w3.org/TR/wai-aria/states_and_properties#aria-haspopup

[3] https://bugs.webkit.org/show_bug.cgi?id=73822

[4] http://www.punkchip.com/2010/11/aria-basic-findings/

Attachments (6)

DropDownButton_aria_owns_role_presentation.patch (973 bytes) - added by mikeb 7 years ago.
adds aria-owns to the focus node so only the node with aria-haspopup gets aria-owns (Please proxy commit for Michael Billau CCLA on file with IBM)
_HasDropDown.js.patch (316 bytes) - added by mikeb 7 years ago.
patch _hasDropDown to add aria-owns to whichever element "owns" the drop down
_hasDropDown patch with aria-owns.patch (38.4 KB) - added by mikeb 7 years ago.
adds aria-owns to hasDropDown, fixes test pages, please proxy commit for Michael Billau CCLA on file with IBM
_hasDropDown patch2 consolidated.patch (36.2 KB) - added by mikeb 7 years ago.
adds aria-owns to _hasDropDown, sets it on focusNode for everything except ComboBox? and FilteringSelect?
_hasdropDown patch3 use popupNode for all.patch (38.1 KB) - added by mikeb 7 years ago.
move popupStateNode on dropDownBox, DropDownButton?, and Select to the node with role so aria-expanded and aria-owns is valid, please proxy commit for Michael Billau
ticket-15885-backport.patch (6.5 KB) - added by mikeb 6 years ago.
Backport drop down accessibility fixes to 1.8 branch, please commit for Michael Billau CCLA(IBM)

Download all attachments as: .zip

Change History (22)

comment:1 Changed 7 years ago by joanmarie

(I am sure that adding myself to the CC without commenting is obvious, but am just not seeing it. Sorry for the spam!)

Changed 7 years ago by mikeb

adds aria-owns to the focus node so only the node with aria-haspopup gets aria-owns (Please proxy commit for Michael Billau CCLA on file with IBM)

comment:2 Changed 7 years ago by Douglas Hays

The patch should probably also address these widgets: ComboButton? DropDownBox? Select, each of which define aria-haspopup

comment:3 Changed 7 years ago by bill

It seems like you could put general code in _HasDropDown.js that handles this for all cases.

Changed 7 years ago by mikeb

Attachment: _HasDropDown.js.patch added

patch _hasDropDown to add aria-owns to whichever element "owns" the drop down

comment:4 Changed 7 years ago by mikeb

Hey Bill, that's what I was thinking too. I wasn't sure where the best place to put the code was. I tried in the postCreate, but we need to wait until the dropDown has been created and the dom node is on the page. Otherwise, aria-owns will have an invalid id pointing to a non- existing dom node (a separate accessibility violation.)

Putting the code in the loadAndOpenDropDown method means that aria-owns doesn't get applied to the dom node until the dropDown is activated (and the dropDown is created and added to the page in all cases.) This means when the page loads initially it is technically incorrect, since domNodes will have aria-haspopup but will not have the associated aria-owns. However, since the dropDown node doesn't necessarily get created on page initialization, (or ever,) it is possible that we would never know what to fill in for aria-owns. I don't see anything in the spec that deals with this (aria attributes that require the ID's of as-of-yet-to-be-declared DOM nodes)

comment:5 Changed 7 years ago by bill

Hmm. Probably you can't set aria-haspopup in the same way, as the dropdown is created? It seems like aria-haspopup needs to be there before the user focuses the node, so that the screen reader can announce that the widget will show a dropdown (if the user presses the down arrow).

comment:6 Changed 7 years ago by mikeb

After chatting with some accessibility experts, it appears that we have two options:

  1. Assign the aria-owns when you create the pop-up menu. The reason for the aria-owns is so that the user can walk to the parent to find the associate button. The menu is not in the accessibility tree yet. That is no different from when you have a menu with display:none.
  1. Have the aria-owns point to an id of an empty container that is hidden. This is still light weight as the menu is not populated until you drop it down.

I decided to implement #1; the dijits will have aria-haspopup and then HasDropDown? assigns aria-owns to them when the drop down is opened. So, aria-owns will not be assigned initially for any of the drop down dijits: ComboBox?, FilteringSelect?, Select, DropDownButton?, ComboButton?, ScrollingTabContainer?, _DateTimeTextBox. Had to add some additional code in ComboButtonMixin? and _DateTimeTextBox to move the aria-expanded and aria-owns attributes to a node with the correct role.

One interesting thing I noticed was that for DropDownButton? and ComboButton?, it seems that the drop down object is actually created and appended to the bottom of the page even if the drop down is never clicked on. We might be able to find a way to set aria-owns right away for these dijits, but based on #1 above it doesn't seem like this would increase accessibility at all.

Last edited 7 years ago by mikeb (previous) (diff)

Changed 7 years ago by mikeb

adds aria-owns to hasDropDown, fixes test pages, please proxy commit for Michael Billau CCLA on file with IBM

comment:7 Changed 7 years ago by bill

Owner: changed from mikeb to apinheiro
Status: newpending

Hi Mike - generally that looks good but I'm not comfortable having three separate places that set aria-owns and aria-expanded.

Can you use this._popupStateNode instead of this.focusNode?

comment:8 Changed 7 years ago by mikeb

this._popupStateNode usually doesn't have a role, so using it only helps in the case of ComboButton?.

It looks like it goes on the focusNode for all of the listed dijits unless the focusNode has role="textbox" (ComboBox? and FilteringSelect? - also Select, but for Select the focusNode==domNode). If the focusNode has any other role, it should go on the focusNode.

Changed 7 years ago by mikeb

adds aria-owns to _hasDropDown, sets it on focusNode for everything except ComboBox? and FilteringSelect?

comment:9 Changed 7 years ago by bill

That patch looks better, but I'm skeptical of your claim that this._popupStateNode doesn't work. What's an example of a widget where it doesn't work?

comment:10 Changed 7 years ago by mikeb

dijit role of _popupStateNode is correct? notes
ComboButton? button correct _popupStateNode,focusNode,_buttonNode are all the same element
DropDownButton? no role incorrect _popupStateNode is the <span> with the label. Has no role which makes aria-expanded invalid. Should be placed on the role=button, which is focusNode, titleNode, or _arrowWrapperNode.
Select presentation incorrect role=presentation cannot have aria-expanded. The node is a div that has a parent with role=lisbox and has attach points: _buttonNode, tableNode, or focusNode. Since we expand this dijit, "the combobox role may be more appropriate", but I don't think so since it doesnt auto complete and isn't editable.
ComboBox?,
FilteringSelect?,
DateTimeTextBox?
presentation incorrect node is the down pointing triangle "buttton" container. We should probably change this to role="button" and it would work and also be more in line with aria best practices: http://www.w3.org/TR/wai-aria-practices/#combobox
ScrollingTabContainer? no role incorrect Would be correct if it was role=button on the drop down arrow

So to summarize, with some minor role changes (adding role=button) we can use _popupStateNode for everything execpt DropDownButton? and Select.

I think that we could also move _popupStateNode on DropDownButton?, DropDownBox?, and Select to the appropriate elements. I tried this out, did not see anything wrong, so I created a new patch with this fix.

Changed 7 years ago by mikeb

move popupStateNode on dropDownBox, DropDownButton?, and Select to the node with role so aria-expanded and aria-owns is valid, please proxy commit for Michael Billau

comment:11 Changed 7 years ago by bill

Milestone: tbd1.9
Owner: changed from apinheiro to bill
Status: pendingassigned

OK thanks, that makes sense to me.

I'm unclear why this._popupStateNode existed originally. _HasDropDown.js sets a "popupActive" attribute on this._popupStateNode but no one seems to use it. In any case, seems like using it for the popup related aria roles makes sense.

comment:12 Changed 7 years ago by bill

PS: I'm surprised that aria-labelledby is set on this.domNode rather than this.focusNode. I see that's indeed the case (for example for ComboBox) but not sure how that works for screen readers.

comment:13 Changed 7 years ago by bill

Resolution: fixed
Status: assignedclosed

In [29793]:

Fix aria roles for drop downs, changes from Mike Billau (IBM, CCLA), thanks!

Fixes #15885 !strict.

comment:14 Changed 6 years ago by mikeb

Hello, would it be possible to backport this to the 1.8 branch? I created a patch with all the changes in [29793]. There are additional changes to some of these files (_HasDropDown, test_Select) that are due to [30053]. I plan to backport [30053] but might wait until this gets checked in first (or I'll just apply this patch and then backport [30053] so that it merges cleanly).

Changed 6 years ago by mikeb

Attachment: ticket-15885-backport.patch added

Backport drop down accessibility fixes to 1.8 branch, please commit for Michael Billau CCLA(IBM)

comment:15 Changed 6 years ago by Douglas Hays

Milestone: 1.91.8.4

Refs #15885. Backport [29793] to 1.8.

comment:16 Changed 6 years ago by Douglas Hays

In [30310]

Note: See TracTickets for help on using tickets.