Breaking change in 7.1 about emogrifier HOT 10 CLOSED

Yes, we strive to follow Semantic Versioning.

This indeed is a gray area.

We didn't consider this as a breaking change as the functionality of the generated HTML is not affected by the formatting.

In general, I would recommend to write tests for HTML (and XML) to be formatting-independent in order to make them less brittle.

@JakeQZ What do you think?

from emogrifier.

And @mortenscheel, thanks for reporting this!

from emogrifier.

I notice tha the changelog entry says "Disable HTML formatting by default".

Although there are no methods or parameters in CssInliner to explicitly control the behaviour, there is a method to get the internal DOMDocument object, on which the formatOutput property can be set.

The following should achieve the previous behaviour, though I've not tested it (and setting the formatOutput property after loading the HTML might result in a different behaviour from when it is set before loading the HTML):

$cssInliner = CssInliner::fromHtml($html);
$cssInliner->getDomDocument()->formatOutput = true;
$visualHtml = $cssInliner->inlineCss($css)->render();

The old behaviour (in in which the rendered HTML is formatted) could be considered to be a bug, for example if list items are displayed as inline-block and do not have white space between them, but the formatting introduces some, or if some elements have the white-space property set (e.g. to pre), and extra whitespace is added by the formatting which then affects the layout, though I have not tested these scenarios.

from emogrifier.

In general, I would recommend to write tests for HTML (and XML) to be formatting-independent in order to make them less brittle.

Do you have any tips on how to do that? Currently I'm comparing the output html to a fixture, and I don't know how I can ignore only the redundant whitespace. Except maybe running both throw a DomDocument, but that seems like a hassle.

from emogrifier.

@mortenscheel The XML-specific assertions of PHPUnit might be helpful: https://docs.phpunit.de/en/10.4/assertions.html#xml

from emogrifier.

Or maybe https://github.com/caxy/php-htmldiff is worth a try.

from emogrifier.

The XML-specific assertions of PHPUnit might be helpful: https://docs.phpunit.de/en/10.4/assertions.html#xml

Looks like this internally uses DOMDocument, but it may only allow XML or XHTML if it uses the loadXML method and not the loadHTML method.

I was thinking you could just use regex to collapse all whitespace sequences to a single space:

$normalizedHtml = \preg_replace('/\\s++/', ' ', $html);

This would have the drawback of replacing all line breaks with regular spaces, making the differences diffucult to see. That could be mitigated against by changing spaces after tags to line breaks:

$normalizedHtml = \str_replace('> ', ">\n", \preg_replace('/\\s++/', ' ', $html));

Though differences where whitespace is important, such as preformatted text, wouldn't generate test failures, so you'd have to check those cases differently.

from emogrifier.

Please note that HTML cannot be fully parsed with regular expressions. 😉

from emogrifier.

Please note that HTML cannot be fully parsed with regular expressions. 😉

Indeed, though the above is not parsing it, just removing extra whitespace.

from emogrifier.

Converting to a discussion as this has turned from a potential bug report to a discussion. :-)

from emogrifier.