Closed
Bug 725259
Opened 13 years ago
Closed 13 years ago
ARIA role combobox shouldn't allow aria-valueXXX attributes
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: surkov, Assigned: capella)
References
(Blocks 1 open bug)
Details
(Keywords: access, Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++])
Attachments
(1 file)
(deleted),
patch
|
davidb
:
review+
surkov
:
feedback+
|
Details | Diff | Splinter Review |
ARIA combobox doesn't allow aria-valuenow and etc attributes (http://www.w3.org/TR/wai-aria/roles#combobox).
The behavior was introduced by changeset http://hg.mozilla.org/mozilla-central/rev/a55d93c82a17 which is referred to bug 390129. Unfortunately patch of bug 390129 doesn't contain this change and the bug doesn't provide any description of the change. I think we should revert this part.
Note, neither HTML combobox nor XUL combobox don't implement nsIAccessibleValue interface which is consequence of aria-value... attributes.
Reporter | ||
Comment 1•13 years ago
|
||
to fix this bug you should turn off corresponding flag of combobox role (http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsARIAMap.cpp#139)
fix mochitests if they are depended on this behavior
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → markcapella
Assignee | ||
Updated•13 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•13 years ago
|
||
This looks fairly straight-forward ...
The only relevant mochitest I found was test_valuechange.html ... (3) sliders, a scrollbar and a combobox ... the combobox had no aria-value's
I got a good local build ... and mochitests ran fine otherwise.
Attachment #600023 -
Flags: feedback?(surkov.alexander)
Comment 3•13 years ago
|
||
Please add a test that breaks without your patch and passes with your patch.
Reporter | ||
Updated•13 years ago
|
Attachment #600023 -
Flags: feedback?(surkov.alexander) → feedback+
Reporter | ||
Comment 4•13 years ago
|
||
(In reply to David Bolter [:davidb] from comment #3)
> Please add a test that breaks without your patch and passes with your patch.
that'd be nice but personally I ask for mochitest when something wasn't presented but it should be and I'm more polite when something was presented but it shouldn't. If we start to test ARIA for things that shouldn't be there then I bet it must be a big work and it doesn't have a critical value if the presented wrong behavior doesn't confuse ATs.
Assignee | ||
Comment 5•13 years ago
|
||
Comment on attachment 600023 [details] [diff] [review]
nsAriaMap DIFF (v1)
Asking for review+ if that's the case ... then I can do a push to try server via autoland ...
Attachment #600023 -
Flags: review?(surkov.alexander)
Reporter | ||
Comment 6•13 years ago
|
||
Comment on attachment 600023 [details] [diff] [review]
nsAriaMap DIFF (v1)
let's ask David for review since he had a concern
Attachment #600023 -
Flags: review?(surkov.alexander) → review?(bolterbugz)
Reporter | ||
Comment 7•13 years ago
|
||
(In reply to Mark Capella [:capella] from comment #5)
> Comment on attachment 600023 [details] [diff] [review]
> nsAriaMap DIFF (v1)
>
> Asking for review+ if that's the case ... then I can do a push to try server
> via autoland ...
usually I do try server build before asking for review to ensure the patch works :)
Assignee | ||
Comment 8•13 years ago
|
||
I haven't gotten L1 access yet, so I can only do an autoland to TRY by first getting a review+ from an L2 or above ... or (of course) having someone do it for me :-)
Comment 9•13 years ago
|
||
Comment on attachment 600023 [details] [diff] [review]
nsAriaMap DIFF (v1)
Review of attachment 600023 [details] [diff] [review]:
-----------------------------------------------------------------
r=me
Mochitests ran clean locally.
I agree with comment 4.
Thanks for the patch!
Attachment #600023 -
Flags: review?(bolterbugz) → review+
Updated•13 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 10•13 years ago
|
||
Keywords: checkin-needed
Comment 11•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in
before you can comment on or make changes to this bug.
Description
•