Skip to content

Fixed regression from fixing PHP 8.2 deprecated usages earlier, which… #2

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

Conversation

hostep
Copy link
Contributor

@hostep hostep commented Aug 23, 2023

… broke rendering certain elements in a pdf in Magento

This fixes a regression bug that was created in #1
The bug is described here: magento/magento2#37897

I can not really explain how it triggers, it's very weird and unexpected. But reverting the $value member back to each individual child class and removing it again from the parent Zend_Pdf_Element class fixes the problem.

In order to fix the PHP 8.2 deprecated usage of this->value in Zend_Pdf_Element, I turned the toPhp function into an abstract function which forces the classes that inherit from it to implement it, so I did that. It's a bit of duplicated code each time, but can't find a better way to solve it. At least the return types in docblocks will be more correct now for the toPhp function ...

(the double new lines in between methods is to stay consistent with the coding style of the original code)

… broke rendering certain elements in a pdf in Magento
@hostep
Copy link
Contributor Author

hostep commented Aug 23, 2023

See https://github.com/magento/magento-zend-pdf/compare/1.16.2..6047a4a91f273513364c5a384f0ce8c018088265 for the diff between the original code and both #1 and #2 combined.

@magento-devops-reposync-svc magento-devops-reposync-svc merged commit 4426cdf into magento:main Aug 25, 2023
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.

3 participants