331620 - [FIX]document.write on already open document clears it in some cases

Status: RESOLVED FIXED

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

Description

[Christian Myksvoll]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322)
Build Identifier: All FireFox versions from version 1.5

The bug has been there since FireFox version 1.5.

Check the provided forum link: 
http://forums.mozillazine.org/viewtopic.php?p=2166546#2166546

In addition check this URL for a test:
http://www.edialog24.com/tmp/test/

Click TWO times on Button 1, then ONCE on Button 2. IE6 (and all other major browsers) then displays: content1, content2, content3, content4. 
FireFox 1.5/2.0a1 displays only: content3, content4. 

Reproducible: Always

Steps to Reproduce:
Check Details
Actual Results:  
Check Details

Expected Results:  
Check Details

Comment 1

[Steve England [:stevee]]

Attachment: testcase from reporter's URL

Comment 2

[Ria Klaassen (not reading all bugmail)]

1.8b2_2005060906 works
1.8b2_2005061006 fails
No idea what happened with Steve's testcase :)

Comment 3

[Steve England [:stevee]]

Regression range from comment 2
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-06-09+05&maxdate=2005-06-10+07&cvsroot=%2Fcvsroot

The testcase was dependent on externally held files (my bad) so I obsolete'd it.

Comment 4

[Martijn Wargers (dead)]

With that regression range, I guess this could be a regression from bug 295983.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Please do attach a usable testcase to the bug instead of hosting it somewhere else?

This is a pretty major regression....  I'll try to figure it out when I get back, I guess, if no one does by then.

Comment 6

[Christian Myksvoll]

Attachment: Test case.

Open index.html in the browser.

Comment 7

[Daniel Veditz [:dveditz]]

bz: when you get a chance to look at this let us know if you still think it's safe and sane for the 1.8.0 branch

Comment 8

[Frank Wein [:mcsmurf]]

BTW: These assertions happen when clicking Button1 for the second time:
###!!! ASSERTION: Found more undisplayed content data after removal: 'context ==
 nsnull', file h:/mozilla/tree-main/mozilla/layout/base/nsFrameManager.cpp, line
 628
###!!! ASSERTION: EndLoad called early: 'mWriteState == eNotWriting || mWriteSta
te == ePendingClose || mWriteState == eDocumentClosed', file h:/mozilla/tree-mai
n/mozilla/content/html/document/src/nsHTMLDocument.cpp, line 1031

And these when clicking on Button2 then:
###!!! ASSERTION: Found more undisplayed content data after removal: 'context ==
 nsnull', file h:/mozilla/tree-main/mozilla/layout/base/nsFrameManager.cpp, line
 628
###!!! ASSERTION: nsHTMLDocument::Reset() - Wyciwyg Channel  still exists!: '!mW
yciwygChannel', file h:/mozilla/tree-main/mozilla/content/html/document/src/nsHT
MLDocument.cpp, line 426

But not sure how far those are related to a bad state in general, since the throbber does not stop spinning after clicking Button1 (this also happens in builds without that regression).

Comment 9

[Boris Zbarsky [:bzbarsky]]

Testcase URI:

jar:https://bugzilla.mozilla.org/attachment.cgi?id=216307!/index.html

Comment 10

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #6)
> Test case.
> Open index.html in the browser.

With seamonkey(2006040110-trunk/Win-2K), problem couldn't be created when fisrt click of button1(document.close&open, document.write content1/2) after Load folowed by several clicks fo butron2(write content3/4), but problem was re-created when second or later series of click of button1 and button2's(no assertion was observed).
Putting alert('Hello') between document.close() and document.open() was a effective workaround of this bug's phenomenon.
Seriarization of document.close() and document.open() is required?

(In reply to comment #9)
> jar:https://...
Oh, it's a nice feature!
I can easily attach zip file as attachment of test cases. 

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

The idea is really simple.  Since the close() and following write() have already terminated the mParser we set the termination function for, the termination function shouldn't mess with it.  That means the termination function should know what parser it's supposed to be terminating.

Note that I could _maybe_ have gotten to the document from the parser, but passing both in seems to be safer.

I do think this is safe for the branches.

With this fix we still get the undisplayed content assertions -- that's bug 332644.

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 217117 [details] [diff] [review]
Fix

r=mrbkap

Comment 13

[Johnny Stenback (:jst)]

Comment on attachment 217117 [details] [diff] [review]
Fix

sr=jst

Comment 14

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 217117 [details] [diff] [review]
Fix

approved for 1.8.0 branch, a=dveditz for drivers

Comment 16

[Boris Zbarsky [:bzbarsky]]

Attachment: 1.8 branch patch

Comment 17

[Boris Zbarsky [:bzbarsky]]

Attachment: 1.8.0 branch patch

Comment 18

[Boris Zbarsky [:bzbarsky]]

Fixed on both branches.