Use nsITreeView* instead of nsCOMPtr<nsITreeView> as followup to bug#739524

RESOLVED FIXED in mozilla15

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: capella, Assigned: Infinity)

Tracking

(Blocks: 1 bug)

unspecified
mozilla15
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++], URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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
(Reporter)

Updated

5 years ago
Summary: Use nsITreeView* instead of nsRefPtr<nsITreeView> as followup to bug#739524 → Use nsITreeView* instead of nsCOMPtr<nsITreeView> as followup to bug#739524

Updated

5 years ago
Blocks: 531850
(Reporter)

Updated

5 years ago
Depends on: 739524
(Reporter)

Updated

5 years ago
Assignee: nobody → junky.argonaut
Status: NEW → ASSIGNED
(Assignee)

Comment 1

5 years ago
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.
Attachment #618682 - Flags: feedback?(trev.saunders)
Attachment #618682 - Flags: feedback?(surkov.alexander)

Comment 2

5 years ago
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)
Attachment #618682 - Flags: review?(surkov.alexander)
Attachment #618682 - Flags: feedback?(trev.saunders)
Attachment #618682 - Flags: feedback?(surkov.alexander)
(Assignee)

Comment 3

5 years ago
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

5 years ago
/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;
}
(Assignee)

Comment 5

5 years ago
Created attachment 618724 [details] [diff] [review]
Patchv1

Functional Patch with required changes incorporated in it.
Attachment #618682 - Attachment is obsolete: true
Attachment #618682 - Flags: review?(surkov.alexander)
Attachment #618724 - Flags: review?(surkov.alexander)

Comment 6

5 years ago
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
Attachment #618724 - Flags: review?(surkov.alexander) → review+

Updated

5 years ago
Whiteboard: [lang=c++] → [good first bug][mentor=surkov.alexander@gmail.com][lang=c++]

Comment 7

5 years ago
and please make sure you keep sources up to dated, i.e. the patch is against trunk

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/103044663fd3
Target Milestone: --- → mozilla15
http://hg.mozilla.org/mozilla-central/rev/103044663fd3
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
(Assignee)

Comment 10

5 years ago
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

5 years ago
(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.
You need to log in before you can comment on or make changes to this bug.