integreat-cms
integreat-cms copied to clipboard
Update xhtml2pdf
Short description
Finally, xhtml2pdf released a new version including @charludo's upstream contribution :tada:
I did not add a new test case for the specific fixed issue, maybe this would be a good idea to make sure it isn't re-introduced in future versions.
Proposed changes
- Update xhtml2pdf
- Update test files
Side effects
- Unfortunately, page number seem to have stopped working (see https://github.com/xhtml2pdf/xhtml2pdf/issues/670)
- Apparently, it re-introduces the line reversing bug (see https://github.com/xhtml2pdf/xhtml2pdf/issues/538)
Resolved issues
Fixes: #1498 Fixes: #2022
Also, it removes the vulnerable dependency future
(see https://github.com/digitalfabrik/integreat-cms/security/dependabot/20)
Code Climate has analyzed commit 988cd164 and detected 0 issues on this pull request.
The test coverage on the diff in this pull request is 100.0% (50% is the threshold).
This pull request will bring the total coverage in the repository to 75.3% (0.0% change).
View more on Code Climate.
@seluianova If I understood correctly, this update also resolves #2022 / https://github.com/xhtml2pdf/xhtml2pdf/issues/662, right? Could you check whether that's true?
E.g. before: https://raw.githubusercontent.com/digitalfabrik/integreat-cms/develop/tests/pdf/files/3b02f5ea5b/Integreat%20-%20Arabisch%20-%20%D9%85%D8%B9%D9%84%D9%88%D9%85%D8%A7%D8%AA%20%D8%A7%D9%84%D9%88%D8%B5%D9%88%D9%84.pdf After: https://raw.githubusercontent.com/digitalfabrik/integreat-cms/update/xhtml2pdf/tests/pdf/files/3b02f5ea5b/Integreat%20-%20Arabisch%20-%20%D9%85%D8%B9%D9%84%D9%88%D9%85%D8%A7%D8%AA%20%D8%A7%D9%84%D9%88%D8%B5%D9%88%D9%84.pdf
But it makes https://github.com/xhtml2pdf/xhtml2pdf/issues/538 more apparent? Still struggling to figure out the relations between all those issues :sweat_smile:
@timoludwig
If I understood correctly, this update also resolves https://github.com/digitalfabrik/integreat-cms/issues/2022 / https://github.com/xhtml2pdf/xhtml2pdf/issues/662, right? Could you check whether that's true?
Well, kind of. But #2022 was a minor cosmetic issue that didn't affect readability much.
I believe https://github.com/xhtml2pdf/xhtml2pdf/issues/538 was a bigger issue, and now it is broken again. The lines are reversed, it affects the meaning.
@timobrembeck Hey Timo, do you think it might make sense to keep the changes in test_pdf_export.py and revert the others maybe, in order to close this PR? I guess we wouldn't want to upgrade xhtml2pdf to the current version anyway.
@seluianova The changes in the tests only make sense when the xhtml2pdf version is upgraded (since the newer version depends on pypdf
instead of PyPDF3
).
But yes, sadly it seems as I don't have time to contribute to xhtml2pdf in the near future :cry:
I'm going to close this PR but keep the branch, so in case we upgrade eventually, I can reuse the changes.