[webkit-reviews] review denied: [Bug 42909] REGRESSION (r61285): <script /> parses differently in HTML5 : [Attachment 68433] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Andy Estes <aestes at apple.com>'s
request for review:
Bug 42909: REGRESSION (r61285): <script /> parses differently in HTML5
https://bugs.webkit.org/show_bug.cgi?id=42909

Attachment 68433: Patch
https://bugs.webkit.org/attachment.cgi?id=68433&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=68433&action=review

This looks good.  One minor quibble.

> WebCore/html/parser/HTMLTreeBuilder.cpp:2715
> +	   if (m_usePreHTML5ParserQuirks && token.selfClosing()) {
> +	       processSelfClosingScriptTag(token);
> +	       return true;
> +	   }
> +	   
>	   processScriptStartTag(token);

I would have expected the branch on the quirk to go after this line and then
just process a fake end script tag.

> WebCore/html/parser/HTMLTreeBuilder.cpp:2760
> +    m_tree.insertScriptElement(token);
> +    m_originalInsertionMode = m_insertionMode;
> +    m_lastScriptElementStartLine = m_tokenizer->lineNumber();
> +    setInsertionMode(TextMode);

Can we share more code with processScriptStartTag ?

> LayoutTests/fast/parser/pre-html5-parser-quirks.html:18
> +<iframe
src="resources/pre-html5-parser-quirk-self-closing-script.html"></iframe>

Can you add a test for the fragment mode too?  Also, please add a test for
self-closing script tags in the head element b/c that goes through a slightly
different code path.