Open rpatterson opened 10 years ago
I should also note that the tests still failed when I downgraded only libxslt to 1.1.26 but left libxml2 at 2.9.
I think this issue has been fixed with https://github.com/plone/diazo/pull/29. Please re-open the issue if necessary.
@tisto The tests pass after my changes, but it seems like a false pass. When I inspect the Diazo output in those tests I still see the whitespace stripped which doesn't seem like what we want since it would make the Diazo output unreadable for developers or themers trying to debug or inspect their HTML.
Example pdb session in the tests:
testAll (diazo.tests.tests.Test-v1-drop-all-attributes-theme-conditional-5)> /opt/src/diazo/lib/diazo/tests/tests.py(162)testAll()
-> if not xml_compare(etree.fromstring(old.strip()), etree.fromstring(new.strip())):
(Pdb) print new
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<body><h1>Title</h1><div id="alpha" dir="rtl">a</div><div id="beta">b</div></body>
</html>
(Pdb) print old
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<body>
<h1>Title</h1>
<div id="alpha" dir="rtl">a</div>
<div id="beta">b</div>
</body>
</html>
Yeah, I am aware of that problem. The xml_compare function operates on trees, so it completely ignores any whitespace between the nodes. If you want to make it work for your pdb example you would have to deal with whitespace again (which was our problem in the first place). We could run a pretty print on both HTML fragments before comparing them in xml_compare. This way pdb would print out something that is easier to compare for developers.
Pretty printing for debugging diazo itself is a nice to have but not a show stopper, but whitespace can effect browser rendering. It looks like https://github.com/plone/diazo/tree/master/lib/diazo/tests/indent-off is the only test with pretty-print = false
. It may need special attention here to ensure the test is still testing the important behaviour.
I'm not quite sure what you're both saying here. Are you saying the whitespace stripping in the test doesn't happen when Diazo is processing HTML in real deployments under these versions of libxml2/libxslt? I'm saying that if it does strip white space in real deployments under these versions, then this is a bug somewhere and a ticket should be open somewhere.
On Fri, Jun 20, 2014 at 1:53 AM, Laurence Rowe notifications@github.com wrote:
Pretty printing for debugging diazo itself is a nice to have but not a show stopper, but whitespace can effect browser rendering. It looks like https://github.com/plone/diazo/tree/master/lib/diazo/tests/indent-off is the only test with pretty-print = false. It may need special attention here to ensure the test is still testing the important behaviour.
— Reply to this email directly or view it on GitHub https://github.com/plone/diazo/issues/27#issuecomment-46649853.
As far as I can tell none of the above tests cases include checking whitespace remains between elements (as noted in #25) should this also be part of these test or am I barking up the wrong tree here?
I've just noticed that the comment previous to my last one was over a year old. This may no longer be an issue.
The Plone coredev tests fail because newlines are being stripped on my laptop running Ubuntu Saucy and @davisagli's laptop as well but pass on Plone's Jenkins server. I downgraded my version of libxml2 to 2.8 and my version of libxslt to 1.1.26, the versions used on Plone's Jenkins server and in Ubuntu Quantal and confirmed that the tests then pass locally on my laptop.
Here are the failures when running under Ubuntu's Saucy libxml2 version 2.9 and libxslt version 1.1.28: