Description of the bug
A document parsed with html5qp() loses the closing ? of every processing instruction when it is
serialized with html() or writeHTML(). A <?php … ?> block comes back out as <?php … >, which
is not valid PHP.
$html = '<html><body><h1><?php echo $title; ?></h1></body></html>';
echo html5qp($html)->top()->html();
// <h1><?php echo $title; ></h1> <-- terminator dropped
echo htmlqp($html)->top()->html();
// <h1><?php echo $title; ?></h1> <-- correct
The same happens with writeHTML(). Note this is the mirror image of #65: there the terminator was
doubled, here it is dropped.
Scope
Only two methods, and only for html5qp(). Every other serializer is fine, because
html5()/writeHTML5() go through Masterminds and the xml()/inner*() family go through
libxml's XML serializer — both of which append their own ?>.
|
htmlqp() |
html5qp() |
qp() (XML) |
html() |
ok |
dropped |
ok |
writeHTML() |
ok |
dropped |
ok |
html5(), writeHTML5() |
ok |
ok |
ok |
xml(), writeXML() |
ok |
ok |
ok |
innerHTML(), innerHTML5(), innerXML() |
ok |
ok |
ok |
This is pre-existing, not a regression — it reproduces on 4.1.0 and on main. It is called out
separately from #65 because the two have opposite symptoms and the fix for one does not reach the
other.
Root cause
libxml's HTML serializer writes a processing instruction verbatim as <?target data> and, unlike
its XML serializer and the Masterminds serializer, never appends the closing ? itself. It relies
on the terminator already being part of the node's data, which is exactly what libxml's HTML
parser leaves there.
Masterminds does not leave it there, so the two do not pair up:
$html5 = new Masterminds\HTML5();
$doc = $html5->loadHTML('<html><body><h1><?php echo $t; ?></h1></body></html>');
$doc->saveHTML(); // <h1><?php echo $t; ></h1> libxml drops it
$html5->saveHTML($doc); // <h1><?php echo $t; ?></h1> Masterminds appends itWhy the fix in #68 does not cover this
#68 normalises PI data on load so that no parser leaves the terminator in data, and re-adds it
around saveHTML()/saveHTMLFile() — the one output path that needs it. It knows a document is
normalised because QueryPath parses into a QueryPath\Document subclass; the type is the marker.
QueryPath::withHTML5() hands the parse to Masterminds, which builds a plain DOMDocument. That
document does satisfy the invariant — Masterminds never writes the terminator into data — but
QueryPath has no way to say so, so saveDocumentHTML() treats it like a caller-supplied document
and writes it as-is.
Candidate fixes
Both verified locally against the current issue-65 branch.
1. Parse into a QueryPath\Document via Masterminds' target_document option. Masterminds
honours DOMTreeBuilder::OPT_TARGET_DOC, so withHTML5() can hand it a QueryPath\Document and
the terminator problem disappears. The catch is the doctype: Masterminds only creates
<!DOCTYPE html> when it creates the document itself, so with a target document $doc->doctype
is NULL and html()/writeHTML() lose the doctype they emit today. html5()/writeHTML5() are
unaffected, since Masterminds' own serializer writes the doctype unconditionally. Attaching a
doctype afterwards does not work — DOMDocument::importNode() returns false for a
DOMDocumentType, and the only way to attach one is DOMImplementation::createDocument(), which
returns a plain DOMDocument rather than the subclass. So this route needs a way to give the
target document a doctype first.
2. Mark the document through registerNodeClass('DOMElement', …) instead of the document type.
Unlike registerNodeClass('DOMDocument', …), which has no effect on a document whose wrapper is
already live, registering an element class works retroactively on a Masterminds document and leaves
the doctype intact:
$doc->registerNodeClass('DOMElement', Marker::class);
$doc->documentElement instanceof Marker; // true
$doc->doctype->name; // 'html'The cost is that it changes the class of every element node a caller gets back, which is a much
larger surface change than a marker on the document.
Related
Description of the bug
A document parsed with
html5qp()loses the closing?of every processing instruction when it isserialized with
html()orwriteHTML(). A<?php … ?>block comes back out as<?php … >, whichis not valid PHP.
The same happens with
writeHTML(). Note this is the mirror image of #65: there the terminator wasdoubled, here it is dropped.
Scope
Only two methods, and only for
html5qp(). Every other serializer is fine, becausehtml5()/writeHTML5()go through Masterminds and thexml()/inner*()family go throughlibxml's XML serializer — both of which append their own
?>.htmlqp()html5qp()qp()(XML)html()writeHTML()html5(),writeHTML5()xml(),writeXML()innerHTML(),innerHTML5(),innerXML()This is pre-existing, not a regression — it reproduces on 4.1.0 and on
main. It is called outseparately from #65 because the two have opposite symptoms and the fix for one does not reach the
other.
Root cause
libxml's HTML serializer writes a processing instruction verbatim as
<?target data>and, unlikeits XML serializer and the Masterminds serializer, never appends the closing
?itself. It relieson the terminator already being part of the node's
data, which is exactly what libxml's HTMLparser leaves there.
Masterminds does not leave it there, so the two do not pair up:
Why the fix in #68 does not cover this
#68 normalises PI data on load so that no parser leaves the terminator in
data, and re-adds itaround
saveHTML()/saveHTMLFile()— the one output path that needs it. It knows a document isnormalised because QueryPath parses into a
QueryPath\Documentsubclass; the type is the marker.QueryPath::withHTML5()hands the parse to Masterminds, which builds a plainDOMDocument. Thatdocument does satisfy the invariant — Masterminds never writes the terminator into
data— butQueryPath has no way to say so, so
saveDocumentHTML()treats it like a caller-supplied documentand writes it as-is.
Candidate fixes
Both verified locally against the current
issue-65branch.1. Parse into a
QueryPath\Documentvia Masterminds'target_documentoption. Mastermindshonours
DOMTreeBuilder::OPT_TARGET_DOC, sowithHTML5()can hand it aQueryPath\Documentandthe terminator problem disappears. The catch is the doctype: Masterminds only creates
<!DOCTYPE html>when it creates the document itself, so with a target document$doc->doctypeis
NULLandhtml()/writeHTML()lose the doctype they emit today.html5()/writeHTML5()areunaffected, since Masterminds' own serializer writes the doctype unconditionally. Attaching a
doctype afterwards does not work —
DOMDocument::importNode()returnsfalsefor aDOMDocumentType, and the only way to attach one isDOMImplementation::createDocument(), whichreturns a plain
DOMDocumentrather than the subclass. So this route needs a way to give thetarget document a doctype first.
2. Mark the document through
registerNodeClass('DOMElement', …)instead of the document type.Unlike
registerNodeClass('DOMDocument', …), which has no effect on a document whose wrapper isalready live, registering an element class works retroactively on a Masterminds document and leaves
the doctype intact:
The cost is that it changes the class of every element node a caller gets back, which is a much
larger surface change than a marker on the document.
Related
writeHTML5()returns NULL instead of the DOMQuery, and reports an unwritable path as a raw TypeError #80 —writeHTML5()returnsNULLinstead of theDOMQuery