Relative images are not supported on DomPDF v0.8.0+

Thanks Eduardo,

patch in #3 works for me! ;-)

I can confirm that this issue is working properly. It would be great if this could be included in a release.

Thanks, I think the approach makes sense. I like the base64 encoding more than allowing remote files.

1. +++ b/modules/commerce_billy_pdf/commerce_billy_pdf.module
   @@ -233,11 +233,25 @@ function commerce_billy_pdf_commerce_order_view($order, $view_mode) {
   + * @return string
   

   There should be an empty line after the @param section

2. +++ b/modules/commerce_billy_pdf/commerce_billy_pdf.module
   @@ -233,11 +233,25 @@ function commerce_billy_pdf_commerce_order_view($order, $view_mode) {
   +  $imageData = base64_encode(file_get_contents($image_path));
   

   we should first check if the file exists and only then do the base64 encoding. We should write a watchdog error if the file does not exist.

   We also don't want to support remote files for security reasons, so the file_exists() check is correct.

Can you update the issue summary and fix the minor things I found?

I have created a revised patch for review and an interdiff. I will also update the issue summary.

Per this patch I wrote in 2019 (https://www.drupal.org/project/commerce_billy/issues/3081653), there is also a critical 0.8.0+ issue with the invoice logo that does not allow it to work when the local file system has been replaced with S3, as is the case for our sites.

Given it is clear from this thread that the wish is to avoid the use of isRemoteEnabled, the patch in #3 has been modified to include @klausi's recommendations, and to support the use of full paths that reconcile with available Drupal file system schemes. Therefore, users can enter relative paths, and paths that are prefixed by https://, public://, s3://, etc., as long as the path reconciles to a Drupal scheme.

Please review and let me know your feedback, thank you.

Thanks Ron, very sorry to bump this back again.

I think we should not mix 2 issues here. The S3 and other file system support should be a separate issue. Let's fix the simple base64 approach here first to get that out of the way.

So we should have a relatively short patch hear based on Eduardo's patch above.

@klausi, that's fine. Was trying to incorporate both since I'm always concerned when will be the next time someone might look at these old patches and make a commit.

I'll re-roll the patch, and open the other issue with a separate patch for S3.

See attached patch and interdiff, thanks.

Thanks!

+++ b/modules/commerce_billy_pdf/commerce_billy_pdf.module
@@ -226,11 +226,35 @@ function commerce_billy_pdf_commerce_order_view($order, $view_mode) {
+  watchdog('commerce_billy_pdf', 'Invoice logo does not exist or cannot be found: <em>%path</em>',

this will create double em tags, the %placeholder already adds them.

Fixed that and committed it!