The default bug view has changed. See this FAQ.

[FIX]Crash - accessing TR.rowIndex

RESOLVED FIXED in mozilla1.5alpha

Status

()

Core
DOM: Core & HTML
P2
major
RESOLVED FIXED
14 years ago
9 years ago

People

(Reporter: bc, Assigned: bz)

Tracking

({crash, fixed1.4.1, testcase})

Trunk
mozilla1.5alpha
crash, fixed1.4.1, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

14 years ago
Create a TR element and access TR.rowIndex before attaching to a TABLE to crash.
(Reporter)

Comment 1

14 years ago
Created attachment 126876 [details]
testcase
(Reporter)

Comment 2

14 years ago
http://climate/reports/SingleIncidentInfo.cfm?dynamicBBID=21533709
http://climate/reports/SingleIncidentInfo.cfm?dynamicBBID=21533590
Severity: normal → major
Keywords: testcase
Created attachment 126883 [details] [diff] [review]
patch

The null-checking here got badly broken at some point....
Comment on attachment 126883 [details] [diff] [review]
patch

jst, is peterv generally around?  I'm feeling bad about all these review
requests...
Attachment #126883 - Flags: superreview?(jst)
Attachment #126883 - Flags: review?(jst)
.
Assignee: dom_bugs → bzbarsky
OS: Windows 2000 → All
Priority: -- → P2
Hardware: PC → All
Summary: Crash - accessing TR.rowIndex → [FIX]Crash - accessing TR.rowIndex
Target Milestone: --- → mozilla1.5alpha
Comment on attachment 126883 [details] [diff] [review]
patch

   nsresult rv = GetParentNode(getter_AddRefs(sectionNode));
-  NS_ENSURE_SUCCESS(rv, rv);
+  NS_ENSURE_TRUE(sectionNode, rv);

   nsCOMPtr<nsIDOMNode> tableNode;
   rv = sectionNode->GetParentNode(getter_AddRefs(tableNode));
-  if (NS_SUCCEEDED(rv) && sectionNode) {
-    rv = CallQueryInterface(tableNode, aTable);
-  }
-
-  return rv;
+  NS_ENSURE_TRUE(tableNode, rv);
+  
+  return CallQueryInterface(tableNode, aTable);

Shouldn't those be just "if (!...) return rv;"? We don't want to print out a
warning just because a tr element has no parent when accessing .rowIndex do we?

r+sr=jst
Attachment #126883 - Flags: superreview?(jst)
Attachment #126883 - Flags: superreview+
Attachment #126883 - Flags: review?(jst)
Attachment #126883 - Flags: review+
Indeed.  Changed those to normal ifs and checked in.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
(Reporter)

Comment 8

14 years ago
This isn't that big a deal, but should we get it on the 1.4 branch as well?
Might be a good idea, yes... simple fix and all.
Flags: blocking1.4.x?

Updated

14 years ago
Attachment #126883 - Flags: approval1.4.x?

Comment 10

14 years ago
Comment on attachment 126883 [details] [diff] [review]
patch

a=asa (on behalf of drivers) for checkin to the 1.4 branch. Time is short. Who
can land this for us?
Attachment #126883 - Flags: approval1.4.x? → approval1.4.x+
Fixed on 1.4 branch.
Flags: blocking1.4.x?
Keywords: fixed1.4
Keywords: fixed1.4 → fixed1.4.1

Updated

14 years ago
Blocks: 224532

Updated

9 years ago
Component: DOM: HTML → DOM: Core & HTML
QA Contact: desale → general
You need to log in before you can comment on or make changes to this bug.