The root of this problem is sitting somewhere inside Dompdf library. However it is triggered by entity_print's default CSS styles and therefore a potential problem for every user of this module.
The CSS code added in #2744511: Fix max-width on img for entity-print.css is causing the problem described here: https://github.com/dompdf/dompdf/issues/1448#issuecomment-327211626
As soon as you work with max-width on images inside tables, you will get render problems. In that case you should:
I'd suggest keeping this issue open until there's a generic solution within Dompdf and/or point this problem out on the project page and readme.
In the Github thread, I just realized that I was using the wrong default value, when I tried to override the max-width for images inside tables. I haven't tried it yet, but adding this should/could fix the problem:
.page td img {
max-width: none;
}
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 2906839-11.patch | 237 bytes | benjy |
| #5 | fix_dompdf_tiny_images_in_tables_problem-2906839-5.patch | 247 bytes | agoradesign |
Comments
Comment #2
agoradesign commentedComment #3
benjy commentedDo you want to submit a patch with that CSS, I think we could commit it here and close the issue?
Comment #4
agoradesign commentedYes, that's what I was planning anyway :) It was already too late yesterday and I was out of office today until now. I'll have to test the line within Drupal, if it really works as expected first
Comment #5
agoradesign commentedThis works for me :)
Maybe we should document this somewhere anyway? + the tiny drawback of these new CSS lines is that images do not have a max-width any more within tables. At least for DomPdf users they never really worked anyway, but it could theoretically affect someone using images in tables on one of the other supported render engines (chances are small however)
Comment #7
benjy commentedWe can document this in the handbook if you want but I think this issue is enough. Committed and pushed. Thanks!
Comment #8
agoradesign commentedAgree... this should be solid default now + one can find this issue. should be enough documentation :)
Comment #9
benjy commentedI think maybe we should remove the .page looking at this again, not all themes will have that class?
Comment #10
agoradesign commentedIt wouldn't harm of course.
On the one side, having .page would be still ok because the CSS is just a default that doesn't have to be used, as same as the Twig template. As soon as you start customizing the Twig template, I'm pretty sure you begin to do the same with CSS and have a closer look what's in the defaults.
On the other side, it would make sense for any img and any td img scenario -> so let's remove the .page :)
Comment #11
benjy commentedCommitted the attached patch.
Comment #13
agoradesign commentedThis one was bad... you have to either drop ".page" off both selectors (".page img" and ".page td img") or none. This way the table override will never be used
Comment #14
agoradesign commentedI'll put it back to "needs work" then
Comment #15
agoradesign commenteda little bit off-topic, but it may interest you: I've added a comment to #2806113-28: No option to print header and footer in pdf describing what you have to take care of when adding header and/or footer to DomPdf via CSS positioning
Comment #17
benjy commentedahhh, right you are. .page is in the entity-print template, I didn't realise that the first time. I've reverted the last commit and kept your original. Thanks!
Yeah I saw your other comments, thanks.