211357 - [FIX]Crash - accessing TR.rowIndex

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Bob Clary [:bc] (inactive)]

Create a TR element and access TR.rowIndex before attaching to a TABLE to crash.

Comment 1

[Bob Clary [:bc] (inactive)]

Attachment: testcase

Comment 2

[Bob Clary [:bc] (inactive)]

http://climate/reports/SingleIncidentInfo.cfm?dynamicBBID=21533709
http://climate/reports/SingleIncidentInfo.cfm?dynamicBBID=21533590

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: patch

The null-checking here got badly broken at some point....

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 126883 [details] [diff] [review]
patch

jst, is peterv generally around?  I'm feeling bad about all these review
requests...

Comment 5

[Boris Zbarsky [:bzbarsky]]

.

Comment 6

[Johnny Stenback (:jst)]

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

Comment 7

[Boris Zbarsky [:bzbarsky]]

Indeed.  Changed those to normal ifs and checked in.

Comment 8

[Bob Clary [:bc] (inactive)]

This isn't that big a deal, but should we get it on the 1.4 branch as well?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Might be a good idea, yes... simple fix and all.

Comment 10

[Asa Dotzler [:asa]]

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?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Fixed on 1.4 branch.