Opened 7 years ago

Closed 7 years ago

#16091 closed defect (fixed)

[PATCH][CCLA] dijit.Form: invalid aria-readonly attribute on form elements

Reported by: mikeb Owned by: Douglas Hays
Priority: undecided Milestone: 1.8.2
Component: Dijit - Form Version: 1.8.0
Keywords: Cc:
Blocked By: Blocking:

Description

In Form dijits, you can use

set("readOnly",...)

to set an element as read-only. _WidgetBase also takes care of setting "aria-readonly" on the focusNode when we do this.

The problem is that "aria-readonly" is only valid for elements with role="grid|gridcell|textbox". For dijit.Form widgets, often times we exclude the role attribute because it shouldn't be used when native HTMl semantics are enough. This patch removes the aria-readonly attribute when using set("readOnly") on form Widgets that do not have role="textbox." Tested it against TextBox?/validation dijits, Select, Combobox/FilteringSelect?, and Date/Time? textbox.

Attachments (2)

_FormValueWidget.js.patch (657 bytes) - added by mikeb 7 years ago.
removes aria-readonly from form dijits without role=textbox, please proxy commit for michael billau CCLA on file with IBM
_FormValueMixin.js.patch (413 bytes) - added by mikeb 7 years ago.
remove aria-readonly altogether since native input readonly semantics are enough

Download all attachments as: .zip

Change History (12)

Changed 7 years ago by mikeb

Attachment: _FormValueWidget.js.patch added

removes aria-readonly from form dijits without role=textbox, please proxy commit for michael billau CCLA on file with IBM

comment:1 Changed 7 years ago by bill

Which code in _WidgetBase sets aria-readonly? I don't see any _setReadOnlyAttr() method there. The only _setReadOnlyAttr() I see is in _FormValueMixin. Perhaps you should be changing that method instead of adding a new _setReadOnlyAttr().

Also, the !role condition of if( !role || role!=="textbox" ) has no purpose.

comment:2 Changed 7 years ago by mikeb

Hi Bill. Doug also brought this up and you guys are right. Somehow I missed _setReadOnlyAttr() in _FormValueMixing. I made the change to _FormValueMixin instead. I tested it with the above dijits and it solves the violation. I'm not sure what I was thinking with !role...but thanks for pointing that out.

comment:3 Changed 7 years ago by Douglas Hays

Milestone: tbd1.8.2

mikeb, why does aria-readonly ever need to be set? focusNode should have readOnly attribute set and I think the screen-reader will see that and act appropriately.

comment:4 Changed 7 years ago by mikeb

doughays, the only reason we would need aria-readonly is if there is a Form dijit that can be set readOnly whose focusNode is not an <input> element. I have not been able to find any dijit like this, so I believe you are right, we should just remove aria-readonly altogether. I tested the Form dijits with JAWS and found that in readOnly mode, 'aria-readonly' doesn't seem to affect JAWS behavior one way or the other.

Changed 7 years ago by mikeb

Attachment: _FormValueMixin.js.patch added

remove aria-readonly altogether since native input readonly semantics are enough

comment:5 Changed 7 years ago by Douglas Hays

From mikeb,
CheckBox should not have aria-readonly (JAWS bug, never says readonly).
Select should have aria-readonly to get JAWS to say readonly, but generates RPT violation since role=listbox. (It might be better to wait for the RPT violation to be corrected)
ComboBox should have aria-readonly (JAWS bug since role=textbox and should work natively). (I'm not convinced this is really needed)

comment:6 Changed 7 years ago by mikeb

I did some more testing with JAWS and aria-readonly attribute:

dijit only "readonly" "aria-readonly" Notes
Select JAWS:"listbox closed" JAWS: "listbox closed readonly" WAI-ARIA spec violation since role="listbox" cant have aria-readonly, however it increases a11y and useability
ComboBox? JAWS:"Edit Combo {Value} to change the selection use the arrow keys or type the selection" JAWS: "Edit Combo READONLY. {Value}" With aria-readonly, sometimes JAWS will not say "READONLY". This is a JAWS bug but is probably ok since aria-readonly causes JAWS to NEVER speak the instructions.
CheckBox? JAWS:"Checkbox checked." JAWS:"Checkbox checked" Seems to be no way to indicate to JAWS that a checkbox is "readonly", perhaps because native checkboxes can't be readonly?
TextBox? JAWS:"{Label} READONLY edit {content}" JAWS:"{Label} READONLY edit {content} adding aria-readonly is invalid because it doesn't have a role (uses native semantics)
TextArea? JAWS:"{Label} READONLY edit {content}" JAWS:"{Label} READONLY edit {content} adding aria-readonly is invalid because it doens't have a role (uses native semantics)
DateTextBox?JAWS:"Edit combo {value} To set the value use the...""Edit combo READONLY {value} readonly edit, To set the value..."With aria-readonly, JAWS only says "READONLY" once in a while (but it never speaks it without aria-readonly attr)

Based on this information above, I think that the best thing to do is to allow aria-readonly on elements where the focusNode has a role (any role). This means at least Select, ComboBox?, and FilteringSelect? will have aria-readonly. I think the ComboBox? demonstrates a clear JAWS bug (I will follow up with FS), but I think we should include aria-readonly for ComboBox? anyway until the JAWS bug gets resolved.

We will just have to live with the fact that Select dijit will be invalid according to the spec, since I think removing it would greatly reduce accessibility of the dijit and goes against the spirit of WAI-ARIA.

We shouldn't add aria-readonly to CheckBox? since it doesn't do anything for JAWS and is in violation of the spec (since no role).

comment:7 Changed 7 years ago by mikeb

I talked this out with an accessibility expert and it was suggested to remove aria-readonly from Select box. doughays I agree that the ComboBox? doesn't need it either, since we apply readonly to the actual native input box. So I think that the _FormValueMixin.js.patch is good.

comment:8 Changed 7 years ago by Douglas Hays

mikeb, should aria-readonly be removed from CheckBox?

comment:9 in reply to:  8 Changed 7 years ago by mikeb

Replying to doughays:

mikeb, should aria-readonly be removed from CheckBox?

Yes it should be removed, since aria-readonly on CheckBox? doesn't affect JAWS, and it is also not allowed on role=checkbox based on the spec.

comment:10 Changed 7 years ago by Douglas Hays

Resolution: fixed
Status: newclosed

In [29810]:

Fixes #16091. Remove aria-readonly since it's only valid for role=textbox and screen readers pick it up natively from the DOM attribute for TextBox? widgets.

Note: See TracTickets for help on using tickets.