Bug 4395

Summary:

REGRESSION: document.open doesn't clear the document

Product:

WebKit

Reporter:

Alexey Proskuryakov <ap>

Component:

DOM

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

alice.barraclough, gavin.sharp

Priority:

Keywords:

HasReduction, InRadar, Regression

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
test case	none
self-contained test case	none
proposed fix	mjs: review-
test case that this change would break	none
revised fix	none
revised fix	mjs: review+

Alexey Proskuryakov

Reported 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).

Attachments
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
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Alexey Proskuryakov

Comment 1 2005-08-11 12:20:55 PDT

Created attachment 3346 [details] test case

Alexey Proskuryakov

Comment 2 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).

Alexey Proskuryakov

Comment 3 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.

Joost de Valk (AlthA)

Comment 4 2006-01-22 04:34:16 PST

Adding Regression keyword.

Joost de Valk (AlthA)

Comment 5 2006-01-22 04:41:10 PST

Upping to P1 because this is a regression.

Alice Liu

Comment 6 2006-01-26 17:44:57 PST

<rdar://problem/4424096>

Alexey Proskuryakov

Comment 7 2006-01-31 11:28:44 PST

Opened a Mozilla bug: https://bugzilla.mozilla.org/show_bug.cgi?id=325352

Alexey Proskuryakov

Comment 8 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).

Alexey Proskuryakov

Comment 9 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.

Geoffrey Garen

Comment 10 2006-03-08 16:22:32 PST

What are the consequences of opening the document while parsing? Should ::open cancel parsing?

Alexey Proskuryakov

Comment 11 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.

Maciej Stachowiak

Comment 12 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.

Alexey Proskuryakov

Comment 13 2006-03-15 11:13:27 PST

Created attachment 7091 [details] revised fix

Alexey Proskuryakov

Comment 14 2006-03-15 11:15:22 PST

Created attachment 7092 [details] revised fix Oops, forgot the include the original test.

Maciej Stachowiak

Comment 15 2006-03-15 20:56:52 PST

Comment on attachment 7092 [details] revised fix r=me

Alexey Proskuryakov

Comment 16 2006-03-18 23:36:12 PST

Several regressions were caused by this change: bug 7804, bug 7818, bug 7848.

Note You need to log in before you can comment on or make changes to this bug.