Closed Bug 1044431 Opened 10 years ago Closed 10 years ago

Listbox owned by combobox has the wrong role

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla36

People

(Reporter: athena, Assigned: athena)

References

Details

Attachments

(1 file, 1 obsolete file)

missing-else-combobox_list.patch
2.96 KB, patch
MarcoZ
: review+
MarcoZ
: checkin+
Details | Diff | Splinter Review
Attached patch missing-else-combobox_list (obsolete) — — Splinter Review 
Looks like a simple typo. There's code in place that tries to make it return roles::COMBOBOX_LIST, but we return early.

(Basing my understanding of owned by, etc, on the comment in 6baaadf1ddd0)
Assignee: nobody → athena
Status: NEW → ASSIGNED
Attachment #8462987 - Flags: review?(dbolter)
Comment on attachment 8462987 [details] [diff] [review]
missing-else-combobox_list

Athena, thanks for this! Marco graciously said he'd review this one, so transferring r?
Attachment #8462987 - Flags: review?(dbolter) → review?(marco.zehe)
Attachment #8462987 - Attachment is patch: true
Comment on attachment 8462987 [details] [diff] [review]
missing-else-combobox_list

Review of attachment 8462987 [details] [diff] [review]:
-----------------------------------------------------------------

Also, you need to change the reviewer. :) And please make sure that the next patch is marked as such by checking the appropriate checkbox. :)

Great find, by the way!

::: accessible/tests/mochitest/role/test_general.html
@@ +81,5 @@
>      SimpleTest.waitForExplicitFinish();
>      addA11yLoadEvent(doTests);
>    </script>
>  </head>
>  <body>

Please also add a reference to this bug in the bugs section below.
Attachment #8462987 - Flags: review?(marco.zehe) → review+
Thanks, and here you go!
Attachment #8462987 - Attachment is obsolete: true
Attachment #8463453 - Flags: review?(marco.zehe)
Comment on attachment 8463453 [details] [diff] [review]
missing-else-combobox_list.patch

r=me!

Try run is at: https://tbpl.mozilla.org/?tree=Try&rev=4c719d319d2a
Attachment #8463453 - Flags: review?(marco.zehe) → review+
Massive apologies, I just realized this never got committed! What happens here? It still applies and it's trivial enough to be unlikely to affect anything else, but it's been a really long time unmerged...
I think you can just request checkin-needed. If it still applies cleanly, it is unlikely to cause any breakage now. But if you feel more confident, and have try access, you can kick one off before doing that.
Comment on attachment 8463453 [details] [diff] [review]
missing-else-combobox_list.patch

Thanks for the advice, marking for checkin.
Attachment #8463453 - Flags: checkin?(mzehe)
Comment on attachment 8463453 [details] [diff] [review]
missing-else-combobox_list.patch

Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/8670dbe3bfcc
Attachment #8463453 - Flags: checkin?(mzehe) → checkin+
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/8670dbe3bfcc
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
See Also: → 1795203
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: