document.open doesn't clear the document

RESOLVED FIXED in Firefox 69

Status

()

defect
P5
normal
RESOLVED FIXED
14 years ago
25 days ago

People

(Reporter: ap, Assigned: bzbarsky)

Tracking

({html5, testcase})

Trunk
mozilla68
PowerPC
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 wontfix, firefox69 fixed)

Details

()

Attachments

(4 attachments)

Reporter

Description

14 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; ru-ru) AppleWebKit/417.9 (KHTML, like Gecko) Safari/417.8
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

This is split from WebKit bug <http://bugzilla.opendarwin.org/show_bug.cgi?id=4395>.

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

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

The test case works as expected in WinIE 6.

Reproducible: Always

Steps to Reproduce:
Reporter

Comment 1

14 years ago
Posted file test case
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase
It seems like IE implicitly calls close() from open(). Should we do the same?
We should do according to HTMLDocument::open() of W3C DOM Level 2 HTML, which in this case is what MSHTML-based UAs like IE (maybe by accident) do:

,-<http://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-72161170>
|
| open
|   Open a document stream for writing. If a document exists in the target,
|   this method clears it.
The behavior of document.open() is now covered by HTML5 making this an HTML5 conformance issue. For my ongoing HTML5 parsing efforts, I have the code for clearing the document, which isn't a big deal.

The problematic thing still appears to be that removing the root element and inserting a new one confuses the scroll bar (on iframes at least).

http://livedom.validator.nu/ clears the root element and creates a new one, and the scrollbar gets confused.
Keywords: html5
Depends on: 78070
No longer depends on: 78070
Depends on: 78070
> It seems like IE implicitly calls close() from open(). Should we do the same?

That would make sense.  Note that the testcases for this bug can be fixed without removing the root on open(), and removing the root on open() won't fix the testcases here (because open() is currently a no-op if we have a parser).

So removing dependency on bug 78070.
I filed bug 486741 on removing the root element hackery, by the way.
Assignee: general → nobody
QA Contact: ian → general
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.
Priority: -- → P5

Hmm. The behavior of Gecko after bug 1489308 does not match the behavior of Chrome on attachment 210306 [details]. As far as I can tell, Gecko is following the spec as currently written. What is Chrome doing? Timothy, do you know?

Depends on: 1489308
Flags: needinfo?(timothygu99)

Comment 10

Last month

Some digging around shows that Blink clears the parser in document.close(), allowing a subsequent synchronous document.open() to work. This behavior was in fact inherited from a 2003 change in WebKit, and probably the cause of this longstanding bug.

I was not aware of this behavior difference (as I focused mostly on document.open() itself), but it might worth filing an HTML bug about it. Boris, would implementing the behavior I just described in Gecko be feasible?

Flags: needinfo?(timothygu99)

The issue is that not that document.open() works after document.close(). That part is interoperable across browsers. The issue is that in Blink it looks like this loop:

	for (i = 0; i < 5; i++) {
		document.open();
		document.write("There should be only one line of text.<br>");
	}

ends up resetting the DOM on each open() call, even though there is a parser (the one the previous open() call set up!), so the end result only has one line of text in the DOM, not 5 lines. This seems like squarely an issue of what document.open() does in Blink, as far as I can tell.

Flags: needinfo?(timothygu99)

Comment 12

Last month

Oh oops, let's try this once more.

The document open steps say:

  1. If document has an active parser whose script nesting level is greater than 0, then return document.

  1. Create a new HTML parser and associate it with document.

The definition of script nesting level says:

parsers have a script nesting level, which must be initially set to zero

In the for-loop, after the document.open() call, a new parser is created. However, when a parser is created, the script nesting level is (re)set to zero, only to be in/decremented when <script> tags are seen in the parsed HTML text. So when the next document.open() call executes, it would see an HTML parser that is not running script, and thus overwrite it.

Indeed, if I try to call document.open() again in the document.write() call in a <script> tag, like

function mainFunc() {
	document.open();
	document.write("will not be visible<br>");
	document.close();

	for (i = 0; i < 5; i++) {
		document.open();
		document.write(`
			There should be two lines of text.<br>
			<script>
				document.open();
				document.write('extra');
			</${''}script>
		`);
	}
}

Chrome ignores the extra document.open() calls and print both “There should be two lines of text” and “extra”.

Flags: needinfo?(timothygu99)

Ah, I see.

Henri, does our parser have this concept of "script nesting level" at all?

It sounds like there are no web platform tests for this script nesting level bit, unfortunately... :(

Flags: needinfo?(hsivonen)

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #13)

Henri, does our parser have this concept of "script nesting level" at all?

I think https://searchfox.org/mozilla-central/rev/e7d9a8749303b39dadcc0e18ea0d60a570a68145/parser/html/nsHtml5Parser.h#280 is it, and the naming mismatch is thanks to the counter having been named according to previous spec concept. That is, I think I figured that a counter is needed when the spec didn't say it that way and once the spec agreed, it changed the terminology.

Flags: needinfo?(hsivonen)

It sounds like there are no web platform tests for this script nesting level bit, unfortunately... :(

Yep, definitely no tests, in that fixing this bug does not lead to any changes in wpt results. Need to write some tests...

This also exposes an accessor for whether the parser has a nonzero script nesting level.

Comment 18

Last month
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/990c8a382cf3
part 1.  Align our "defined insertion point" concept more closely with the spec's "script nesting level" concept.  r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/264fe248bca5
part 2.  Don't no-op document.open if our parser has a script nesting level equal to 0.  r=hsivonen

Comment 19

Last month
bugherder
Status: NEW → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee: nobody → bzbarsky
Regressions: 1550524
Regressions: 1550811
Blocks: 1550845

I backed out part 2 to fix bug 1550524. I'll reland it, along with fixes for bug 1550524, after the next branchpoint.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16888 for changes under testing/web-platform/tests
Upstream PR merged

Comment 23

28 days ago
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c27a9d783734
part 2.  Don't no-op document.open if our parser has a script nesting level equal to 0.  r=hsivonen

Comment 24

27 days ago
bugherder
Status: REOPENED → RESOLVED
Closed: Last month27 days ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.