Skip to content
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

Update phpThumb to v1.7.22-202312071641 #16506

Merged
merged 1 commit into from
Feb 27, 2024
Merged

Conversation

opengeek
Copy link
Member

What does it do?

Updates phpThumb to v1.7.22-202312071641

Why is it needed?

Fixes the issue with thumbnails not being generated when running on PHP 7.4 (or earlier).

How to test

Make sure thumbnails are being generated in the media browser when running on PHP 7.4.

Related issue(s)/PR(s)

Resolves #16468 for the 2.x branch

@opengeek opengeek added bug The issue in the code or project, which should be addressed. dependencies Pull requests that update a dependency file labels Dec 21, 2023
@opengeek opengeek added this to the v2.8.7 milestone Dec 21, 2023
@halftrainedharry
Copy link
Contributor

running on PHP 7.4 (or earlier).

If you want to support PHP versions < PHP 7.1, then there is still a problem with the void return type.

See this forum topic:
https://community.modx.com/t/phpthumb-issue-with-modx-2-8-6-and-php-5-6/7224

@opengeek
Copy link
Member Author

running on PHP 7.4 (or earlier).

If you want to support PHP versions < PHP 7.1, then there is still a problem with the void return type.

See this forum topic: https://community.modx.com/t/phpthumb-issue-with-modx-2-8-6-and-php-5-6/7224

We have no intention of maintaining support for a PHP version that ended both active and security support over five years ago. And I would never expect the phpThumb project to do so, either.

@rthrash rthrash added the urgent The issue requires attention and has higher priority over others. label Jan 24, 2024
@rthrash
Copy link
Member

rthrash commented Jan 24, 2024

I can confirm when manually applying the changes to the files in this PR that it definitely fixes the problem of missing thumbnails.

@rthrash rthrash added the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Jan 24, 2024
Copy link
Member

@theboxer theboxer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@opengeek opengeek merged commit f21a43a into modxcms:2.x Feb 27, 2024
5 checks passed
@opengeek opengeek deleted the phpthumb-1.7.22 branch February 27, 2024 20:52
@modxcommunity
Copy link
Collaborator

This pull request has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/problem-with-uncacher-when-upgrading-to-2-8-7/8035/5

@leoandrews
Copy link

leoandrews commented Sep 24, 2024

running on PHP 7.4 (or earlier).

If you want to support PHP versions < PHP 7.1, then there is still a problem with the void return type.
See this forum topic: https://community.modx.com/t/phpthumb-issue-with-modx-2-8-6-and-php-5-6/7224

We have no intention of maintaining support for a PHP version that ended both active and security support over five years ago. And I would never expect the phpThumb project to do so, either.

Not a unreasonable position to have, on paper. HOWEVER - your support docs for 2.8.x clearly state PHP5.6 is supported. So you probably DO need support it. Failing to do so, is breaking people's upgrades and eroding trust in the product.

@rthrash
Copy link
Member

rthrash commented Sep 25, 2024

The support docs need to be updated, then, and will be. Encouraging/facilitating people to deploy sites on unsupported ancient versions of PHP is not in anyone's best interest.

If someone needs old PHP, they can stick with 2.8.x until the next major version of MODX is released at which point 3.x will be the older supported version going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue in the code or project, which should be addressed. dependencies Pull requests that update a dependency file pr/ready-for-merging Pull request reviewed and tested and ready for merging. urgent The issue requires attention and has higher priority over others.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants