fix #1330 add intval() to improve PHP 7.1 compatibility #1331

Merged
merged 1 commit into from Jan 8, 2017

Projects

None yet

4 participants

@dsampaolo

No description provided.

@bsweeney bsweeney self-requested a review Dec 16, 2016
@bsweeney bsweeney added this to the 0.7.1 milestone Dec 16, 2016
@bsweeney
bsweeney requested changes Dec 24, 2016 edited View changes

I think we should go ahead and cast to float rather than int. The affect is small (maybe even unnoticeable), but accuracy is important.

Also, where possible let's use type casting ((float)$var) rather than a function call (floatval($var)).

@bsweeney
Member
bsweeney commented Dec 24, 2016 edited

Also, any opinion on casting to float instead of int (as in #1339)?

@bsweeney bsweeney dismissed their review Jan 4, 2017

Didn't mean to approve just yet.

@bsweeney bsweeney self-requested a review Jan 4, 2017
@bsweeney

I think we should go ahead and cast to float rather than int. The affect is small (maybe even unnoticeable), but accuracy is important.

Also, where possible let's use type casting ((float)$var) rather than a function call (floatval($var)).

@steffenweber
steffenweber commented Jan 4, 2017 edited

In the method Style::length_in_pt, there are more (float) casts required. For some reason, its code for handling the units pt, %, rem, em, in and pc uses a (float) cast but the code for the units px, cm, mm and ex does not (example).

Adding the (float) casts fixes for example the following error:

A non well formed numeric value encountered in vendor/dompdf/dompdf/src/Css/Style.php:515

In my case the root cause for this error was the HTML code <img src="..." width="177" height="84" alt="..."> whose height attribute for some reason arrived in Style::length_in_pt as 84 px (notice the whitespace between the number and the unit).

@bsweeney
Member
bsweeney commented Jan 4, 2017

@steffenweber could be an issue in the attribute translator. I'll be sure to take a look at that in conjunction with this update.

@steffenweber
steffenweber commented Jan 4, 2017 edited

Yeah, removing the whitespace before px in line 36 and in line 39 of AttributeSelector.php solves this issue. (It seems odd that it exists in the first place but I'm not sure whether removing it could maybe break something else.)

@bsweeney
Member
bsweeney commented Jan 5, 2017

I imagine it was added to avoid issues with the sprintf format string. I don't think the change will break anything, but I'll look into it a bit more.

@bsweeney bsweeney merged commit 5eb0558 into dompdf:develop Jan 8, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bsweeney bsweeney referenced this pull request Jan 9, 2017
@bsweeney bsweeney Clean up format string spacing in AttributeTranslator
Resolves an issue brought up in the comments of #1330
9a42f95
@v-wolf v-wolf referenced this pull request in niklasravnsborg/laravel-pdf Jan 14, 2017
Closed

PHP 7.1 A non-numeric value encountered #21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment