Last Comment Bug 740750 - Use nsITreeView* instead of nsCOMPtr<nsITreeView> as followup to bug#739524
: Use nsITreeView* instead of nsCOMPtr<nsITreeView> as followup to bug#739524
Status: RESOLVED FIXED
[good first bug][mentor=surkov.alexan...
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Nagarjuna Varma [:Infinity]
:
: alexander :surkov
Mentors:
https://bugzilla.mozilla.org/show_bug...
Depends on: 739524
Blocks: a11yperf
  Show dependency treegraph
 
Reported: 2012-03-30 02:59 PDT by Mark Capella [:capella]
Modified: 2012-04-30 06:51 PDT (History)
3 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patchv0 (5.72 KB, patch)
2012-04-26 09:14 PDT, Nagarjuna Varma [:Infinity]
no flags Details | Diff | Splinter Review
Patchv1 (6.44 KB, patch)
2012-04-26 11:11 PDT, Nagarjuna Varma [:Infinity]
surkov.alexander: review+
Details | Diff | Splinter Review

Description Mark Capella [:capella] 2012-03-30 02:59:08 PDT
Change pointers in affected files, and remove cycle collection macros related to mTreeView.

Foolow-up to https://bugzilla.mozilla.org/show_bug.cgi?id=739524
Comment 1 Nagarjuna Varma [:Infinity] 2012-04-26 09:14:30 PDT
Created attachment 618682 [details] [diff] [review]
Patchv0

I have made a patch based on what I hae understood so far about the bug. But the build fails with this. Could I get some help on what I have done wrong or what I should further do for this bug to get it fixed.
Comment 2 alexander :surkov 2012-04-26 09:30:16 PDT
Comment on attachment 618682 [details] [diff] [review]
Patchv0

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

it looks ok, I'll do review later (Trevor is busy until Friday iirc)
Comment 3 Nagarjuna Varma [:Infinity] 2012-04-26 09:52:12 PDT
I have also pastebin it on http://pastebin.mozilla.org/1599146. My build error is as shown below:

../../../dist/include/nsIDOMXULSelectCntrlEl.h:33:60: warning: ‘virtual nsresult nsIDOMXULSelectControlElement::GetSelectedItem(nsIDOMXULSelectControlItemElement**)’ was hidden
../../../dist/include/nsIDOMXULMultSelectCntrlEl.h:73:60: warning:   by ‘virtual nsresult nsIDOMXULMultiSelectControlElement::GetSelectedItem(PRInt32, nsIDOMXULSelectControlItemElement**)’
/home/arjun/src/accessible/src/xul/nsXULTreeAccessible.cpp: In constructor ‘nsXULTreeAccessible::nsXULTreeAccessible(nsIContent*, nsDocAccessible*)’:
/home/arjun/src/accessible/src/xul/nsXULTreeAccessible.cpp:75:45: error: no matching function for call to ‘nsITreeBoxObject::GetView(const already_AddRefed<nsITreeView>)’
../../../dist/include/nsITreeBoxObject.h:48:60: note: candidate is: virtual nsresult nsITreeBoxObject::GetView(nsITreeView**)
Comment 4 alexander :surkov 2012-04-26 09:56:58 PDT
/home/arjun/src/accessible/src/xul/nsXULTreeAccessible.cpp: In constructor ‘nsXULTreeAccessible::nsXULTreeAccessible(nsIContent*, nsDocAccessible*)’:
/home/arjun/src/accessible/src/xul/nsXULTreeAccessible.cpp:75:45: error: no matching function for call to ‘nsITreeBoxObject::GetView(const already_AddRefed<nsITreeView>)’
../../../dist/include/nsITreeBoxObject.h:48:60: note: candidate is: virtual nsresult nsITreeBoxObject::GetView(nsITreeView**)

so you need to do something like

if (mTree) {
  nsCOMPtr<nsITreeView> treeView;
  mTree->GetView(getter_AddRefs(treeView));
  mTreeView = treeView;
}
Comment 5 Nagarjuna Varma [:Infinity] 2012-04-26 11:11:07 PDT
Created attachment 618724 [details] [diff] [review]
Patchv1

Functional Patch with required changes incorporated in it.
Comment 6 alexander :surkov 2012-04-26 22:19:24 PDT
Comment on attachment 618724 [details] [diff] [review]
Patchv1

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

r=me, I'll fix nits before landing. In general please take care about code style.

::: accessible/src/xul/nsXULTreeAccessible.cpp
@@ +74,5 @@
> +  if (mTree) {
> +  nsCOMPtr<nsITreeView> treeView;
> +  mTree->GetView(getter_AddRefs(treeView));
> +  mTreeView = treeView;
> +}

wrong indentation, use two spaces for indent
Comment 7 alexander :surkov 2012-04-26 22:23:17 PDT
and please make sure you keep sources up to dated, i.e. the patch is against trunk
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-04-29 13:50:56 PDT
http://hg.mozilla.org/mozilla-central/rev/103044663fd3
Comment 10 Nagarjuna Varma [:Infinity] 2012-04-30 05:59:25 PDT
Sure surkov, won't make the same mistakes again.
Had a good time working with you on the bug. Thanks for all the help with it.
Comment 11 alexander :surkov 2012-04-30 06:51:50 PDT
(In reply to Arjun from comment #10)
> Sure surkov, won't make the same mistakes again.
> Had a good time working with you on the bug. Thanks for all the help with it.

ok, cool. thanks.

Note You need to log in before you can comment on or make changes to this bug.