Bug 24336

Summary: document.write() should be able to make a document into strict mode
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, hyatt, jparent, ojan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
test case
none
First try, removing unneeded <html> write
none
First try, removing unneeded <html> write hyatt: review+

Eric Seidel (no email)
Reported 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.
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
DetailsFormatted DiffDiff
First try, removing unneeded <html> write (9.83 KB, patch)
2009-03-18 12:31 PDT, Eric Seidel (no email) 		hyatt: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2009-03-03 15:17:16 PST
Created attachment 28242 [details] test case
Alexey Proskuryakov
Comment 2 2009-03-04 10:24:41 PST
I click on the test in Safari 3.2.1, and it says "PASSED". A regression?
Eric Seidel (no email)
Comment 3 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.
Ojan Vafai
Comment 4 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.
Eric Seidel (no email)
Comment 5 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.
Eric Seidel (no email)
Comment 6 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
Eric Seidel (no email)
Comment 7 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.
Eric Seidel (no email)
Comment 8 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.
Eric Seidel (no email)
Comment 9 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(-)
Alexey Proskuryakov
Comment 10 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.
Eric Seidel (no email)
Comment 11 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.)
Eric Seidel (no email)
Comment 12 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(-)
Dave Hyatt
Comment 13 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...
Eric Seidel (no email)
Comment 14 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
Note You need to log in before you can comment on or make changes to this bug.