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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jcnventura’s picture

Status: Active » Needs review
jcnventura’s picture

Status: Needs review » Fixed

Thanks for the patch.. I've fixed in dev.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

thebuckst0p’s picture

Title: correction to dompdf font cache warning in status report » correction to dompdf (and tcpdf) font cache warning in status report
Category: feature » bug
Status: Closed (fixed) » Active

Re-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.

thebuckst0p’s picture

Version: 6.x-1.14 » 6.x-1.15
Status: Active » Needs review
FileSize
2.26 KB

Patch 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.

pirog’s picture

hey 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

jcnventura’s picture

Status: Needs review » Closed (won't fix)

I'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.

brad.bulger’s picture

since #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.

pirog’s picture

i 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