-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Properly support template elements in DOM #14906
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
cbff8b8
to
7ba97ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable, I've got some questions
Also feel free to already merge the first two commits if you want |
ff6eaa5
to
0ca7791
Compare
The template element in HTML 5 is special in the sense that it does not add its contents into the DOM tree, but instead keeps them in a separate shadow DOM document fragment. Interacting with the DOM tree cannot touch the elements in the document fragment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks great!
This still needs a bit more work apparently; and I need to check if the new code works causes problems with nsDef being filled in... |
d6c52c1
to
52f2737
Compare
I realized that the destruction solution was not good yet because of the following two reasons:
So it doesn't suffice to add the logic for destroying templates in the dtor of dom. Instead, we have to callback from ext/libxml to destroy our associated template content. To accomplish that, I introduce the ability for namespaces to signal additional information to ext/libxml. We already used the |
Upon merging I will keep the first two commits separately and then squash the others into a third one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hook idea makes sense, I had also forgotten about SimpleXML
ext/dom/private_data.c
Outdated
@@ -137,4 +142,28 @@ void php_dom_remove_templated_content(php_dom_private_data *private_data, const | |||
} | |||
} | |||
|
|||
zend_long php_dom_get_template_count(const php_dom_private_data *private_data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this return a uint32_t
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm probably
The template element in HTML 5 is special in the sense that it does not
add its contents into the DOM tree, but instead keeps them in a separate
shadow DOM document fragment. Interacting with the DOM tree cannot touch
the elements in the document fragment.