265339 - nsCParserStartNode::~nsCParserStartNode asserts unnecessarily

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

nsCParserStartNode::~nsCParserStartNode asserts to make sure it has an
allocator, but it only ever uses that allocator to delete its attributes. This
means that if you have the equivilant of:

{
  nsCParserStartNode node;
}

You will assert because mTokenAllocator is NULL. This will happen once bug
263083 is checked in, if you never view-source any pages that have errors on
them. Trivial patch coming up.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v1

Only assert if we're actually going to use mTokenAllocator.

Comment 2

[Peter Van der Beken [:peterv]]

Comment on attachment 162764 [details] [diff] [review]
patch v1

>Index: src/nsParserNode.h
>===================================================================
>RCS file: /cvsroot/mozilla/parser/htmlparser/src/nsParserNode.h,v
>retrieving revision 3.47
>diff -u -5 -p -r3.47 nsParserNode.h
>--- src/nsParserNode.h	23 Apr 2004 13:28:51 -0000	3.47
>+++ src/nsParserNode.h	20 Oct 2004 22:57:47 -0000
>@@ -294,11 +294,12 @@ public:
>                        nsNodeAllocator* aNodeAllocator = 0) 
>       : nsCParserNode(aToken, aTokenAllocator, aNodeAllocator), mAttributes(0) { }
> 
>     virtual ~nsCParserStartNode() 
>     {
>-      NS_ASSERTION(0 != mTokenAllocator, "Error: no token allocator");
>+      NS_ASSERTION(0 != mTokenAllocator || 0 == mAttributes.GetSize(),

I'd prefer

      NS_ASSERTION(mTokenAllocator || mAttributes.GetSize() == 0,

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Checked in.