[webkit-reviews] review granted: [Bug 125279] Change how the form element pointer affects parsing template elements, to reduce weirdness in templates : [Attachment 218489] Updates the parser

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 125279: Change how the form element pointer affects parsing template
elements, to reduce weirdness in templates
https://bugs.webkit.org/show_bug.cgi?id=125279

Attachment 218489: Updates the parser
https://bugs.webkit.org/attachment.cgi?id=218489&action=review

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=218489&action=review


> Source/WebCore/html/parser/HTMLConstructionSite.cpp:416
> +    if (!insideTemplateElement())
> +	   m_form = static_pointer_cast<HTMLFormElement>(element);
> +    static_pointer_cast<HTMLFormElement>(element)->setDemoted(isDemoted);

Please do the cast only once and put it to a local

> Source/WebCore/html/parser/HTMLConstructionSite.cpp:544
> +inline bool HTMLConstructionSite::insideTemplateElement()
> +{
> +    return !ownerDocumentForCurrentNode().frame();
> +}

Maybe we should just have a boolean for this state? This is bit hard to
understand.

> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:1869
> +#if ENABLE(TEMPLATE_ELEMENT)
> +	   if (!isParsingTemplateContents()) {
> +#endif

Not a big fan of #ifs like this