Last Comment Bug 211357 - [FIX]Crash - accessing TR.rowIndex
: [FIX]Crash - accessing TR.rowIndex
: crash, fixed1.4.1, testcase
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: P2 major (vote)
: mozilla1.5alpha
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
: Andrew Overholt [:overholt]
Depends on:
Blocks: 224532
  Show dependency treegraph
Reported: 2003-07-01 19:07 PDT by Bob Clary [:bc:]
Modified: 2008-07-31 02:37 PDT (History)
0 users
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (454 bytes, text/html)
2003-07-01 19:08 PDT, Bob Clary [:bc:]
no flags Details
patch (1.22 KB, patch)
2003-07-01 21:35 PDT, Boris Zbarsky [:bz] (still a bit busy)
jst: review+
jst: superreview+
asa: approval1.4.1+
Details | Diff | Splinter Review

Description Bob Clary [:bc:] 2003-07-01 19:07:19 PDT
Create a TR element and access TR.rowIndex before attaching to a TABLE to crash.
Comment 1 Bob Clary [:bc:] 2003-07-01 19:08:10 PDT
Created attachment 126876 [details]
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2003-07-01 21:35:16 PDT
Created attachment 126883 [details] [diff] [review]

The null-checking here got badly broken at some point....
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2003-07-01 21:36:56 PDT
Comment on attachment 126883 [details] [diff] [review]

jst, is peterv generally around?  I'm feeling bad about all these review
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2003-07-01 21:38:06 PDT
Comment 6 Johnny Stenback (:jst, 2003-07-02 17:21:55 PDT
Comment on attachment 126883 [details] [diff] [review]

   nsresult rv = GetParentNode(getter_AddRefs(sectionNode));
+  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?

Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2003-07-02 19:47:42 PDT
Indeed.  Changed those to normal ifs and checked in.
Comment 8 Bob Clary [:bc:] 2003-07-03 07:47:35 PDT
This isn't that big a deal, but should we get it on the 1.4 branch as well?
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2003-07-03 10:09:13 PDT
Might be a good idea, yes... simple fix and all.
Comment 10 Asa Dotzler [:asa] 2003-08-07 12:17:55 PDT
Comment on attachment 126883 [details] [diff] [review]

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 [:bz] (still a bit busy) 2003-08-09 08:20:40 PDT
Fixed on 1.4 branch.

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