Summary: | document.write() should be able to make a document into strict mode | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||
Component: | DOM | Assignee: | 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
Eric Seidel (no email)
2009-03-03 15:11:44 PST
Created attachment 28242 [details]
test case
I click on the test in Safari 3.2.1, and it says "PASSED". A regression? Probably. It fails in Safari Version 4 Developer Preview (5526.11.2) (which I haven't upgraded to the real Safari 4 yet) too. 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. void HTMLDocument::determineParseMode() is the code which needs to be checked here to see if it's running correctly. 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 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. 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. 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(-)
+debug("any document.write calls after <html> has been encountered cannot change the doctype") <html> didn't make it into results, should be <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. (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 <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.) 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 on attachment 28727 [details]
First try, removing unneeded <html> write
r=me. Hopefully stuff doesn't break...
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 |