333497 - HTMLContentSink::EndContext. SeaMonkey Trunk Browser crash if JavaScript is turned off

Status: RESOLVED FIXED

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

Description

[Ruediger Lahl]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060410 SeaMonkey/1.5a
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060410 SeaMonkey/1.5a

If you visit http://tekade.de , SM-Trunk crashes, if JavaScript is turned off.
The site checks, if JavaScript is anabled and sends a warningpage if its not anabled. At this moment SM crashes.

Reproducible: Always

Steps to Reproduce:
1.turn JS off
2.visit http://tekade.de
3.crash
Actual Results:  
SM crashes

Expected Results:  
SM should show the warningpage

Talkback-ID is: TB17412065Z (thanks to toscha)

Searching for the last good build was tricky.

2005090804 was the last build that show the warningpage on my system.

2005090904 show an empty page but do not crash.
2005102104 always do so.

2005102112 crashes.

This is a very small window and gives a good chance to find the responsible check in!

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

This crashes for me when javascript is turned off.

Comment 2

[Ruediger Lahl]

(In reply to comment #1)
> Created an attachment (id=217942) [edit]
> testcase
> 
> This crashes for me when javascript is turned off.

ACK.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix, v1

This patch contains two fixes:
-- Note the return value from BeginContext. nsIHTMLContentSink::BeginContext returns failure when we pass it a bad index, and soldiering on after doing so will result in guaranteed crashes.

-- Always push a stack entry on the DTD context for opened heads. Not doing so is the root cause of this bug because when we try to pass an index from the DTD into the sink, the two stacks *must* match.

With this patch, we don't quite build the expected DOM for reasons I'm not sure of, but I'm not sure if anybody cares. It might be worth filing a new bug on that.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix, v2

This fixes the content model weirdness by making the head behave more like a normal tag. It also makes sure that if we're going to try to stick a tag in the body, that we actually have a body.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 217976 [details] [diff] [review]
Proposed fix, v2

Admittedly, I don't really understand all that's going on in this patch.

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 217976 [details] [diff] [review]
Proposed fix, v2

sr=jst

Comment 7

[Martijn Wargers (dead)]

Blake, the patch has r+ and sr+, so it can be checked in. Not sure whether you're waiting on something or simply forgot about this one.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

I'm hoping to find some time to re-review it myself to make sure I really understand how it works and what's going on.

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked in.

Comment 10

[Ruediger Lahl]

(In reply to comment #9)
> Fix checked in.

Thank you. I cant check out, if this patch eleminate my crash, cause now I found out, that the site http://tekade.de *works* since my SM-Trunk-build 2006060804. Last bad here was 2006060700. Something must be done in between...

Anyway. Thanks for your work.