WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
44788
Implement HTML5-compliant doctype switching
https://bugs.webkit.org/show_bug.cgi?id=44788
Summary
Implement HTML5-compliant doctype switching
Dave Hyatt
Reported
2010-08-27 12:40:34 PDT
This bug is about bringing our crufty old doctype switching code up to the HTML5 standard. The forthcoming patch will rename all the modes to match the HTML5 specification and let the HTML5 parsing algorithm actually drive the mode setting.
Attachments
Patch
(74.51 KB, patch)
2010-08-27 13:00 PDT
,
Dave Hyatt
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Dave Hyatt
Comment 1
2010-08-27 13:00:30 PDT
Created
attachment 65754
[details]
Patch
WebKit Review Bot
Comment 2
2010-08-27 13:02:59 PDT
Attachment 65754
[details]
did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/rendering/RenderListItem.cpp:167: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] WebCore/dom/Element.cpp:371: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] WebCore/dom/Element.cpp:392: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] WebCore/rendering/RenderBlock.cpp:4910: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] WebCore/rendering/InlineFlowBox.cpp:925: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] Total errors found: 5 in 50 files If any of these errors are false positives, please file a bug against check-webkit-style.
Simon Fraser (smfr)
Comment 3
2010-08-27 13:19:26 PDT
Comment on
attachment 65754
[details]
Patch
> + // Check for Quirks Mode. > + if (docType->name() != "html" > + || docType->publicId().startsWith("+//Silmaril//dtd html Pro v0r11 19970101//", false) > + || docType->publicId().startsWith("-//AdvaSoft Ltd//DTD HTML 3.0 asWedit + extensions//", false) > + || docType->publicId().startsWith("-//AS//DTD HTML 3.0 asWedit + extensions//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML 2.0 Level 1//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML 2.0 Level 2//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML 2.0 Strict Level 1//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML 2.0 Strict Level 2//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML 2.0 Strict//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML 2.0//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML 2.1E//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML 3.0//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML 3.2 Final//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML 3.2//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML 3//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML Level 0//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML Level 1//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML Level 2//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML Level 3//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML Strict Level 0//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML Strict Level 1//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML Strict Level 2//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML Strict Level 3//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML Strict//", false) > + || docType->publicId().startsWith("-//IETF//DTD HTML//", false) > + || docType->publicId().startsWith("-//Metrius//DTD Metrius Presentational//", false) > + || docType->publicId().startsWith("-//Microsoft//DTD Internet Explorer 2.0 HTML Strict//", false) > + || docType->publicId().startsWith("-//Microsoft//DTD Internet Explorer 2.0 HTML//", false) > + || docType->publicId().startsWith("-//Microsoft//DTD Internet Explorer 2.0 Tables//", false) > + || docType->publicId().startsWith("-//Microsoft//DTD Internet Explorer 3.0 HTML Strict//", false) > + || docType->publicId().startsWith("-//Microsoft//DTD Internet Explorer 3.0 HTML//", false) > + || docType->publicId().startsWith("-//Microsoft//DTD Internet Explorer 3.0 Tables//", false) > + || docType->publicId().startsWith("-//Netscape Comm. Corp.//DTD HTML//", false) > + || docType->publicId().startsWith("-//Netscape Comm. Corp.//DTD Strict HTML//", false) > + || docType->publicId().startsWith("-//O'Reilly and Associates//DTD HTML 2.0//", false) > + || docType->publicId().startsWith("-//O'Reilly and Associates//DTD HTML Extended 1.0//", false) > + || docType->publicId().startsWith("-//O'Reilly and Associates//DTD HTML Extended Relaxed 1.0//", false) > + || docType->publicId().startsWith("-//SoftQuad Software//DTD HoTMetaL PRO 6.0::19990601::extensions to HTML 4.0//", false) > + || docType->publicId().startsWith("-//SoftQuad//DTD HoTMetaL PRO 4.0::19971010::extensions to HTML 4.0//", false) > + || docType->publicId().startsWith("-//Spyglass//DTD HTML 2.0 Extended//", false) > + || docType->publicId().startsWith("-//SQ//DTD HTML 2.0 HoTMetaL + extensions//", false) > + || docType->publicId().startsWith("-//Sun Microsystems Corp.//DTD HotJava HTML//", false) > + || docType->publicId().startsWith("-//Sun Microsystems Corp.//DTD HotJava Strict HTML//", false) > + || docType->publicId().startsWith("-//W3C//DTD HTML 3 1995-03-24//", false) > + || docType->publicId().startsWith("-//W3C//DTD HTML 3.2 Draft//", false) > + || docType->publicId().startsWith("-//W3C//DTD HTML 3.2 Final//", false) > + || docType->publicId().startsWith("-//W3C//DTD HTML 3.2//", false) > + || docType->publicId().startsWith("-//W3C//DTD HTML 3.2S Draft//", false) > + || docType->publicId().startsWith("-//W3C//DTD HTML 4.0 Frameset//", false) > + || docType->publicId().startsWith("-//W3C//DTD HTML 4.0 Transitional//", false) > + || docType->publicId().startsWith("-//W3C//DTD HTML Experimental 19960712//", false) > + || docType->publicId().startsWith("-//W3C//DTD HTML Experimental 970421//", false) > + || docType->publicId().startsWith("-//W3C//DTD W3 HTML//", false) > + || docType->publicId().startsWith("-//W3O//DTD W3 HTML 3.0//", false) > + || equalIgnoringCase(docType->publicId(), "-//W3O//DTD W3 HTML Strict 3.0//EN//") > + || docType->publicId().startsWith("-//WebTechs//DTD Mozilla HTML 2.0//", false) > + || docType->publicId().startsWith("-//WebTechs//DTD Mozilla HTML//", false) > + || equalIgnoringCase(docType->publicId(), "-/W3C/DTD HTML 4.0 Transitional/EN") > + || equalIgnoringCase(docType->publicId(), "HTML") > + || equalIgnoringCase(docType->systemId(), "
http://www.ibm.com/data/dtd/v11/ibmxhtml1-transitional.dtd
") > + || (docType->systemId().isEmpty() && docType->publicId().startsWith("-//W3C//DTD HTML 4.01 Frameset//", false)) > + || (docType->systemId().isEmpty() && docType->publicId().startsWith("-//W3C//DTD HTML 4.01 Transitional//", false))) { > + setCompatibilityMode(QuirksMode); > + return;
Ick.Can we at least pull docType->publicId() into a local variable?
Dave Hyatt
Comment 4
2010-08-27 13:29:47 PDT
Fixed in
r66247
.
Darin Adler
Comment 5
2010-08-27 13:55:53 PDT
Comment on
attachment 65754
[details]
Patch
> @@ -1394,6 +1404,7 @@ void Document::unscheduleStyleRecalc() > documentsThatNeedStyleRecalc->remove(this); > > m_styleRecalcTimer.stop(); > + m_pendingStyleRecalcShouldForce = false; > } > > void Document::styleRecalcTimerFired(Timer<Document>*) > @@ -1431,6 +1442,9 @@ void Document::recalcStyle(StyleChange c > ASSERT(!renderer() || renderArena()); > if (!renderer() || !renderArena()) > goto bail_out; > + > + if (m_pendingStyleRecalcShouldForce) > + change = Force; > > if (change == Force) { > // style selector may set this again during recalc > @@ -1488,9 +1502,7 @@ void Document::updateStyleIfNeeded() > if (m_frame) > m_frame->animation()->beginAnimationUpdate(); > > - recalcStyle(m_pendingStyleRecalcShouldForce ? Force : NoChange); > - > - m_pendingStyleRecalcShouldForce = false; > + recalcStyle(NoChange); > > // Tell the animation controller that updateStyleIfNeeded is finished and it can do any post-processing > if (m_frame)
These changes seem good, but separate from the doctype changes. Can you land them separately? Can you test them?
WebKit Review Bot
Comment 6
2010-08-27 14:19:39 PDT
http://trac.webkit.org/changeset/66247
might have broken Qt Linux Release The following changes are on the blame list:
http://trac.webkit.org/changeset/66248
http://trac.webkit.org/changeset/66246
http://trac.webkit.org/changeset/66247
Eric Seidel (no email)
Comment 7
2010-08-27 14:22:21 PDT
So this was landed I take it? Very strange that this all happened "in secret" w/o any r+ on bugzilla, or the original authors of this code CC'd. :p
Eric Seidel (no email)
Comment 8
2010-08-27 14:23:17 PDT
By "this code" I meant, the parser, which was an overstatement, since most of this code doesn't affect the parser. :) Regardless, thanks for fixing. Still seemed to happen kinda strangely.
Adam Barth
Comment 9
2010-08-27 14:26:59 PDT
It's unclear to me why HTMLDocument::setCompatibilityModeFromDoctype() part of HTMLDocument instead of part of HTMLTreeBuilder, which is where this logic is in the spec...
Eric Seidel (no email)
Comment 10
2010-08-27 14:30:49 PDT
Comment on
attachment 65754
[details]
Patch WebCore/dom/Document.cpp:592 + // All user stylesheets have to reparse using the different mode. You mentioned that the renderers have to update too (over irc). Or is that no longer the case? WebCore/dom/Document.h:586 + bool inNoQuirksMode() const { return m_compatibilityMode == NoQuirksMode; } The new names are much better. :) WebCore/html/HTMLDocument.cpp:359 + || docType->publicId().startsWith("-//AdvaSoft Ltd//DTD HTML 3.0 asWedit + extensions//", false) I agree with simon. :) This seems to want a helper function. :) WebCore/html/HTMLTreeBuilder.cpp:434 + m_dummyDocumentForFragmentParsing->setCompatibilityMode(fragment->document()->compatibilityMode()); Do we have tests for this? I retract my earlier comments. I lept to judgement. Fantastic patch. Thanks again.
Tony Chang
Comment 11
2010-08-27 14:35:08 PDT
I think these tests started to fail after
http://trac.webkit.org/changeset/66247
: tables/mozilla_expected_failures/bugs/
bug56024
.html fast/multicol/span/span-as-nested-columns-child.html
Darin Adler
Comment 12
2010-08-28 15:56:17 PDT
(In reply to
comment #11
)
> I think these tests started to fail after
http://trac.webkit.org/changeset/66247
: > tables/mozilla_expected_failures/bugs/
bug56024
.html > fast/multicol/span/span-as-nested-columns-child.html
Hyatt, will you be fixing this? Do we need a bug report?
Ryosuke Niwa
Comment 13
2010-08-28 17:17:43 PDT
(In reply to
comment #11
)
> I think these tests started to fail after
http://trac.webkit.org/changeset/66247
: > tables/mozilla_expected_failures/bugs/
bug56024
.html > fast/multicol/span/span-as-nested-columns-child.html
I believe these tests no longer fails after rebaselined in
http://trac.webkit.org/changeset/66274
and
http://trac.webkit.org/changeset/66278
. The rational for rebaseline being their getting the old beahvior back after rebaselines in
http://trac.webkit.org/changeset/66154
and
http://trac.webkit.org/changeset/66147
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug