Closed
Bug 699017
Opened 13 years ago
Closed 13 years ago
aria-required attribute on file input not read by JAWS
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: damian.chojna, Assigned: surkov)
References
(Blocks 2 open bugs)
Details
Attachments
(1 file, 2 obsolete files)
User Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-GB; rv:1.9.2.23) Gecko/20110920 Firefox/3.6.23 Build ID: 20110920075126 Steps to reproduce: When applying aria-required="true" on a file input, JAWS does not read the required attribute. E.g. <input type="file" aria-required="true"></input> Actual results: The aria-required attribute is ignored when tabbing onto the File browse button Expected results: JAWS should have detected and read that the input field is required.
Assignee | ||
Updated•13 years ago
|
Component: General → Disability Access APIs
Product: Firefox → Core
QA Contact: general → accessibility-apis
Version: 3.6 Branch → Trunk
Assignee | ||
Updated•13 years ago
|
Comment 1•13 years ago
|
||
Also confirmed with NVDA. The text frame parent of the read-only textbox and the button does get the required state, but the child elements don't.
Assignee | ||
Comment 2•13 years ago
|
||
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #575163 -
Flags: review?(trev.saunders)
Comment 3•13 years ago
|
||
Comment on attachment 575163 [details] [diff] [review] patch Question, do you also want to check if the button gets these states? After all that's the one that gets focus, not the read-only textfield.
Assignee | ||
Comment 4•13 years ago
|
||
(In reply to Marco Zehe (:MarcoZ) from comment #3) > Comment on attachment 575163 [details] [diff] [review] [diff] [details] [review] > patch > > Question, do you also want to check if the button gets these states? After > all that's the one that gets focus, not the read-only textfield. I'm not sure about button what would required or invalid states mean on it?
Comment 5•13 years ago
|
||
"required" would indicate to the user that they absolutely have to select a file here. When tabbing, not when navigating with the virtual cursor, this information would not be spoken otherwise. So if we expose "required" or the other universal states on the button, we're OK, but I think we should cover that in the test.
Assignee | ||
Comment 6•13 years ago
|
||
(In reply to alexander surkov from comment #4) > (In reply to Marco Zehe (:MarcoZ) from comment #3) > > Comment on attachment 575163 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review] > > patch > > > > Question, do you also want to check if the button gets these states? After > > all that's the one that gets focus, not the read-only textfield. > > I'm not sure about button what would required or invalid states mean on it? but yes, if the button is what user sees and it's treated as file control then yes, you're right
Assignee | ||
Updated•13 years ago
|
Attachment #575163 -
Flags: review?(trev.saunders)
Comment 7•13 years ago
|
||
Comment on attachment 575163 [details] [diff] [review] patch Review of attachment 575163 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/src/base/nsAccessible.cpp @@ +1600,5 @@ > void > nsAccessible::ApplyARIAState(PRUint64* aState) > { > // Test for universal states first > + *aState |= nsARIAMap::UniversalStatesFor(mContent); Happy to see this logic move to nsARIAMap. Nice.
Assignee | ||
Comment 8•13 years ago
|
||
Attachment #575163 -
Attachment is obsolete: true
Attachment #575191 -
Flags: review?(trev.saunders)
Attachment #575191 -
Flags: review?(marco.zehe)
Comment 9•13 years ago
|
||
Comment on attachment 575191 [details] [diff] [review] patch2 r=me, but please add a link to this bug to >diff --git a/accessible/tests/mochitest/states/test_aria.html Thanks!
Attachment #575191 -
Flags: review?(marco.zehe) → review+
Comment 10•13 years ago
|
||
(In reply to Marco Zehe (:MarcoZ) from comment #5) > "required" would indicate to the user that they absolutely have to select a > file here. When tabbing, not when navigating with the virtual cursor, this > information would not be spoken otherwise. So if we expose "required" or the > other universal states on the button, we're OK, but I think we should cover > that in the test. There's a part of me that says that the At should look at the whole widget to decide if it is required, and so they shouldn't need us to expose these states on the button itself so long as its parent has them. On the other hand all the accessibles are conceptually the same "widget" so if one of them is required it sort of makes sense that all the accessibles have the required state.
Comment 11•13 years ago
|
||
Comment on attachment 575191 [details] [diff] [review] patch2 >+ while (nsStateMapEntry::MapToStates(aContent, &state, gWAIUnivStateMap[index])) >+ ++ index; ++ index is weird I'd prefer index++ but don't really mind ++index. >+ // Inherit ARIA states from input@type="file" suitable for the text field. >+ if (mContent->IsInNativeAnonymousSubtree()) { >+ nsIContent* parentContent = mContent->GetParent(); >+ if (parentContent && parentContent->IsHTML() && >+ parentContent->Tag() == nsGkAtoms::input) { >+ *aState |= nsARIAMap::UniversalStatesFor(parentContent); >+ return; >+ } >+ } to state the obvious I'm not a big fan of copying code, but don't have a better idea for this. > // disabled, too. See bug 429285. > testAriaDisabledTree("group"); > >+ // universal ARIA properties inherited from file input control >+ var fileTextField = getAccessible("fileinput").firstChild; >+ testStates(fileTextField, >+ STATE_BUSY | STATE_UNAVAILABLE | STATE_REQUIRED | STATE_HASPOPUP | STATE_INVALID); >+ var fileBrowseButton = getAccessible("fileinput").lastChild; >+ testStates(fileBrowseButton, >+ STATE_BUSY | STATE_UNAVAILABLE | STATE_REQUIRED | STATE_HASPOPUP | STATE_INVALID); >+ > // offscreen test > testStates("aria_offscreen_textbox", STATE_OFFSCREEN); > > // > // This section tests aria roles on links/anchors for underlying > // nsHTMLLinkAccessible creation. (see closed bug 494807) > // > >@@ -210,17 +218,25 @@ > <div tabindex="0" role="listbox" aria-activedescendant="item1"> > <div role="option" id="item1">Item 1</div> > <div role="option" id="item2">Item 2</div> > <div role="option" id="item3">Item 3</div> > <div role="option" id="item4">Item 4</div> > </div> > <div role="slider" tabindex="0">A slider</div> > </div> >- >+ >+ <!-- universal ARIA properties should be inherited by text field of file input --> >+ <input type="file" id="fileinput" >+ aria-busy="true" >+ aria-disabled="true" >+ aria-required="true" >+ aria-haspopup="true" >+ aria-invalid="true"> >+ > <div id="offscreen_log" role="log" class="offscreen"> > <div id="aria_offscreen_textbox" role="textbox" aria-readonly="true">This text should be offscreen</div> > </div> > > <a id="aria_menuitem_link" role="menuitem" href="foo">menuitem</a> > <a id="aria_button_link" role="button" href="foo">button</a> > <a id="aria_checkbox_link" role="checkbox" href="foo">checkbox</a> >
Comment 12•13 years ago
|
||
Another question, what do we want to do about state change events here?
Assignee | ||
Comment 13•13 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #10) > (In reply to Marco Zehe (:MarcoZ) from comment #5) > > "required" would indicate to the user that they absolutely have to select a > > file here. When tabbing, not when navigating with the virtual cursor, this > > information would not be spoken otherwise. So if we expose "required" or the > > other universal states on the button, we're OK, but I think we should cover > > that in the test. > > There's a part of me that says that the At should look at the whole widget > to decide if it is required, and so they shouldn't need us to expose these > states on the button itself so long as its parent has them. On the other > hand all the accessibles are conceptually the same "widget" so if one of > them is required it sort of makes sense that all the accessibles have the > required state. that's right but this case is special since no standard way for AT to say this is a widget, they could deal with upload button only. (In reply to Trevor Saunders (:tbsaunde) from comment #12) > Another question, what do we want to do about state change events here? good point, I'm tempting to say we should fire events for every underlying control in this case.
Assignee | ||
Comment 14•13 years ago
|
||
(In reply to alexander surkov from comment #13) > (In reply to Trevor Saunders (:tbsaunde) from comment #12) > > Another question, what do we want to do about state change events here? > > good point, I'm tempting to say we should fire events for every underlying > control in this case. create class for file input control and make it fire event on descendants when it handles statechange event sounds ok?
Comment 15•13 years ago
|
||
(In reply to alexander surkov from comment #14) > (In reply to alexander surkov from comment #13) > > > (In reply to Trevor Saunders (:tbsaunde) from comment #12) > > > Another question, what do we want to do about state change events here? > > > > good point, I'm tempting to say we should fire events for every underlying > > control in this case. > > create class for file input control and make it fire event on descendants > when it handles statechange event sounds ok? I have no immediate objection to that.
Updated•13 years ago
|
Attachment #575191 -
Flags: review?(trev.saunders) → review+
Assignee | ||
Comment 16•13 years ago
|
||
+ fire events
Attachment #575191 -
Attachment is obsolete: true
Attachment #577212 -
Flags: review?(trev.saunders)
Attachment #577212 -
Flags: review?(marco.zehe)
Assignee | ||
Comment 17•13 years ago
|
||
Comment on attachment 577212 [details] [diff] [review] patch3 additional review from Robert for layout part.
Attachment #577212 -
Flags: review?(roc)
Comment 18•13 years ago
|
||
Comment on attachment 577212 [details] [diff] [review] patch3 r=me with these nits: >+ this.invoke = function stateChangeOnFileInput_inovke() Not important for functionality, but please correct spelling of "_invoke". Also, there's still uncommented debug statement just below this invoker in the same file.
Attachment #577212 -
Flags: review?(marco.zehe) → review+
Assignee | ||
Comment 19•13 years ago
|
||
Robert, the layout part is trivial, just making fileinput frame to create an accessible of different type.
Attachment #577212 -
Flags: review?(roc) → review+
Comment 20•13 years ago
|
||
Comment on attachment 577212 [details] [diff] [review] patch3 r=me with comment per irc
Attachment #577212 -
Flags: review?(trev.saunders) → review+
Assignee | ||
Comment 21•13 years ago
|
||
inbound http://hg.mozilla.org/integration/mozilla-inbound/rev/247c27bf0888
Comment 22•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/247c27bf0888
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Assignee | ||
Updated•13 years ago
|
Flags: in-litmus+
Assignee | ||
Updated•13 years ago
|
Flags: in-litmus+ → in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•