Closed Bug 333295 Opened 14 years ago Closed 14 years ago

[@ nsDocAccessible::GetBoundsRect] if !vm

Categories

(Core :: Disability Access APIs, defect, critical)

1.8 Branch
x86
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla1.8.1beta2

People

(Reporter: timeless, Assigned: gaomingcn)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity, crash, fixed1.8.1)

Crash Data

Attachments

(2 files, 1 obsolete file)

found by coverity
Assignee: aaronleventhal → eh
Blocks: fox2access
Attached patch The fix of this bug. (obsolete) — Splinter Review
I take this bug per Aaron's suggestion. And I got a patch ready for review. But I could not assign the bug to myself;)
Attachment #222042 - Flags: review?(aaronleventhal)
Comment on attachment 222042 [details] [diff] [review]
The fix of this bug.

Index: mozilla/accessible/src/base/nsDocAccessible.cpp
===================================================================
RCS file: /cvsroot/mozilla/accessible/src/base/nsDocAccessible.cpp,v
retrieving revision 1.91
diff -p -u -5 -r1.91 nsDocAccessible.cpp
--- mozilla/accessible/src/base/nsDocAccessible.cpp     24 Apr 2006 20:19:44 -0000      1.91
+++ mozilla/accessible/src/base/nsDocAccessible.cpp     15 May 2006 16:30:53 -0000
@@ -505,12 +505,17 @@ void nsDocAccessible::GetBoundsRect(nsRe
       return;
     }
     nsIViewManager* vm = presShell->GetViewManager();

     nsIScrollableView* scrollableView = nsnull;
-    if (vm)
+
+    if (!vm) {
+      return;
+    }
+    else {
       vm->GetRootScrollableView(&scrollableView);
+    }

     nsRect viewBounds(0, 0, 0, 0);
     if (scrollableView) {
       viewBounds = scrollableView->View()->GetBounds();
     }
Should be this one..
Attachment #222042 - Attachment is obsolete: true
Attachment #222049 - Flags: review?(aaronleventhal)
Attachment #222042 - Flags: review?(aaronleventhal)
Comment on attachment 222049 [details] [diff] [review]
The fix of this bug.

r+, but remove the else, because it is not necessary. As a general rule Mozilla code avoids else after return.
Attachment #222049 - Flags: superreview?(neil)
Attachment #222049 - Flags: review?(aaronleventhal)
Attachment #222049 - Flags: review+
Assignee: eh → gaomingcn
Comment on attachment 222049 [details] [diff] [review]
The fix of this bug.

>     nsIViewManager* vm = presShell->GetViewManager();
> 
>     nsIScrollableView* scrollableView = nsnull;
>-    if (vm)
>+
>+    if (!vm) {
>+      return;
>+    }
>+    else {
>       vm->GetRootScrollableView(&scrollableView);
>+    }
As well as what Aaron said, the if (!vm) { return; } belongs just after the construction of vm, rather than just before its use, i.e.

nsIViewManager* vm = presShell->GetViewManager();
if (!vm) {
  return;
}

nsIScrollableView* scrollableView = nsnull;
vm->GetRootScrollableView(&scrollableView);
Attachment #222049 - Flags: superreview?(neil) → superreview+
I updated the patch according to your comments. But the previous patch got (super)review+ already. I am not sure if this is necessary.
Attachment #223286 - Flags: superreview?(neil)
Attachment #223286 - Flags: review?(aaronleventhal)
Comment on attachment 223286 [details] [diff] [review]
Update according to the comments.

r+ as long as you tested it.
Attachment #223286 - Flags: review?(aaronleventhal) → review+
Comment on attachment 223286 [details] [diff] [review]
Update according to the comments.

>     nsIViewManager* vm = presShell->GetViewManager();
> 
>+    if (!vm) {
>+      return;
>+    }
Nit: that blank line isn't needed here.
Attachment #223286 - Flags: superreview?(neil) → superreview+
Attachment #223286 - Flags: approval-branch-1.8.1?
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Attachment #223286 - Flags: approval-branch-1.8.1? → approval1.8.1?
Attachment #223286 - Flags: approval1.8.1? → approval1.8.1+
mozilla/accessible/src/base/nsDocAccessible.cpp 	1.73.2.11
Keywords: fixed1.8.1
Target Milestone: --- → mozilla1.8.1beta2
Version: Trunk → 1.8 Branch
Crash Signature: [@ nsDocAccessible::GetBoundsRect]
You need to log in before you can comment on or make changes to this bug.