Skip to content

Fixes a problem with php 7.4 #76

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 27, 2020
Merged

Fixes a problem with php 7.4 #76

merged 1 commit into from
Jan 27, 2020

Conversation

LinkingYou
Copy link
Contributor

This would fix the issue #75

This would fix the issue dompdf#75
@carlospauluk
Copy link

Please, merge!

@eduardoturconi
Copy link

Why is it taking so long to merge?

@MatthiasKuehneEllerhold

Please merge and release... !

This was referenced Jan 16, 2020
@poldixd
Copy link

poldixd commented Jan 21, 2020

Hm, the last commit of this package is almost two years ago :-(

@marijnbent
Copy link

I think someone needs to fork this

@carlospauluk
Copy link

I think someone needs to fork this

Totally agree!

@carlospauluk
Copy link

Please see: dompdf/dompdf#2063

@andrefedalto
Copy link

andrefedalto commented Jan 24, 2020

I'm using dompdf through laravel and having the same issue. One workaround I could find is:

  1. Apply the fix by @LinkingYou locally on vendor\phenx\php-font-lib\src\FontLib\AdobeFontMetrics.php:142;
  2. Set your font folder for dompdf to a folder in your repository;
  3. Run your script to build the pdf locally, this will make dompdf generate all font temp files (ttf, ufm, php) in your font folder;
  4. Commit these files to your repository;
  5. Revert the local changes in AdobeFontMetrics.php;
  6. 🎉 dompdf will now use the parsed fonts instead of parsing them again. Remember you have to repeat all the steps here for each new font you use

@jeliasson
Copy link

@PhenX Can you kindly release master to 0.5.2? This will allow us to have it fixed in e.g. dompdf/dompdf.

@enumag
Copy link
Contributor

enumag commented Feb 18, 2020

@jeliasson There are more things wrong with 7.4, see #77. Also it would be good to have 7.4 compatibility verified on travis, see #74. In my opinion it makes no sense to make a release until these are merged.

@jeliasson
Copy link

@enumag It would surely be nice to have all 7.4 issues fixed in the same release, but considering there are downstream packages that seems to benefit from this merge alone I think it would not hurt to throw in a minor release. Anyway, let's hope for a few merges and a release bump soon.

@sandervanhooft
Copy link

+1 for a hot fix (minor release); many integrations are (partially) broken by this issue.

That being said, I don't have the full picture of what's involved (or anyone's time schedule).

@bastihilger
Copy link

What @sandervanhooft said. I also would like to kindly ask for a commit just addressing this one issue, if in any way possible.

@jeliasson
Copy link

jeliasson commented Mar 4, 2020

Following up on what @LinkingYou suggested, here is a donkey-patch based off @andrefedalto's contribution (31cad91).

# Donkey patch vendor/phenx/php-font-lib/src/FontLib/AdobeFontMetrics.php
cat vendor/phenx/php-font-lib/src/FontLib/AdobeFontMetrics.php | sed "s/\$tree\W\=\W\$kern\[\"tree\"\]\;/\$tree \= is_array\(\$kern\) \? \$kern\[\"tree\"\]\ : null\;/g" > ./donkey-patch.php && mv ./donkey-patch.php vendor/phenx/php-font-lib/src/FontLib/AdobeFontMetrics.php

# Verify output of donkey patch
cat vendor/phenx/php-font-lib/src/FontLib/AdobeFontMetrics.php | grep '$tree = '

Hope it helps someone!

@dj3plles
Copy link

dj3plles commented Mar 7, 2020

This helps @jeliasson

@ghost
Copy link

ghost commented Mar 10, 2020

@jeliasson Thanks a lot! This really helped

@ngunyimacharia
Copy link

Really hope this is permanently closed soon.

@PhenX
Copy link
Member

PhenX commented Jul 25, 2020

Hello, 0.5.2 already has this fix, nothing was pushed to master since its release. What can I do more ?

@ngunyimacharia
Copy link

Had not noticed, left this thread before it was pushed. Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.