1091425 - Create elements in the document on the "intended parent"

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[William Chen [:wchen]]

When creating an element in the parser, it should be created in the document of the "intended parent".

https://html.spec.whatwg.org/multipage/syntax.html#create-an-element-for-the-token

We currently create all elements in the main document and it causes us to adopt nodes where it is not necessary.

When templates and custom elements are used together, the difference in behavior is very obvious. Custom elements are supposed to be inert inside template contents because it should be created in a special document that does not have a custom element registry. However, we currently create custom elements in the main document (and then gets adopted into another document when appended to the template contents owner). This causes the custom elements to be set up and invokes the created callback.

Comment 1

[William Chen [:wchen]]

Attachment: Java parser - Provide intended parent when creating elements in the parser.

This patch adds an intended parent argument to createElement and updates the callers to pass in the appropriate node.

One of the steps to find the parent in the foster parenting algorithm is to check the parent of a <table> node, and we can only do this during tree op execution. Thus we have to delay creating the fostered child until we have started executing tree op. So I've moved the logic of creating an element and inserting it into a foster parent into tree ops.

Comment 2

[William Chen [:wchen]]

Attachment: Generated C++ parser code.

Comment 3

[William Chen [:wchen]]

Attachment: Use the correct document when creating nodes in the parser.

This patch changes the nsNodeInfoManager used to create elements, text nodes and comments so that we create it in the correct document as specified.

https://html.spec.whatwg.org/multipage/syntax.html#create-an-element-for-the-token
https://html.spec.whatwg.org/multipage/syntax.html#create-an-element-for-the-token
https://html.spec.whatwg.org/multipage/syntax.html#insert-a-comment

It also contains a new tree op to find the foster parent so that we may use it as the "intended parent" when creating elements that are going to be fostered.

Comment 4

[William Chen [:wchen]]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=72196381e469

Comment 5

[Henri Sivonen (:hsivonen)]

Comment on attachment 8514695 [details] [diff] [review]
Java parser - Provide intended parent when creating elements in the parser.

Review of attachment 8514695 [details] [diff] [review]:
-----------------------------------------------------------------

I'm pretty unhappy that the spec has gained a complication like this, but it seems that this patch implements the complication correctly. Thanks.

::: src/nu/validator/htmlparser/impl/TreeBuilder.java
@@ +5641,5 @@
>      protected abstract void insertFosterParentedChild(T child, T table,
>              T stackParent) throws SAXException;
>  
> +    // [NOCPP[
> +

Please add a comment that explains why this is [NOCPP[. (Took me a while to poke around.)

Comment 6

[Henri Sivonen (:hsivonen)]

Comment on attachment 8514699 [details] [diff] [review]
Use the correct document when creating nodes in the parser.

Review of attachment 8514699 [details] [diff] [review]:
-----------------------------------------------------------------

::: parser/html/nsHtml5TreeOperation.cpp
@@ +677,5 @@
>        nsCOMPtr<nsIAtom> name = Reget(mTwo.atom);
>        nsHtml5HtmlAttributes* attributes = mThree.attributes;
> +      nsIContent* intendedParent = mFive.node ? *(mFive.node) : nullptr;
> +
> +      // intendedParent == nullptr is a special case where we the

Stray word "we"?

::: parser/html/nsHtml5TreeOperation.h
@@ +494,5 @@
>        char*                           charPtr;
>        nsHtml5TreeOperationStringPair* stringPair;
>        nsAHtml5TreeBuilderState*       state;
>        int32_t                         integer;
> +    } mOne, mTwo, mThree, mFour, mFive;

It would be interesting to know what the perf cost (if any) of growing the tree ops by the pointer size is... (I think growing the ops is what's needed, though.)

Please file a follow-up for getting rid of nsHtml5TreeOperationStringPair now that the ops themselves can hold one more item, so the indirection of pairing two strings into one item is no longer necessary.

Comment 7

[William Chen [:wchen]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ede99651d089

Comment 8

[William Chen [:wchen]]

Follow up bug for removing nsHtml5TreeOperationStringPair: Bug 1096166

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ede99651d089

Comment 10

[William Chen [:wchen]]

https://hg.mozilla.org/projects/htmlparser/rev/c14b402c9b86