-
Couldn't load subscription status.
- Fork 8k
Feature Request 67948 #800
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
|
👍 |
|
Nice work, you may wish to consider porting it to 5.6 as well :) |
|
Thanks. @datibbaw, I will create a PR for 5.6 once this gets approved (in case there are some remarks, so that I don't have to update both PRs). |
|
Hang on, the expected result is that the parent tag is NOT included in the saved HTML, so it should yield: |
|
But it does include the parent node. $div->childNodes includes <div> as well. It just does not include the whole document (doc param is NULL) |
|
Yeah, what I meant to say is that the parent node should NOT be there; if you look closely at the example given in the bug report you will see that the parent |
|
Yup, I figured that the example in the bug report could be wrong. They probably meant "div->childNodes" instead of "body->childNodes". |
|
No, @DaveRandom meant |
|
@datibbaw is correct, the point of this is to export only the nodes in the NodeList, in the order in which they appear. Exporting the containing tag is already possible (as shown above) and, more importantly, not every NodeList has a single parent: $html = <<<HTML
<header>
<div>This is in the header</div>
</header>
<section>
<div>This is some section content</div>
</section>
HTML;
$doc = new DOMDocument;
$doc->loadHTML($html);
$xpath = new DOMXPath($doc);
$nodes = $xpath->query('//div');
$data = $doc->saveHTML($nodes);
var_dump($data);
/* expected:
<div>This is in the header</div>
<div>This is some section content</div>
*/ |
|
PR updated with the remarks! Thanks guys! Please re-check when you have the time. |
ext/dom/document.c
Outdated
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 output of htmlNodeDump() should be checked at each iteration.
|
Looking better now; I'm just wondering whether we should extend this behaviour to |
|
(As discussed in room 11 yesterday) I agree that |
https://bugs.php.net/bug.php?id=67948 Add handling for DOMNodeList parameter as well
|
Updated PR with saveXML() change. |
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.
This code is nearly identical to the other one, except for htmlNodeDump() vs xmlNodeDump() ... I think we can carve out a static inline xmlBufferPtr *dumpNodeList(...) :)
|
Since this has merge conflicts, targets a security fix only branch, and since a patch against a supported branch would have to look different, I'm closing this PR. Please take this action as encouragement to open a clean PR against a supported branch. |
https://bugs.php.net/bug.php?id=67948
Add handling for DOMNodeList parameter as well