286733 - crash: nightly crash with form and embedded table. SinkContext::Begin

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Tavis Ormandy]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.6) Gecko/20050305 Firefox/1.0.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.6) Gecko/20050305 Firefox/1.0.1

testcase attached seems to crash nightly.

Reproducible: Always

Comment 1

[Tavis Ormandy]

Attachment: testcase

Comment 2

[Mats Palmgren (:mats)]

Attachment: Crash stack

Comment 3

[Tavis Ormandy]

TB4435667K (Linux)
TB4435581W (Windows)

This is reliably reproduced on Linux, in windows I'm told it happens sometimes, 
but everytime if you use:

start->run->mozilla https://bugzilla.mozilla.org/attachment.cgi?id=177869

Comment 4

[Mats Palmgren (:mats)]

Regression window: 2005-01-25-06 -- 2005-01-26-06 (possibly bug 147446?)

Comment 5

[Mats Palmgren (:mats)]

Yep, backing out bug 147446 fixes it...

Comment 6

[Mats Palmgren (:mats)]

Attachment: Minimal testcase (CRASH)

<TABLE>
<FRAMESET><FRAME></FRAMESET>
<TR><TD><TABLE>
	  <TR><BR><TD><MAP><TABLE><BR></MAP>

Comment 7

[Mats Palmgren (:mats)]

Attachment: Testcase #2 - the <TR> case (CRASH)

This also crash after backing out bug 147446, so the problem was also present
after bug 58942. If I add back eHTMLTag_map to gTRKids this crash also goes
away. It's almost the same as the last testcase - just added a <TR>:

<TABLE>
<FRAMESET><FRAME></FRAMESET>
<TR><TD><TABLE>
	  <TR><BR><TD><MAP><TABLE><TR><BR></MAP>

-----------------------------------^^

Comment 8

[Mats Palmgren (:mats)]

Attachment: Patch rev. 1

Removing 'eHTMLTag_map' from 'gHtmlKids' fixes all the crashes -
I have to admit I don't understand exactly why though...

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 177916 [details] [diff] [review]
Patch rev. 1

I don't think this fixes the root cause of this bug. Actually, I think the root
cause is mentioned in bug 269095 comment 10.

I think mBodyContext->mContextTopIndex is getting modified to point to a
removed element (see CNavDTD::HandleOmittedTag and then look at
http://lxr.mozilla.org/seamonkey/source/parser/htmlparser/src/CNavDTD.cpp#2012)
. Note that mBodyContext->mContextTopIndex is not modified, leaving it pointing
to a closed index.

I'll attach a patch that fixes this bug, and handles
mBodyContext->mContextTopIndex more properly.

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: alternative approach

This patch should apply from mozilla/parser/htmlparser. It works as described
above.

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 177930 [details] [diff] [review]
alternative approach

Seems reasonable. r=jst

Comment 12

[rbs]

Comment on attachment 177930 [details] [diff] [review]
alternative approach

sr=rbs

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked in.

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED: all testcases work for me here now with build 2005-04-10-05 on
Windows XP Seamonkey trunk.

Comment 15

[Marcia Knous [:marcia]]

verified fixed on the 1.0.1 branch using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.12) Gecko/20060210 Firefox/1.0.7. I loaded the testcases and no crash. Adding keyword.

Comment 16

[Marcia Knous [:marcia]]

verified fixed using Mozilla 1.7.12 Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.12) Gecko/20060210. No crash with testcases. Adding keyword.

Comment 17

[Bob Clary [:bc] (inactive)]

parser/htmlparser/tests/crashtests/286733-1.html
parser/htmlparser/tests/crashtests/286733-2.html
http://hg.mozilla.org/mozilla-central/rev/b0337b6287f3