Opened 8 years ago
Closed 8 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)
Change History (12)
Changed 8 years ago by
Attachment: | _FormValueWidget.js.patch added |
---|
comment:1 Changed 8 years ago by
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 8 years ago by
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 8 years ago by
Milestone: | tbd → 1.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 8 years ago by
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 8 years ago by
Attachment: | _FormValueMixin.js.patch added |
---|
remove aria-readonly altogether since native input readonly semantics are enough
comment:5 Changed 8 years ago by
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 8 years ago by
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 8 years ago by
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.
removes aria-readonly from form dijits without role=textbox, please proxy commit for michael billau CCLA on file with IBM