Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
this is a followup to #771300: dompdf font cache - it's really the font cache directory that needs to be writable, not necessarily the font directory. it's just that the default is for those to be the same. but if DOMPDF_FONT_CACHE is already defined to some other location, dompdf uses that definition. it would be nice if the status report check could take this into account by checking that constant.
Comment | File | Size | Author |
---|---|---|---|
#5 | print-fix-tcpdf-warning-1394364-4.patch | 2.26 KB | thebuckst0p |
print.dompdf.status.patch | 813 bytes | brad.bulger | |
Comments
Comment #1
jcnventura CreditAttribution: jcnventura commentedComment #2
jcnventura CreditAttribution: jcnventura commentedThanks for the patch.. I've fixed in dev.
Comment #4
thebuckst0p CreditAttribution: thebuckst0p commentedRe-opening this: the same applies to the tcpdf warnings. If the configuration is overridden with K_TCPDF_EXTERNAL_CONFIG (as I'm doing), the status report checks the wrong directory. I'll have a patch shortly.
Comment #5
thebuckst0p CreditAttribution: thebuckst0p commentedPatch attached.
It also fixes a typo, the warning said 'dompdf' but should have said 'tcpdf'.
Tested on our dev site and seems to be working.
Comment #6
pirog CreditAttribution: pirog commentedhey guys,
im working on a similar issue over at #1520416: DOMPDF_FONT_CACHE not defined in print_pdf_requirements() and it might be wise for us to combine our efforts. it also looks like this issue might apply to TCPDF although im only looking at DOMPDF in my issue as of now.
the problem is that during the requirements check none of the constants are defined yet so even if you edit those constants in the respective config files for DOMPDF or TCPDF to point to new directories the requirement hook is still checking the default directories which are /lib/fonts in the case of DOMPDF and cache/images in your patch.
in my patch (see #1520416: DOMPDF_FONT_CACHE not defined in print_pdf_requirements()) i've added a setting to allow the user to configure the font cache directory in DOMPDF and then this is the variable checked in the requirement hook and not the constant. in my personal opinion i would prefer setting these things in the respective config files instead of through the UI but i think in order for us to do it that way we would have to refactor a decent amount of code.
i also posted this issue for the same typo earlier
#1520556: description references dompdf but requirement is for tcpdf
Comment #7
jcnventura CreditAttribution: jcnventura commentedI'll correct the typo, but the module will stop checking for paths when auto-config is off.. As such I'm reverting the original patch here.
For more details, check #1520416: DOMPDF_FONT_CACHE not defined in print_pdf_requirements() and #1538026: PDF library temporary directories should use trello drupalsites/*/files when auto-config is on.
Comment #8
brad.bulger CreditAttribution: brad.bulger commentedsince #1538026: PDF library temporary directories should use trello drupalsites/*/files when auto-config is on would do what we were trying to do in the first place by defining that constant, that sounds fine to me.
Comment #9
pirog CreditAttribution: pirog commentedi totally agree with this approach. you are right that it is too much to add in more settings to handle a custom directory. just have that directory configured to be in sites/*/files with autoconfig and checked by the requirements hook for existence and writability. if you dont want to use autoconfig you are on your own.
ill write a patch and post to #1538026: PDF library temporary directories should use trello drupalsites/*/files when auto-config is on