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

Add LIBXML_NOBLANKS to make the output same on Linux and Windows #204

Closed

Conversation

mhujer
Copy link
Contributor

@mhujer mhujer commented Nov 14, 2020

We are using this library through Symfony to inline CSS in e-mails. We are also
using snapshot testing for rendered e-mails templates and I discovered that the
output of this library differs between Windows (dev machine) and Linux (CI).

I tried running the test suite on Windows and the test
CssToInlineStylesTest::testSpecificity() fails because of whitespace difference:
on Windows there is an extra \n before the closing </a> in the output.

I tried loading and saving a a super-simplified HTML <a>\n<img>\n</a>
through the DOMDocument without this library but with the same settings
and a similar thing happen (newline before is present on Windows but missing
on Linux).

When the option LIBXML_NOBLANKS is added to loadHTML() the output no longer
differs between the Windows and Linux.

To be honest, I'm not sure if this is a proper fix.
It should be rather considered a bug report with a workaround.

@mhujer
Copy link
Contributor Author

mhujer commented Nov 14, 2020

Travis php-nightly fails for unrelated reason: PHP Fatal error: Uncaught SebastianBergmann\CodeCoverage\RuntimeException: xdebug.coverage_enable=On has to be set in php.ini

@stof
Copy link
Collaborator

stof commented Nov 16, 2020

The error seems related to php nightly using Xdebug 3 on Travis.

@stof
Copy link
Collaborator

stof commented Dec 8, 2021

CssToInlineStylesTest::testSpecificity was failing on Linux for me as well (and on the CI), so I fixed the expectation to have a \n in it.

@mhujer mhujer force-pushed the mh-linux-windows-difference branch 3 times, most recently from 0dd083e to 891339f Compare January 27, 2022 12:17
@mhujer mhujer mentioned this pull request Jan 27, 2022
@mhujer mhujer force-pushed the mh-linux-windows-difference branch 2 times, most recently from 88fc1f1 to 8c66705 Compare January 27, 2022 16:19
@stof
Copy link
Collaborator

stof commented Jan 27, 2022

@mhujer I see that you also updated that PR. But please explain again what it is about. #224 is the proof that adding LIBXML_NOBLANKS is not necessary to make the testsuite pass on both Windows and Linux, because it is already the case without it.

@mhujer
Copy link
Contributor Author

mhujer commented Jan 27, 2022

@stof There is still something I need to investigate in our test suite, because the tests there pass on Windows and fail in CI on Linux, but both environments pass when I use my branch with the LIBXML_NOBLANKS. So maybe it is issue just with some more complex HTML.

I'd like to keep the PR open for now at least until I investigate what the issue is.

We are using this library through Symfony to inline CSS in e-mails. We are also
using snapshot testing for rendered e-mails templates and I discovered that the
output of this library differs between Windows (dev machine) and Linux (CI).

I tried running the test suite on Windows and the test
CssToInlineStylesTest::testSpecificity() fails because of whitespace difference:
on Windows there is an extra \n before the closing </a> in the output.

I tried loading and saving a a super-simplified HTML <a>\n<img>\n</a>
through the DOMDocument without this library but with the same settings
and a similar thing happen (newline before </a> is present on Windows but missing
on Linux).

When the option LIBXML_NOBLANKS is added to loadHTML() the output no longer
differs between the Windows and Linux.

To be honest, I'm not sure if this is a proper fix.
It should be rather considered a bug report with a workaround.
@mhujer mhujer force-pushed the mh-linux-windows-difference branch from 8c66705 to c2ffc5e Compare January 28, 2022 15:49
@mhujer
Copy link
Contributor Author

mhujer commented Mar 1, 2022

Surprisingly, the PHP behaviour changed between 8.0.5 and 8.0.14, so the output is same on Windows and Linux even without this patch.

@mhujer mhujer closed this Mar 1, 2022
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 this pull request may close these issues.

2 participants