Skip to content
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

<style> tags trigger \RuntimeException when CSS is wrapped in HTML comment #62

Closed
kAlvaro opened this issue Aug 15, 2019 · 6 comments · Fixed by #65
Closed

<style> tags trigger \RuntimeException when CSS is wrapped in HTML comment #62

kAlvaro opened this issue Aug 15, 2019 · 6 comments · Fixed by #65

Comments

@kAlvaro
Copy link

kAlvaro commented Aug 15, 2019

An HTML with code like this:

<style><!--
h1 {
    color: red;
}
--></style>

... throws:

  • Type: RuntimeException
  • Message: Not valid HTML fragment! %3C%21--h1%7Bcolor%3Ared%7D
  • File: [...]\vendor\voku\simple_html_dom\src\voku\helper\SimpleHtmlDom.php
  • Line: 158

Removing HTML comments is a possible workaround:

<style>
h1 {
    color: red;
}
</style>

This technique may not be fashionable any more but as far as I know it's still valid HTML.

@WyriHaximus
Copy link
Owner

Hmm good catch, had to look it up and it's still allowed. Also this is not something that could be easily fixed as it's something inside @voku's simple HTML DOM package.

@kAlvaro
Copy link
Author

kAlvaro commented Aug 17, 2019

There're a lot of packages involved and I'm not familiar with any of them. But, after some debugging, I suspect it's really a bug in that other library, simple_html_dom. Perhaps I should report it to them.

The exception is thrown at \voku\helper\SimpleHtmlDom::replaceChildWithString() when fed with a fragment of (already compressed) CSS wrapped in an HTML comment from \WyriHaximus\HtmlCompress\Pattern\Style::compress():

// $compressedInnerHtml is like '<!--form{position:...'
$element->innerhtml = $compressedInnerHtml; 

In e.g. JavaScript this seems totally correct:

document.createElement("div").innerHTML = "<!--form{position:..."

This is the check that fails:

$tmpDomString = $this->normalizeStringForComparision($newDocument);
$tmpStr = $this->normalizeStringForComparision($string);
if ($tmpDomString !== $tmpStr) {
    throw new \RuntimeException(
        'Not valid HTML fragment!' . "\n" .
        $tmpDomString . "\n" .
        $tmpStr
    );
}

$tmpDomString is a an empty string but $tmpStr is the URL-encoded version of our HTML comment :-?

@kAlvaro
Copy link
Author

kAlvaro commented Aug 17, 2019

New finding: it fails because $compressedInnerHtml is missing the ending comment separator!

<!-- h1{color:red}

@WyriHaximus
Copy link
Owner

Thanks for the updates, I'll add an edge case test for this and see if I can resolve it in this package or that this has to be fixed upstream

voku added a commit to voku/simple_html_dom that referenced this issue Aug 17, 2019
@WyriHaximus
Copy link
Owner

@kAlvaro #65 is up, tag should follow soon

@WyriHaximus
Copy link
Owner

2.0.1 has been tagged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants