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.
Created attachment 65754 [details] Patch
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.
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?
Fixed in r66247.
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?
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
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
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.
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...
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.
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
(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?
(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.