143512 - Assertion in CNavDTD.cpp (htmlparser code) when loading netscape.com

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P5)

Description

[David Epstein]

not sure who to assign this to. reassign if necessary.
Mozilla 1.0.0+ Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.0.0+) Gecko/20020509

Just load www.netscape.com in mozilla and you get this assert:
"Assertion: attribute encountered - this shouldn't happen unless the attribute
was part of a start tag! 'Error', file /mozilla/htmlparser/src/CNavDTD.cpp line 2229

Call stack:
nsDebug::Assertion(const char * 0x02920390, const char * 0x1012c990, const char
* 0x0292035c, int 2229) line 291 + 13 bytes
nsDebug::Error(const char * 0x02920390, const char * 0x0292035c, int 2229) line
458 + 22 bytes
CNavDTD::HandleAttributeToken(CToken * 0x03f891c8) line 2229 + 21 bytes
CNavDTD::HandleToken(CNavDTD * const 0x03eb6eb0, CToken * 0x03f891c8, nsIParser
* 0x03d35a28) line 910 + 12 bytes
CNavDTD::BuildModel(CNavDTD * const 0x03eb6eb0, nsIParser * 0x03d35a28,
nsITokenizer * 0x03e5fd38, nsITokenObserver * 0x00000000, nsIContentSink *
0x03978e80) line 507 + 20 bytes
nsParser::BuildModel() line 1870 + 34 bytes
nsParser::ResumeParse(int 1, int 0, int 1) line 1737 + 11 bytes
nsParser::OnDataAvailable(nsParser * const 0x03d35a2c, nsIRequest * 0x03cb3c38,
nsISupports * 0x00000000, nsIInputStream * 0x03ab5988, unsigned int 28672,
unsigned int 16384) line 2371 + 21 bytes
nsDocumentOpenInfo::OnDataAvailable(nsDocumentOpenInfo * const 0x03ae1f98,
nsIRequest * 0x03cb3c38, nsISupports * 0x00000000, nsIInputStream * 0x03ab5988,
unsigned int 28672, unsigned int 16384) line 242 + 46 bytes
nsStreamListenerTee::OnDataAvailable(nsStreamListenerTee * const 0x03e012d8,
nsIRequest * 0x03cb3c38, nsISupports * 0x00000000, nsIInputStream * 0x03ca2128,
unsigned int 28672, unsigned int 16384) line 56 + 51 bytes
nsHttpChannel::OnDataAvailable(nsHttpChannel * const 0x03cb3c3c, nsIRequest *
0x03cde93c, nsISupports * 0x00000000, nsIInputStream * 0x03ca2128, unsigned int
28672, unsigned int 16384) line 2982 + 63 bytes
nsOnDataAvailableEvent::HandleEvent() line 193 + 70 bytes
nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x03eb085c) line 116
PL_HandleEvent(PLEvent * 0x03eb085c) line 596 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x01022b80) line 526 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x005605d8, unsigned int 49517, unsigned int 0,
long 16919424) line 1077 + 9 bytes
USER32! 77e71820()
0102

Comment 1

[Darin Fisher]

-> parser

Comment 2

[Bernd]

there is a simple testcase for this already in the tree:
http://lxr.mozilla.org/seamonkey/source/layout/html/tests/block/bugs/60992.html.

This is part of the layout regression tests....

Comment 3

[harishd]

We assert because there are attributes in an _end_ tag!. This is nothing serious.
Though I need to change the assertion message.

Comment 4

[Mike]

I saw this assertion on a web page I created that passed the w3c html validator
test(4.01 Transitional).  Puzzled I went through my code and discovered a line
where I had an incomplete end tag that looked like this: </p

Adding the > stopped the assertion.

I am running an optimized Linux build so thank you for delaying bug 78646 and
helping me code better!  Thank you for coding Moz to not crash on my junk code
as well!

Comment 5

[Mats Palmgren (inactive)]

*** Bug 254726 has been marked as a duplicate of this bug. ***

Comment 6

[Boris Zbarsky [:bzbarsky]]

See bug 254726 comment 2 for what I think should happen here...

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Is there a reason that we actually need that assert? We'll simply end up
ignoring those tokens anyway, it's just a matter of where we do it. We don't
even need to worry about line numbers because that's all handled in
CNavDTD::HandleToken anyway.

It would be trivial to write a function to simply pop all attribute tokens off
of the stack (until a non-attribute token occurs).

Comment 8

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

Attachment: Patch

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 155539 [details] [diff] [review]
Patch

r+sr=bzbarsky

Comment 10

[Boris Zbarsky [:bzbarsky]]

I've checked this in for 1.8a3.

Comment 11

[jag (Peter Annema)]

Hmm, why not get rid of the bool aDiscardAttributes and just let the lack of
nsIParserNode signal that you just wanna ignore the attributes? Think /dev/null :-)

Comment 12

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

Attachment: Remove aDiscardAttributes

Because originally the function didn't take a pointer, so I couldn't send null,
and it didn't occur to me that with a pointer I could just do a null check :).

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 155689 [details] [diff] [review]
Remove aDiscardAttributes

Jag's right.

Comment 14

[Boris Zbarsky [:bzbarsky]]

I checked in the second patch.

Comment 15

[jag (Peter Annema)]

> Because originally the function didn't take a pointer, so I couldn't send null,
> and it didn't occur to me that with a pointer I could just do a null check :).

Heh, yeah, that's happened to me before :-) All good now, yay!