-
-
Notifications
You must be signed in to change notification settings - Fork 852
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
Bump weasyprint #5885
Bump weasyprint #5885
Conversation
✅ Deploy Preview for inventree-web-pui-preview canceled.
|
This issue was blocking using an upgraded Either way: if Weasyprint can be successfully upgraded, #4936 can be closed. |
You are right @miggland, we will have to push this |
The fix in django-weasyprint has actually been merged, but the release was not pushed to GitHub: |
Well spotted @SchrodingersGat - but as of 13 hours ago, that's been corrected as well it seems! |
@miggland yes I reached out to the maintainer of the project :) |
It appears that the rendered PDF does not contain the exact same information. If you are able to test that the generated PDF files are still good, we can adjust the single failing test |
I have tried out (again) with the dev branch: SVGs don't show up. So: this seems to fix #4936, and looks good otherwise. The test: I guess a more robust test would be to try and open the PDF with some tool (eg PikePDF), eg: Alternative: pdftotext:
But that will only work if there is actually text in the PDF.. Of course, both require installation of extra packages.. |
I am not sure adding a dependency is great but I also can not find a better solution, pretty much on the same page as @miggland. I did not work with pikepdf - which is smaller than what I tried. I will adapt this PR to add a test based on it. |
Co-authored-by: miggland <miggland@users.noreply.github.com>
@miggland I added a co-author credit for you as the change is basically what you suggested above |
The order of objects seems flaky between DBMS, a fix will follow shortly. |
@SchrodingersGat this would be ready for review |
Thanks @matmair |
This removes the pin as the issue that caused the pin seems to be resolved in django-waesyprint.
Might also close #4936, pending confirmation by @miggland
Closed #4936