Hoist <template> to head when found between </head> and <body> for consistency with...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Nov 2013 07:00:09 +0000 (07:00 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Nov 2013 07:00:09 +0000 (07:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=123949

Reviewed by Antti Koivisto.

Source/WebCore:

Merge https://chromium.googlesource.com/chromium/blink/+/835fb468fd211054a920fb7612a6dc5043662495

Move template elements between head and body elements into the head to be consistent with script elements.
The HTML5 specification was changed in http://html5.org/tools/web-apps-tracker?from=8217&to=8218.

Inline comments below are cited from https://www.w3.org/Bugs/Public/show_bug.cgi?id=23002
and https://codereview.chromium.org/25900003 for clarity.

* html/parser/HTMLTreeBuilder.cpp:
(WebCore::HTMLTreeBuilder::processStartTag): Add the template element to the list of elements to be hoisted into
the head element.
(WebCore::HTMLTreeBuilder::resetInsertionModeAppropriately):

Replace the assertion that isParsingFragment is true when item->node() == m_tree.openElements()->rootNode() since,
with this change, we can now invoke resetInsertionMode when parsing a normal document (not fragment) and there is
only the html element on the stack of open elements.

For the second change, consider: <head></head><template>

This example breaks in the old HTML parser because the template element is handled by "after head" state which
pushes the head element back on, processes the template element for "in head", then pops the head element off.
EOF is reached, which processes a fake close tag for the template element, which pops the template element off
and resets the insertion mode appropriately

The problem here is that "reset the insertion mode" is going to inspect the bottom-most element on the stack which
is now the html element and it will set the mode to "before head". Nothing good happens after this.

We fix this problem by having the reset algorithm check if the head element pointer is set, and if so, go to after
head instead of before head.

LayoutTests:

Merge https://chromium.googlesource.com/chromium/blink/+/835fb468fd211054a920fb7612a6dc5043662495
and added two more test cases discussed in https://www.w3.org/Bugs/Public/show_bug.cgi?id=23002.

* html5lib/resources/template.dat:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@159610 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/html5lib/resources/template.dat
Source/WebCore/ChangeLog
Source/WebCore/html/parser/HTMLTreeBuilder.cpp

index 6a98289..77599da 100644 (file)
@@ -1,3 +1,15 @@
+2013-11-20  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Hoist <template> to head when found between </head> and <body> for consistency with <script>
+        https://bugs.webkit.org/show_bug.cgi?id=123949
+
+        Reviewed by Antti Koivisto.
+
+        Merge https://chromium.googlesource.com/chromium/blink/+/835fb468fd211054a920fb7612a6dc5043662495
+        and added two more test cases discussed in https://www.w3.org/Bugs/Public/show_bug.cgi?id=23002.
+
+        * html5lib/resources/template.dat:
+
 2013-11-20  Radu Stavila  <stavila@adobe.com>
 
         [CSS Regions] Implement visual overflow for first & last regions
index 714e130..6cf3886 100644 (file)
 |   <body>
 |     <template>
 |       content
+
+#data
+<head></head><template>
+#errors
+#document
+| <html>
+|   <head>
+|     <template>
+|       content
+|   <body>
+
+#data
+<head></head><template>Foo</template>
+#errors
+#document
+| <html>
+|   <head>
+|     <template>
+|       content
+|         "Foo"
+|   <body>
+
+#data
+<html><head></head><template></template><head>
+#errors
+#document
+| <html>
+|   <head>
+|     <template>
+|       content
+|   <body>
+
+#data
+<body></body><template>
+#errors
+#document
+| <html>
+|   <head>
+|   <body>
+|     <template>
+|       content
index 2221c77..e8ca609 100644 (file)
@@ -1,3 +1,40 @@
+2013-11-20  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Hoist <template> to head when found between </head> and <body> for consistency with <script>
+        https://bugs.webkit.org/show_bug.cgi?id=123949
+
+        Reviewed by Antti Koivisto.
+
+        Merge https://chromium.googlesource.com/chromium/blink/+/835fb468fd211054a920fb7612a6dc5043662495
+
+        Move template elements between head and body elements into the head to be consistent with script elements.
+        The HTML5 specification was changed in http://html5.org/tools/web-apps-tracker?from=8217&to=8218.
+
+        Inline comments below are cited from https://www.w3.org/Bugs/Public/show_bug.cgi?id=23002
+        and https://codereview.chromium.org/25900003 for clarity.
+
+        * html/parser/HTMLTreeBuilder.cpp:
+        (WebCore::HTMLTreeBuilder::processStartTag): Add the template element to the list of elements to be hoisted into
+        the head element.
+        (WebCore::HTMLTreeBuilder::resetInsertionModeAppropriately):
+
+        Replace the assertion that isParsingFragment is true when item->node() == m_tree.openElements()->rootNode() since,
+        with this change, we can now invoke resetInsertionMode when parsing a normal document (not fragment) and there is
+        only the html element on the stack of open elements.
+
+        For the second change, consider: <head></head><template>
+
+        This example breaks in the old HTML parser because the template element is handled by "after head" state which
+        pushes the head element back on, processes the template element for "in head", then pops the head element off.
+        EOF is reached, which processes a fake close tag for the template element, which pops the template element off
+        and resets the insertion mode appropriately
+
+        The problem here is that "reset the insertion mode" is going to inspect the bottom-most element on the stack which
+        is now the html element and it will set the mode to "before head". Nothing good happens after this.
+
+        We fix this problem by having the reset algorithm check if the head element pointer is set, and if so, go to after
+        head instead of before head.
+
 2013-11-20  Radu Stavila  <stavila@adobe.com>
 
         [CSS Regions] Implement visual overflow for first & last regions
index 4a5d091..a3ab34b 100644 (file)
@@ -1129,6 +1129,9 @@ void HTMLTreeBuilder::processStartTag(AtomicHTMLToken* token)
             || token->name() == noframesTag
             || token->name() == scriptTag
             || token->name() == styleTag
+#if ENABLE(TEMPLATE_ELEMENT)
+            || token->name() == templateTag
+#endif
             || token->name() == titleTag) {
             parseError(token);
             ASSERT(m_tree.head());
@@ -1632,9 +1635,15 @@ void HTMLTreeBuilder::resetInsertionModeAppropriately()
     while (1) {
         RefPtr<HTMLStackItem> item = nodeRecord->stackItem();
         if (item->node() == m_tree.openElements()->rootNode()) {
-            ASSERT(isParsingFragment());
             last = true;
-            item = HTMLStackItem::create(m_fragmentContext.contextElement(), HTMLStackItem::ItemForContextElement);
+#if ENABLE(TEMPLATE_ELEMENT)
+            bool shouldCreateItem = isParsingFragment();
+#else
+            ASSERT(isParsingFragment());
+            bool shouldCreateItem = true;
+#endif
+            if (shouldCreateItem)
+                item = HTMLStackItem::create(m_fragmentContext.contextElement(), HTMLStackItem::ItemForContextElement);
         }
 #if ENABLE(TEMPLATE_ELEMENT)
         if (item->hasTagName(templateTag))
@@ -1679,6 +1688,8 @@ void HTMLTreeBuilder::resetInsertionModeAppropriately()
             return setInsertionMode(InFramesetMode);
         }
         if (item->hasTagName(htmlTag)) {
+            if (m_tree.headStackItem())
+                return setInsertionMode(AfterHeadMode);
             ASSERT(isParsingFragment());
             return setInsertionMode(BeforeHeadMode);
         }