Skip to content

Fix #63575: Root elements are not properly cloned #5438

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.

Sign up for GitHub

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

Jump to bottom
Closed
cmb69 wants to merge 1 commit into from
Closed

Fix #63575: Root elements are not properly cloned #5438

cmb69 wants to merge 1 commit into from

Conversation

@cmb69
Copy link
Member

@cmb69 cmb69 commented Apr 23, 2020

Cloning of root elements has to preserve that property, so they require
some special treatment.

@nikic
Copy link
Member

nikic commented Apr 24, 2020

cc @beberlei

@cmb69
Fix #63575: Root elements are not properly cloned
7d3dddd 
Cloning of root elements has to preserve that property, so they require
some special treatment.
@cmb69 cmb69 changed the base branch from PHP-7.3 to master May 4, 2020 21:40
@cmb69
Copy link
Member Author

cmb69 commented May 4, 2020

In hindsight, I think it's best to target master only.

@beberlei
Copy link
Contributor

beberlei commented May 4, 2020

Its really hard for me to say anything about the change. The test looks good to cover that bugfix. It would be nice to explain why its sxe->node->node, i suppose sxe->node is the root element and then sxe->node->node is the document?

@cmb69
Copy link
Member Author

cmb69 commented May 5, 2020

[…], i suppose sxe->node is the root element and then sxe->node->node is the document?

Actually, sxe->node is a php_libxml_node_ptr *, and sxe->node->node is an xmlNodePtr. So this is just about resolving PHP's wrapping object to get the libxml object.

@cmb69
Copy link
Member Author

cmb69 commented May 5, 2020

Close and re-open to trigger AppVeyor builds (which erroneously still targeted PHP-7.3 after the rebase).

@cmb69 cmb69 closed this May 5, 2020
@cmb69 cmb69 reopened this May 5, 2020
@cmb69
Copy link
Member Author

cmb69 commented Jun 10, 2020

If there are no objections, I'll merge this PR in a week.

@cmb69
Copy link
Member Author

cmb69 commented Jun 17, 2020

Thanks! Applied as 3a0bdb7.

@cmb69 cmb69 closed this Jun 17, 2020
@cmb69 cmb69 deleted the cmb/63575 branch June 17, 2020 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@beberlei beberlei beberlei approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cmb69 @nikic @beberlei @carusogabriel