Closed
Bug 737379
Opened 12 years ago
Closed 12 years ago
change Comments to roles::ROLENAME
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: capella, Assigned: Infinity)
References
(Blocks 1 open bug)
Details
(Whiteboard: [good first bug][mentor=markcapella@twcny.rr.com][lang=c++])
Attachments
(1 file, 1 obsolete file)
20.92 KB,
patch
|
surkov
:
review+
capella
:
feedback+
|
Details | Diff | Splinter Review |
INCLUDE file has comments that could be made more clear.
See ::: accessible/src/mac/nsRoleMap.h and the below for example ...
@@ +166,5 @@
> NSAccessibilityGroupRole, // ROLE_GRID_CELL
> NSAccessibilityGroupRole, // ROLE_EMBEDDED_OBJECT
> NSAccessibilityGroupRole, // ROLE_NOTE
> NSAccessibilityGroupRole, // ROLE_FIGURE
> + NSAccessibilityCheckBoxRole, // ROLE_CHECK_RICH_OPTION
Reporter | ||
Updated•12 years ago
|
Assignee: nobody → junky.argonaut
Reporter | ||
Comment 1•12 years ago
|
||
At the bottom of the file, accessible/src/mac/nsRoleMap.h has comments like: // ROLE_CHECK_RICH_OPTION At the bottom of accessible/src/atk/nsRoleMap.h has comments like: // roles::CHECK_RICH_OPTION 125 for the matching role definition. Make mac/ version comments look like the atk/ version Usually you'll just change // ROLE_foo to be // roles::foo
Updated•12 years ago
|
Whiteboard: [good first bug] → [good first bug][mentor=markcapella@twcny.rr.com][lang=c++]
Assignee | ||
Comment 2•12 years ago
|
||
Reporter | ||
Comment 3•12 years ago
|
||
In the patch, you inserted two lines .... (title then a blank line) ... before the first role ... please remove them + // Cross Platform Roles # Description + + NSAccessibilityUnknownRole, // roles::NOTHING 0 + NSAccessibilityUnknownRole, // roles::TITLEBAR 1 (irrelevant on OS X; windows are always native.) My version of the nsrolemap.h file (before any changes you made) ends with these three entries ... your seems to have lost the CHECK_RICH_ OPTION entry ... did you start from the right file? NSAccessibilityGroupRole, // ROLE_FIGURE NSAccessibilityCheckBoxRole, // ROLE_CHECK_RICH_OPTION @"ROLE_LAST_ENTRY" // ROLE_LAST_ENTRY. bogus role th Let me know ...
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #610709 -
Attachment is obsolete: true
Attachment #611249 -
Flags: feedback?
Reporter | ||
Comment 5•12 years ago
|
||
Looks good with the previous requested changes, and the proper start file .. You did a local build with this new .h file and that worked ok? You ran the whole mochitest-a11y suite locally successfully?
Reporter | ||
Updated•12 years ago
|
Attachment #611249 -
Flags: review?(surkov.alexander)
Attachment #611249 -
Flags: feedback?
Attachment #611249 -
Flags: feedback+
Comment 6•12 years ago
|
||
Comment on attachment 611249 [details] [diff] [review] Updated Patch1 Review of attachment 611249 [details] [diff] [review]: ----------------------------------------------------------------- r=me, thank you (I'll fix nit before landing) ::: accessible/src/mac/nsRoleMap.h @@ +88,5 @@ > + NSAccessibilityButtonRole, // roles::PUSHBUTTON 43 > + NSAccessibilityCheckBoxRole, // roles::CHECKBUTTON 44 > + NSAccessibilityRadioButtonRole, // roles::RADIOBUTTON 45 > + NSAccessibilityPopUpButtonRole, // roles::COMBOBOX 46 > + NSAccessibilityPopUpButtonRole, // roles::DROPLIST. 47 nit: get rid dot in the end
Attachment #611249 -
Flags: review?(surkov.alexander) → review+
Updated•12 years ago
|
Target Milestone: --- → mozilla14
Comment 8•12 years ago
|
||
This patch has landed in mozilla-central for Firefox 14, and will be included in tomorrow's nightly build. Thank you! https://hg.mozilla.org/mozilla-central/rev/0296cdb4cb2b
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 9•12 years ago
|
||
Its been a pleasure to work with you guys! Glad to see it out there.
Comment 10•12 years ago
|
||
(In reply to Arjun from comment #9) > Its been a pleasure to work with you guys! Glad to see it out there. yep, nice work, thank you. If you like then please take another bug https://bugzilla.mozilla.org/buglist.cgi?emailtype1=exact;resolution=---;email1=nobody%40mozilla.org;emailassigned_to1=1;component=Disability%20Access%20APIs;product=Core;status_whiteboard=mentor;list_id=2709998;status_whiteboard_type=allwordssubstr;query_format=advanced
Assignee | ||
Comment 11•12 years ago
|
||
You are welcome! I will check the list out and let you know if I am interested in anything.
Comment 12•12 years ago
|
||
(In reply to Arjun from comment #11) > You are welcome! > I will check the list out and let you know if I am interested in anything. ok, thanks again!
Comment 13•12 years ago
|
||
And this patch reverted a change made in changeset: 90162:fc9658fae44b *sigh* I was wondering why suddenly it was broken. Bug 743114
You need to log in
before you can comment on or make changes to this bug.
Description
•