dev/core#2126 - Allow setting dompdf options, e.g. an external font folder

[demeritcowboy]

Overview

https://lab.civicrm.org/dev/core/-/issues/2126

For space reasons the stock fonts included with dompdf in civi only include Helvetica, Times, etc..., which have some non-english characters like ü, but not cyrillic or other characters like д. Even if it included the whole dompdf tree like it does in drupal 8, it doesn't include a good font with CJK characters.

Another reason you might want to set one of the dompdf options is that after dompdf did a recent security update local images don't work if they're outside the dompdf folder (see e.g. #20047). FYI @omarabuhussein

This PR adds the settings to the Miscellaneous admin settings form. The log file one which is unlikely to be used by regular users you can set in civicrm.settings.php or via an extension.

Before

While you can add fonts into vendor/dompdf/dompdf/lib/fonts, they get wiped on upgrade.

After

Can add fonts that survive upgrade. Can set some other settings.

Technical Details

The list of available settings is in Options.php. As per comments below we're only supporting a few.

I couldn't really write a test to test additional fonts, but it tests the general ability to set settings.

Comments

I can make a docs PR to update https://docs.civicrm.org/installation/en/latest/general/unicode_pdf

Steps to reproduce

1. Find Activities
2. Select one and choose Print/Merge document.
3. Put д or something like that in the body. Don't use ü or characters like that which happen to be in the standard Times font.
4. Click download.
5. The pdf will contain ? instead of the letters.

Quickie steps to test the PR

1. Make a folder like sites/default/files/fonts

2. Grab the files DejaVuSans.ttf and DejaVuSans.ufm from https://github.com/dompdf/dompdf/tree/master/lib/fonts and put them in sites/default/files/fonts. Don't worry about the bold or other variations, but if you really wanted to you could test those.

3. Make a file in there called dompdf_font_family_cache.php which contains:

   (Note $fontDir is a php variable - don't replace it with a path just leave it as $fontDir)

   <?php return array(
       'dejavu sans' => array(
         'bold' => $fontDir . '/DejaVuSans-Bold',
         'bold_italic' => $fontDir . '/DejaVuSans-BoldOblique',
         'italic' => $fontDir . '/DejaVuSans-Oblique',
         'normal' => $fontDir . '/DejaVuSans',
       ),
   );
   

4. At Administer - System Settings - Miscellaneous, set the full path to your font folder.

5. Now repeat the steps to reproduce above.

To install a full font from scratch involves another utility. I can reference it in the docs PR.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[seamuslee001]

@demeritcowboy are you planning on putting the metadata in as well for the setting? I like this idea myself I haven't r-run this as yet tho

[demeritcowboy]

Good idea. I wasn't sure what settings to give it - I've made it like other multi-select arrays, just there's no list of options.

[eileenmcnaughton]

@demeritcowboy I think I would go with just a setting for the specific option you want to override rather than an open ended array - we have had problems with the open ended arrays before & I think you really just want to override one key within it?

[omarabuhussein]

Thanks @demeritcowboy and sorry It seems I missed your comments on my other PR before, but yeah I like your solution better.

@eileenmcnaughton what kind of issues you had with open-ended arrays before ? do you remember?

[eileenmcnaughton]

@omarabuhussein so the 'contribute_invoice_settings' is an example where someone put in an array & we spent a long time later splitting it out into settings. The settings form is the most obvious thing - but in general the settings logic is all around one key (with metadata to describe it) per setting

[demeritcowboy]

Ok I can split it into a couple. I'd do at least one for the font, one for the chroot, one for remote_enable because it would be nice to turn that off for security reasons, and one for the log since otherwise I wouldn't have a unit test. I don't think the "default_XXX" ones are too useful because the page format setting pages in civi allow setting those anyway.

[demeritcowboy]

Ok I've updated. I debated whether to put them on a UI form - the log file setting would be developers-only so I left it off the form, but the other three it seemed like something you'd want to set in the UI. It just makes that miscellaneous page even more miscellaneous, but that's where wkhtmltopdf is already.

[eileenmcnaughton]

thanks @demeritcowboy - @omarabuhussein were you going to test this?

[omarabuhussein]

@eileenmcnaughton will do, yes. Will try to test it tomorrow.

[omarabuhussein]

I tested "DOMPDF Local Images Folder" and "DOMPDF Enable Remote Images" settings and both are working fine, but I could not manage to get the fonts working for some reason, it keeps showing up as "?".

[demeritcowboy]

Thanks for testing. I just tried again and it worked for me. A couple possibilities:

1. If you copy/pasted the text from somewhere like MS Word, ckeditor might pick up that font from where you copied it and so then it won't use the dejavu font. Click the Source button on ckeditor and if there's any fonts specified remove the style entry.
2. Similarly if you only downloaded the DejaVuSans.ttf and not the bold etc variations then make sure the style in ckeditor isn't bold or weird.
3. Use the full path to your custom font folder in the setting.
4. Double-check the font file you downloaded from github to make sure it's good. On windows you can double-click it to preview it. Probably safest to download the zip file of the https://github.com/dompdf/dompdf/archive/refs/heads/master.zip repo rather than trying to get individual files from the web interface.
5. What CMS are you using? It should work for any of them though.

[omarabuhussein]

@demeritcowboy

I quickly redid the test with all of what you mentioned in mind but with no luck, I am on Drupal 7 BTW, I will spend more time (hopefully before Friday) to see why it is not working for me, but I am almost certain that it is something with my configs and not your solution, given that your solution is merely sets the domepdf option, so for it not to work is most likely a config problem rather than a code problem.

[demeritcowboy]

Ok thanks. You can also PM me in chat.civicrm.org and we could see if we can solve it, or if you want to send me screenshots there I can take a look.

[eileenmcnaughton]

merging as I agree this is likely config

[demeritcowboy]

Thanks. I'll do a docs PR.

[demeritcowboy]

Docs PR: https://lab.civicrm.org/documentation/docs/installation/-/merge_requests/35