noscript in head should allow link, meta, style tags and in body other tags #98
Comments
|
I suppose another approach would be to create a subclass of the
|
|
For additional reference, the HTML5 spec notes this as well in https://www.w3.org/TR/html5/scripting-1.html#the-noscript-element |
|
Just FYI we're using the Mastermind HTML5 library (and QueryPath) in the AMP PHP Library |
sidkshatriya
changed the title
|
This is general issue with the There is another workaround: $html = '<html><head>
<noscript><style>a { color: red;}</style></noscript>
</head><body></body></html>';
// Using QueryPath
$qp = qp($html);
// do DOM manipulation
...
...
// Now its time to change the noscript tags to contain text (just before output)
$noscripts = $qp->find('noscript');
foreach($noscripts as $noscript) {
// If you care about CDATA not appearing then QueryPath's innerHTML() might prove
// problematic but you can implement your own innerHTML method by using saveHTML()
// call on DOMNodes.
$innerhtml = $noscript->innerHTML();
$noscript->removeChildren();
// now all nested children are just text
$noscript->text($innerhtml);
}
// All noscript nested children are text so html5() function call will not give problems.
print($qp->top()->html5()); |
|
In the body when scripting is disabled (case we need to shoot for) the Pull requests are welcome and if not I'll try to look at this soon. |
|
@mattfarina It would probably take me some time to understand the internals of the module. So if you're able to get some time to fix this it would be very much appreciated :-) [Off topic: If possible can we have a release for the 2.x series -- the last release was some time ago.. Also what is the difference between the 2.x branch and master branch?] |
|
My plan is to cut a release after fixing this bug. The master branch meets all the PSRs. There were some naming things that are breaking (e.g., Html vs HTML). That will be the basis for a 3.x series when time becomes available to make a handful of changes. |
|
I'm still reading through the HTML5 spec on this. There is some different direction in different sections. In particular on tags allowed in the Since this is not a validating parser I'm tempted to make |
|
I think thats fair -- just check if there are any nested noscripts (and any other cases you may care). Would it be a fatal parse error? It would probably be OK if you didn't do anything at all (didn't check for nesting etc). Since this is not a validating parser why do you care to validate at all here? This would actually be more consistent... |
|
@sidkshatriya Can you take a look at the PR on #99. |
Fixes #98: a DOM is allowed within noscript tags
The
<noscript>tag in<head>treats everything it contains as raw text. In fact it says in https://html.spec.whatwg.org/multipage/scripting.html#the-noscript-element that noscript can contain the link, meta and style tags.If I load a trivial html document
I get an ugly PHP notice:
This PHP notice is actually not benign. All the contents of the
<noscript>tag are lost and we get the output:It might be a good idea to load the html like this:
But this does not serve my purpose because the DOM is now loaded with only a
<noscript>tag and everything underneath is a string. So you cannot query, manipulate the inner<style>tag.There is a kludge in dealing with the above problem (and its inefficient if you have large documents):
Could we fix the Mastermind code so we don't have to adopt the kludge?
Is there a smarter way to fix this problem without having to fix mastermind code?
This functionality is important for manipulating AMP HTML documents which always have some tags in the
<head><noscript>The text was updated successfully, but these errors were encountered: