Bug 78734 - Modify HTML parser to support <template> element
: Modify HTML parser to support <template> element
Status: RESOLVED DUPLICATE of bug 86031
: WebKit
New Bugs
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 78733
  Show dependency treegraph
 
Reported: 2012-02-15 13:15 PST by
Modified: 2012-11-28 12:30 PST (History)


Attachments
WIP: First stab. (11.82 KB, patch)
2012-02-15 13:18 PST, Dimitri Glazkov (Google)
no flags Review Patch | Details | Formatted Diff | Diff
Moar better, moar tests. (15.79 KB, patch)
2012-02-22 15:15 PST, Dimitri Glazkov (Google)
no flags Review Patch | Details | Formatted Diff | Diff
Faster, harder, stronger. (16.11 KB, patch)
2012-02-23 15:57 PST, Dimitri Glazkov (Google)
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-02-15 13:15:39 PST
Modify HTML parser to support <template> element
------- Comment #1 From 2012-02-15 13:18:08 PST -------
Created an attachment (id=127223) [details]
WIP: First stab.
------- Comment #2 From 2012-02-15 13:19:13 PST -------
For the moment, this is a playground for me to learn about parser and understand what changes to the algorithm are necessary.
------- Comment #3 From 2012-02-15 13:32:01 PST -------
(From update of attachment 127223 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=127223&action=review

I know you said that this patch was a just a playground, but I thought the comments below might be helpful:

> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2248
> +        if (token.name() == templateTag) {
> +            m_tree.openElements()->pop();
> +            return;
> +        }

This change is slightly surprising.  You could be popping any sort of element off the stack, not necessarily a template element.  You probably either want:

if (m_tree.currentNode()->hasTagName(templateTag)) {
    m_tree.openElements()->pop();
    return;
}

or

m_tree.openElements()->popUntilPopped(templateTag.localName());
resetInsertionModeAppropriately();
return;

That's likely what's triggering your ASSERT on line 1686.

> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2275
> +    case TemplateContentsMode:
> +        m_tree.openElements()->pop();
> +        resetInsertionModeAppropriately();
> +        break;

This parsing might be somewhat surprising:

<div><template></div>Hello

I think it would result in the following DOM:

- <div>
    - <template>
    - #text: hello

I'm not 100% sure what the best thing to do in this case is.  Maybe:

1) If the end tag is </template>, then pop the open elements.
2) otherwise, pop the pop the open elements, reset the insertion mode, and reprocess the token

?
------- Comment #4 From 2012-02-15 13:33:38 PST -------
(From update of attachment 127223 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=127223&action=review

> LayoutTests/html5lib/resources/template.dat:187
> +|     <table>
> +|       <thead>
> +|         <template>
> +|           <tr>
> +|             <td>

This case is interesting. Where should implied <tr> appear?
------- Comment #5 From 2012-02-15 13:40:46 PST -------
(From update of attachment 127223 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=127223&action=review

>> LayoutTests/html5lib/resources/template.dat:187
>> +|             <td>
> 
> This case is interesting. Where should implied <tr> appear?

Conceptually, from a parsing point of view, what you've written here is the most natural.  The <td> creates the implied <tr>, and it would do so as a child of the currently open element, which is the <template>.
------- Comment #6 From 2012-02-15 13:50:39 PST -------
(From update of attachment 127223 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=127223&action=review

>> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2248
>> +        }
> 
> This change is slightly surprising.  You could be popping any sort of element off the stack, not necessarily a template element.  You probably either want:
> 
> if (m_tree.currentNode()->hasTagName(templateTag)) {
>     m_tree.openElements()->pop();
>     return;
> }
> 
> or
> 
> m_tree.openElements()->popUntilPopped(templateTag.localName());
> resetInsertionModeAppropriately();
> return;
> 
> That's likely what's triggering your ASSERT on line 1686.

Yay, thanks! I misunderstood what token means in this context.

>> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2275
>> +        break;
> 
> This parsing might be somewhat surprising:
> 
> <div><template></div>Hello
> 
> I think it would result in the following DOM:
> 
> - <div>
>     - <template>
>     - #text: hello
> 
> I'm not 100% sure what the best thing to do in this case is.  Maybe:
> 
> 1) If the end tag is </template>, then pop the open elements.
> 2) otherwise, pop the pop the open elements, reset the insertion mode, and reprocess the token
> 
> ?

Oh, right. Yes. I think your plan makes sense.
------- Comment #7 From 2012-02-15 14:08:11 PST -------
There's a general question of how "scoping" you want </template> to be.  In the

<div><template></div>Hello

example, there are two reasonable parses:

- <div>
  - <template>
    - #text: Hello

or

- <div>
  - <template>
- #text: Hello

In the first case, <template> is strongly scoping, which means it's hard to escape from the template element's scope.  In the second case, the template element isn't very scoping, which means the </div> token can find and close the div element.

I'd recommend trying to match the scoping of down-rev parsers as much as feasible, which, in this case, means you'll want the second (less scoping) parsing.
------- Comment #8 From 2012-02-15 14:10:16 PST -------
Some examples:

<div><object></div>Hello
<div><foo></div>Hello

These examples parse differently because <object> is pretty strongly scoping.
------- Comment #9 From 2012-02-22 15:15:51 PST -------
Created an attachment (id=128299) [details]
Moar better, moar tests.
------- Comment #10 From 2012-02-22 15:35:14 PST -------
(From update of attachment 128299 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=128299&action=review

This is starting to look really good.

> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2281
> +    case TemplateContentsMode:
> +        if (token.name() == templateTag) {
> +            m_tree.openElements()->pop();
> +            resetInsertionModeAppropriately();
> +        }
> +        break;

Can you ASSERT here that the top element is a templateTag?

> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2578
> +    case TemplateContentsMode:
> +        // FIXME: What should we do here?

Probably pop the <template> element off the stack of open element, reset the insertion mode appropriately, and reprocess the token.  :)
------- Comment #11 From 2012-02-22 17:22:12 PST -------
(From update of attachment 128299 [details])
Don't you hide <template> behind the flag?
------- Comment #12 From 2012-02-22 17:35:03 PST -------
(In reply to comment #11)
> (From update of attachment 128299 [details] [details])
> Don't you hide <template> behind the flag?

Nah. This patch is just for me to learn what to write in the spec.
------- Comment #13 From 2012-02-22 17:56:34 PST -------
> Nah. This patch is just for me to learn what to write in the spec.
Ah got it.
------- Comment #14 From 2012-02-23 15:57:51 PST -------
Created an attachment (id=128579) [details]
Faster, harder, stronger.
------- Comment #15 From 2012-11-28 12:13:04 PST -------
Dimtiri, can we close this now?
------- Comment #16 From 2012-11-28 12:17:23 PST -------
(In reply to comment #15)
> Dimtiri, can we close this now?

Yes sir! Please dupe it against the real bug for future archeologists.
------- Comment #17 From 2012-11-28 12:30:25 PST -------

*** This bug has been marked as a duplicate of bug 86031 ***