254989 - assert: all the skipped content tokens did not get handled: 'mSkippedContent.GetSize() == 0' CNavDTD.cpp line 978

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

###!!! ASSERTION: all the skipped content tokens did not get handled:
'mSkippedContent.GetSize() == 0', file
d:/cvs-1.11.5/mozilla/parser/htmlparser/src/CNavDTD.cpp, line 978

I get this when viewing a specific piece of junk mail in mailnews as Simple
HTML.  Original HTML and Plain Text do not produce the assert.  Opening the HTML
from the email in the browser doesn't produce the assert.

I have the patch to bug 143512 in my tree, but looking at the HTML, it shouldn't
have had any effect here.

Comment 1

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

NTDLL! DbgBreakPoint address 0x77f75a58
nsDebug::Assertion(const char * 0x01b52d2c `string', const char * 0x01b52d6c
`string', const char * 0x01b52b98 `string', int 978) line 109
CNavDTD::HandleToken(CNavDTD * const 0x079fa390, CToken * 0x058b1a68, nsIParser
* 0x050e7018) line 978 + 33 bytes
CNavDTD::BuildModel(CNavDTD * const 0x00000000, nsIParser * 0x050e7018,
nsITokenizer * 0x04fd8f50, nsITokenObserver * 0x00000000, nsIContentSink *
0x058085bc) line 471 + 10 bytes
nsParser::BuildModel(nsParser * const 0x04fd8f50) line 1900
nsParser::ResumeParse(nsParser * const 0x009c4880, int 0, int 0, int 0) line
1762 + 6 bytes
nsParser::Parse(nsParser * const 0x05536aa8, const nsAString & {...}, void *
0x00000000, const nsACString & {...}, int 0, int 1, nsDTDMode
eDTDMode_autodetect) line 1645 + 10 bytes
HTMLSanitize(const nsString & {...}, nsString & {...}, unsigned int 0, const
nsAString & {...}) line 2259 + 72 bytes
MimeInlineTextHTMLSanitized_parse_eof(MimeObject * 0x0292bf00, int 94239240)
line 180 + 56 bytes
MimeMultipartAlternative_display_cached_part(MimeObject * 0x04668453) line 362 +
7 bytes
MimeMultipartAlternative_parse_eof(MimeObject * 0x07635f60, int 0) line 137 + 6
bytes
MimeContainer_parse_eof(MimeObject * 0x07d152e8, int 0) line 141 + 10 bytes
MimeMessage_parse_eof(MimeObject * 0x07d152e8, int 0) line 550 + 10 bytes
mime_display_stream_complete(_nsMIMESession * 0x062bc1b0) line 965
nsStreamConverter::OnStopRequest(nsStreamConverter * const 0x01c63b81,
nsIRequest * 0x01c6c1fc, nsISupports * 0x067e8f08, unsigned int 103581688) line
1014 + 6 bytes
nsCSubstring::Truncate(nsCSubstring * const 0x009c4880, unsigned int 53955210)
line 384 + 9 bytes
nsDebug::Assertion(const char * 0x00000001, const char * 0x002a4fa0, const char
* 0x05479350, int 88576800) line 105
MIME! const nsStreamConverter::`vftable'{for `nsIMimeStreamConverter'} address
0x0467b358

Comment 2

[Boris Zbarsky [:bzbarsky]]

You could hack the IsDataAvailable code to dump out what the mailnews converter
actually passes to the parser....

Comment 3

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

<mrbkap> I'll bet you'd assert converting anything with
<script></script><script></script> to text, also.

<mrbkap> When the parser adds a leaf to the sink, you need to check if it's
<script>, <style>, etc...
<mrbkap> And if it is, you need to call dtd->CollectSkippedContent()

<mrbkap> CTho|studying: If you're going to fix it, I'd recommend seeing the HTML
content sink to see when to call CollectSkippedContent.

Comment 4

[Ben Bucksch (:BenB)]

Assigning to Mike Kaplan, because he's fixing it, it seems.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: tentative concept patch

This needs testing before I even think about requesting review. I don't know if
it will work, segfault, or do something even stranger. Any takers on testing
it?

Comment 6

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 169164 [details] [diff] [review]
tentative concept patch

Patch worked fine for me.  Thanks, Blake.

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 169164 [details] [diff] [review]
tentative concept patch

- In mozSanitizingHTMLSerializer::DoAddLeaf():

+  else if (type == eHTMLTag_script ||
+	    type == eHTMLTag_style ||
+	    type == eHTMLTag_textarea ||
+	    type == eHTMLTag_server)
+  {
+    // These special tags require some extra care. The parser gives them
+    // to us as leaves, but they're really containers. Their content is
+    // contained in the "skipped content" of the parser. This code is
+    // adapted from nsHTMLContentSink.cpp
+    nsString skippedContent;
+    PRInt32 lineNo;
+
+    nsCOMPtr<nsIDTD> dtd;
+    mParser->GetDTD(getter_AddRefs(dtd));

I don't think there's any guarantee that there will be a parser here at all
times (in the case where a DOM tree is serialized using this serializer there
won't be one), so it seems like we'd want and "&& mParser" in the condition
that gets us here.

sr=jst with that.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #7)
> I don't think there's any guarantee that there will be a parser here at all
> times (in the case where a DOM tree is serialized using this serializer there
> won't be one), so it seems like we'd want and "&& mParser" in the condition
> that gets us here.

How about an assertion? CNavDTD is probably the only code in existance to treat
these tags as leaves (it's an unfortunate that it does so) and I hope that when
serializing DOMs, <script> and friends get passed in as containers (which they
are). Something like:
NS_ASSERTION(mParser, "We are receiving containers as leaves with no skipped
content");

I'm not entirely sure how to test this, though.

Comment 9

[Peter Van der Beken [:peterv]]

Comment on attachment 169164 [details] [diff] [review]
tentative concept patch

I suppose an assert would be ok.

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Fix has been checked in (with the assertion and an additional comment suggested
by Ben).