Bug 24336 - document.write() should be able to make a document into strict mode
Summary: document.write() should be able to make a document into strict mode
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-03 15:11 PST by Eric Seidel (no email)
Modified: 2009-03-23 19:10 PDT (History)
4 users (show)

See Also:


Attachments
test case (551 bytes, text/html)
2009-03-03 15:17 PST, Eric Seidel (no email)
no flags Details
First try, removing unneeded <html> write (6.28 KB, patch)
2009-03-17 17:30 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
First try, removing unneeded <html> write (9.83 KB, patch)
2009-03-18 12:31 PDT, Eric Seidel (no email)
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2009-03-03 15:11:44 PST
document.write() should be able to make a document into strict mode

I don't know of any other way to make an existing iFrame into strict mode.  This works in FF.  See test case.
Comment 1 Eric Seidel (no email) 2009-03-03 15:17:16 PST
Created attachment 28242 [details]
test case
Comment 2 Alexey Proskuryakov 2009-03-04 10:24:41 PST
I click on the test in Safari 3.2.1, and it says "PASSED". A regression?
Comment 3 Eric Seidel (no email) 2009-03-04 10:33:32 PST
Probably.  It fails in Safari Version 4 Developer Preview (5526.11.2) (which I haven't upgraded to the real Safari 4 yet) too.
Comment 4 Ojan Vafai 2009-03-04 10:35:18 PST
Actually, if I remember correctly, the behavior changed between Safari 3 and 4. In Safari 3- you could only get iframes in CSS1Compat mode. In Safari 4, they are BackCompat by default.

BackCompat by default matches IE/FF/Opera. So, I think that's the right direction once this bug is fixed.
Comment 5 Eric Seidel (no email) 2009-03-17 12:37:06 PDT
void HTMLDocument::determineParseMode()
is the code which needs to be checked here to see if it's running correctly.
Comment 6 Eric Seidel (no email) 2009-03-17 14:02:58 PDT
This fails due to this check failing:

void HTMLParser::parseDoctypeToken(DoctypeToken* t)
{
    // Ignore any doctype after the first.  Ignore doctypes in fragments.
    if (m_document->doctype() || m_isParsingFragment || m_current != m_document)
        return;

m_current != m_document

(gdb) p m_current->showTreeForThis()
#document	0x7a6d600
*	HTML	0x1b9d1060
$4 = void
Comment 7 Eric Seidel (no email) 2009-03-17 16:12:21 PDT
OK, this is the code which is getting us in trouble:

void Document::write(const SegmentedString& text, Document* ownerDocument)
{
    if (!m_tokenizer) {  // WE TAKE THIS PATH IN THE iframe.contentDocument.write()
        open(ownerDocument);
        ASSERT(m_tokenizer);
        if (!m_tokenizer)
            return;
        UChar documentPrefix[] = { '<', 'h', 't', 'm', 'l', '>' };
        m_tokenizer->write(SegmentedString(documentPrefix, sizeof(documentPrefix) / sizeof(documentPrefix[0])), false);
    }

When the iframe.contentDocument.write() happens, the tokenizer has of course already been destroyed.  So we create a new one, and prime it with <html> (not sure why?)  This breaks our ability to document.write things like doctypes which must go before the <html>.

Not sure what the elegant solution here would be.
Comment 8 Eric Seidel (no email) 2009-03-17 16:35:19 PDT
writing the <html> first as part of document.write() came as part of:
http://trac.webkit.org/changeset/798

I doubt there are associated layout tests, unfortunately.
Comment 9 Eric Seidel (no email) 2009-03-17 17:30:29 PDT
Created attachment 28712 [details]
First try, removing unneeded <html> write

 LayoutTests/ChangeLog                              |   15 +++++++++++++
 .../Document/document-write-doctype-expected.txt   |   19 ++++++++++++++++
 .../fast/dom/Document/document-write-doctype.html  |   13 +++++++++++
 .../Document/resources/document-write-doctype.js   |   23 ++++++++++++++++++++
 WebCore/ChangeLog                                  |   21 ++++++++++++++++++
 WebCore/dom/Document.cpp                           |    9 +------
 6 files changed, 93 insertions(+), 7 deletions(-)
Comment 10 Alexey Proskuryakov 2009-03-18 01:35:12 PDT
+debug("any document.write calls after <html> has been encountered cannot change the doctype")

<html> didn't make it into results, should be &lt;html>.

+        Remove an implicit write of "<html>" on the first document.write call

Will the document have a document element after document.write("")? Should it?

+iframe.contentWindow.document.write("<html>");
+shouldBeEqualToString("iframe.contentWindow.document.compatMode", "BackCompat");

I think that it would be nice to check the resulting DOM in the tests, not just document.compatMode.
Comment 11 Eric Seidel (no email) 2009-03-18 01:39:17 PDT
(In reply to comment #10)
> +debug("any document.write calls after <html> has been encountered cannot
> change the doctype")
> 
> <html> didn't make it into results, should be &lt;html>.
> 
> +        Remove an implicit write of "<html>" on the first document.write call
> 
> Will the document have a document element after document.write("")? Should it?
> 
> +iframe.contentWindow.document.write("<html>");
> +shouldBeEqualToString("iframe.contentWindow.document.compatMode",
> "BackCompat");
> 
> I think that it would be nice to check the resulting DOM in the tests, not just
> document.compatMode.

Interesting.  I'll fix all 3 of these tomorrow and upload an even better patch!  (This one still stands for review though.)
Comment 12 Eric Seidel (no email) 2009-03-18 12:31:05 PDT
Created attachment 28727 [details]
First try, removing unneeded <html> write

 LayoutTests/ChangeLog                              |   15 +++
 .../Document/document-write-doctype-expected.txt   |   33 +++++++
 .../fast/dom/Document/document-write-doctype.html  |   13 +++
 .../Document/resources/document-write-doctype.js   |   93 ++++++++++++++++++++
 WebCore/ChangeLog                                  |   21 +++++
 WebCore/dom/Document.cpp                           |    9 +--
 6 files changed, 177 insertions(+), 7 deletions(-)
Comment 13 Dave Hyatt 2009-03-23 17:31:27 PDT
Comment on attachment 28727 [details]
First try, removing unneeded <html> write

r=me. Hopefully stuff doesn't break...
Comment 14 Eric Seidel (no email) 2009-03-23 19:10:04 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/dom/Document/document-write-doctype-expected.txt
	A	LayoutTests/fast/dom/Document/document-write-doctype.html
	A	LayoutTests/fast/dom/Document/resources/document-write-doctype.js
	M	WebCore/ChangeLog
	M	WebCore/dom/Document.cpp
Committed r41932