Bug 4395 - REGRESSION: document.open doesn't clear the document
Summary: REGRESSION: document.open doesn't clear the document
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Nobody
Keywords: HasReduction, InRadar, Regression
Depends on:
Reported: 2005-08-11 12:20 PDT by Alexey Proskuryakov
Modified: 2006-03-18 23:36 PST (History)
2 users (show)

See Also:

test case (2.39 KB, application/octet-stream)
2005-08-11 12:20 PDT, Alexey Proskuryakov
no flags Details
self-contained test case (620 bytes, text/html)
2006-01-31 11:32 PST, Alexey Proskuryakov
no flags Details
proposed fix (2.79 KB, patch)
2006-03-08 10:25 PST, Alexey Proskuryakov
mjs: review-
Details | Formatted Diff | Diff
test case that this change would break (179 bytes, patch)
2006-03-12 13:21 PST, Maciej Stachowiak
no flags Details | Formatted Diff | Diff
revised fix (2.50 KB, patch)
2006-03-15 11:13 PST, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
revised fix (3.63 KB, patch)
2006-03-15 11:15 PST, Alexey Proskuryakov
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2005-08-11 12:20:15 PDT
document.open() should clear the document, as described in <http://www.mozilla.org/docs/dom/
domref/dom_doc_ref51.html> and <http://developer.mozilla.org/en/docs/document.open>. This 
doesn't work in Safari 2.0 and ToT (did work in 1.2.4).

Steps to reproduce: open the attached test case. In the bottom frame, there should be only one line "line".

This regression causes a problem in "real life" code that I cannot mention (it's under an NDA).
Comment 1 Alexey Proskuryakov 2005-08-11 12:20:55 PDT
Created attachment 3346 [details]
test case
Comment 2 Alexey Proskuryakov 2005-09-10 01:43:51 PDT
Lowering the priority to P2 since this isn't a regression from a previously released version.

This also fails in Firefox, but works in MacIE (haven't tried WinIE).
Comment 3 Alexey Proskuryakov 2005-09-10 01:45:57 PDT
(In reply to comment #2)
> isn't a regression from a previously released version.

Sorry, I meant _previous_ released version.
Comment 4 Joost de Valk (AlthA) 2006-01-22 04:34:16 PST
Adding Regression keyword.
Comment 5 Joost de Valk (AlthA) 2006-01-22 04:41:10 PST
Upping to P1 because this is a regression.
Comment 6 Alice Liu 2006-01-26 17:44:57 PST
Comment 7 Alexey Proskuryakov 2006-01-31 11:28:44 PST
Opened a Mozilla bug: https://bugzilla.mozilla.org/show_bug.cgi?id=325352
Comment 8 Alexey Proskuryakov 2006-01-31 11:32:27 PST
Created attachment 6162 [details]
self-contained test case

Verified that it works in WinIE; doesn't work in Firefox (looks like it also uncovers an unrelated issue in the latter).
Comment 9 Alexey Proskuryakov 2006-03-08 10:25:47 PST
Created attachment 6944 [details]
proposed fix

The code I removed here has appeared four years ago (r798, "Merged changes from LABYRINTH_KDE_3_MERGE branch"). I wonder why this worked in Safari 1.2.4.
Comment 10 Geoffrey Garen 2006-03-08 16:22:32 PST
What are the consequences of opening the document while parsing? Should ::open cancel parsing?
Comment 11 Alexey Proskuryakov 2006-03-10 11:56:11 PST
(In reply to comment #10)
Sorry, I cannot answer this question - all I know is that this change fixes one test case, and doesn't break existing tests. And, oh, my changelog entry is wrong.
Comment 12 Maciej Stachowiak 2006-03-12 13:21:25 PST
Created attachment 7034 [details]
test case that this change would break

This patch would break the attached test case. Sorry that layout tests didn't cover this already, I'll try to turn the test case into a layout test soon.

I think the real check in ::open() should not be parsing() but rather some check that the document's Frame is still loading the main resource.
Comment 13 Alexey Proskuryakov 2006-03-15 11:13:27 PST
Created attachment 7091 [details]
revised fix
Comment 14 Alexey Proskuryakov 2006-03-15 11:15:22 PST
Created attachment 7092 [details]
revised fix

Oops, forgot the include the original test.
Comment 15 Maciej Stachowiak 2006-03-15 20:56:52 PST
Comment on attachment 7092 [details]
revised fix

Comment 16 Alexey Proskuryakov 2006-03-18 23:36:12 PST
Several regressions were caused by this change: bug 7804, bug 7818, bug 7848.