Closed
Bug 675879
Opened 13 years ago
Closed 12 years ago
clean up nsIAccessible action methods
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: surkov, Assigned: oussamabadr)
References
(Blocks 1 open bug)
Details
(Keywords: access, Whiteboard: [good first bug][mentor=trev.saunders@gmail.com][lang=c++])
Attachments
(1 file, 3 obsolete files)
23.55 KB,
patch
|
surkov
:
review+
|
Details | Diff | Splinter Review |
numActions -> actionCount not sure whether we should rename getActionName to getActionNameAt and others.
Reporter | ||
Comment 1•12 years ago
|
||
Trevor, what do you think about adding 'at' postfix to method names?
Comment 2•12 years ago
|
||
(In reply to alexander :surkov from comment #1) > Trevor, what do you think about adding 'at' postfix to method names? I don't have much of an opinion either way. Treating the actions like an array feels funny to me in general, but given windows and linux both want it it that way I guess we should go with that api like that.
Reporter | ||
Comment 3•12 years ago
|
||
Yep, it's sort of weird. So let's do not do extra work we aren't sure about. So do numActions -> actionCount for now.
Whiteboard: [good first bug][mentor=trev.saunders@gmail.com][lang=c++]
Assignee | ||
Comment 4•12 years ago
|
||
Alexander, Can you put more details? are you talking about renaming action methods in nsIAccessible( getActionName; getActionDescription; doAction) by putting "At" as a postfix ?
Reporter | ||
Comment 5•12 years ago
|
||
(In reply to Oussama BADR from comment #4) > Alexander, Can you put more details? > are you talking about renaming action methods in nsIAccessible( > getActionName; getActionDescription; doAction) by putting "At" as a postfix > ? no, just change numActions to actionCount (per comment #3)
No longer blocks: cleanxpcoma11y
Assignee | ||
Comment 6•12 years ago
|
||
Ok, you can assign me in, thanks in advance!
Reporter | ||
Updated•12 years ago
|
Assignee: nobody → oussamabadr
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #625902 -
Flags: review?(surkov.alexander)
Reporter | ||
Comment 8•12 years ago
|
||
Comment on attachment 625902 [details] [diff] [review] Patch to fix Bug 675879. Review of attachment 625902 [details] [diff] [review]: ----------------------------------------------------------------- you need to change UUID of nsIAccessible interface and you didn't changed implementation (look for GetNumActions method)
Attachment #625902 -
Flags: review?(surkov.alexander)
Assignee | ||
Comment 9•12 years ago
|
||
Attachment #626419 -
Flags: review?(surkov.alexander)
Reporter | ||
Comment 10•12 years ago
|
||
Comment on attachment 626419 [details] [diff] [review] Patch V2 to fix Bug 675879. Review of attachment 626419 [details] [diff] [review]: ----------------------------------------------------------------- you didn't change all instances of 'numActions' in mochitests folder (for example, actions/test_general.html), canceling review, I need to look at next patch. ::: accessible/public/nsIAccessible.idl @@ +289,5 @@ > > /** > * The number of accessible actions associated with this accessible > */ > + readonly attribute PRUint8 actionCount; you didn't change the uuid of nsIAccessible interface, please do that ::: accessible/src/base/nsAccessible.cpp @@ +1867,5 @@ > > *aActionCount = ActionCount(); > return NS_OK; > } > + whitespace on empty line
Attachment #626419 -
Flags: review?(surkov.alexander)
Reporter | ||
Updated•12 years ago
|
Attachment #625902 -
Attachment is obsolete: true
Comment 11•12 years ago
|
||
Over the weekend an issue occurred with the BMO database which resulted in duplication of dependencies. The dependency issue may have resulted in "Depends On" and "Blocks" values being removed while updating a bug. This issue should now be resolved, however dependencies may need to be manually restored to some bugs. This bug had dependencies removed during the failure period and will need verification that the dependency removal(s) were intentional. Please help out by taking a look at this bug and adding anything back that was mistakenly removed.
Assignee | ||
Comment 12•12 years ago
|
||
> > /**
> > * The number of accessible actions associated with this accessible
> > */
> > + readonly attribute PRUint8 actionCount;
>
> you didn't change the uuid of nsIAccessible interface, please do that
I didn't match what I have to do for UUID, I have found this line for UUID in nsIAccessible interface :
...
[scriptable, uuid(46d422d1-c92f-4536-bdef-f77bc8350ec7)]
interface nsIAccessible : nsISupports
...
really sorry, but I have lost my fingers in!!!
Assignee | ||
Updated•12 years ago
|
Attachment #626419 -
Attachment is obsolete: true
Reporter | ||
Comment 13•12 years ago
|
||
(In reply to Oussama BADR from comment #12) > [scriptable, uuid(46d422d1-c92f-4536-bdef-f77bc8350ec7)] > interface nsIAccessible : nsISupports yes, you need to replace this uuid on new one
Assignee | ||
Comment 14•12 years ago
|
||
Attachment #627150 -
Flags: review?(surkov.alexander)
Assignee | ||
Comment 15•12 years ago
|
||
(In reply to alexander :surkov from comment #10) > Comment on attachment 626419 [details] [diff] [review] > ----------------------------------------------------------------- > > you didn't change all instances of 'numActions' in mochitests folder (for > example, actions/test_general.html), canceling review, I need to look at > next patch. > to check out 'numActions' instances in all accessible folders, I have used 'grep' command in my Ubuntu OS instead of Mozilla mxr web tool, the last one didn't display all occurrences!
Reporter | ||
Comment 16•12 years ago
|
||
Comment on attachment 627150 [details] [diff] [review] Patch V3 to fix Bug 675879. Review of attachment 627150 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/tests/mochitest/actions/test_general.html @@ +45,5 @@ > testActions(actionsArray); > > getAccessible("onclick_img").takeFocus(); > +is(getAccessible("link1").actionCount, 1, "links should have one action"); > +is(getAccessible("link2").actionCount, 1, "link with onclick handler should have 1 action"); please make proper indentation while you are here
Attachment #627150 -
Flags: review?(surkov.alexander) → review+
Assignee | ||
Comment 17•12 years ago
|
||
...with right indentations!
Attachment #627171 -
Flags: review?(surkov.alexander)
Reporter | ||
Comment 18•12 years ago
|
||
Comment on attachment 627171 [details] [diff] [review] Patch V4 to fix Bug 675879. you don't need to rerequest review for minor changes. I'll land the patch, thank you for the fix
Attachment #627171 -
Flags: review?(surkov.alexander) → review+
Assignee | ||
Comment 19•12 years ago
|
||
welcome! but, why did I change the UUID of nsIAccessible interface?
Reporter | ||
Comment 20•12 years ago
|
||
(In reply to Oussama BADR from comment #19) > welcome! > > but, why did I change the UUID of nsIAccessible interface? Let's you have a c++ extension that use that interface, you do query interface by passing that UUID then you get back an interface pointer and you call a method. Since interface was changed then you may end up with calling wrong code or crash. You need to read how methods calls are implemented in c++ for more information. If you changed UUID then that QueryInterface will return null pointer.
Assignee | ||
Comment 21•12 years ago
|
||
thks for explanations!! may be I should find out a new bug, but (I think) I'm not ready to do it, so please led me to the next bug!
Reporter | ||
Updated•12 years ago
|
Attachment #627150 -
Attachment is obsolete: true
Reporter | ||
Updated•12 years ago
|
Blocks: cleanxpcoma11y
Reporter | ||
Comment 22•12 years ago
|
||
(In reply to Oussama BADR from comment #21) > may be I should find out a new bug, but (I think) I'm not ready to do it, so > please led me to the next bug! thank you, I cc'ed you on bug 672514.
Comment 23•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ca31a899ce4e
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in
before you can comment on or make changes to this bug.
Description
•