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

Thumbnails not working in the media browser in PHP 7.4 #16468

Closed
DmitryFX opened this issue Sep 6, 2023 · 19 comments
Closed

Thumbnails not working in the media browser in PHP 7.4 #16468

DmitryFX opened this issue Sep 6, 2023 · 19 comments
Labels
bug The issue in the code or project, which should be addressed. needs-backport Issues that need to be backported to the previous maintenance version.
Milestone

Comments

@DmitryFX
Copy link

DmitryFX commented Sep 6, 2023

Bug report

Summary

Thumbnails not working in the media browser.
image

Step to reproduce

Upload any image (jpg, png) in the media browser, no thumbnails.

Observed behavior

Trying to navigate the url given in the 'not found' <img src='/site/connectors/system/phpthumb.php?src=assets%252Fvideo-iframe.png&w=100&h=0&HTTP_MODAUTH=modx64e51c266752a2.30117216_164f338950c1714.28557021&f=png&q=90&wctx=mgr&source=1&t=1693977472&ar=x'>
I get:

Fatal error: Uncaught TypeError: Argument 2 passed to phpthumb::__set() must be an instance of mixed, bool given in 

D:\site\site_www\core\vendor\james-heinrich\phpthumb\phpthumb.class.php:321 Stack trace: #0 

D:\site\site_www\core\vendor\james-heinrich\phpthumb\phpthumb.class.php(405): phpthumb->__set('cache_source_en...', false) #1 

D:\site\site_www\core\src\Revolution\modPhpThumb.php(100): phpthumb->setParameter('cache_source_en...', false) #2 

D:\site\site_www\core\src\Revolution\Processors\System\PhpThumb.php(140): MODX\Revolution\modPhpThumb->initialize() #3 

D:\site\site_www\core\src\Revolution\Processors\System\PhpThumb.php(84): MODX\Revolution\Processors\System\PhpThumb->loadPhpThumb() #4 

D:\site\site_www\core\src\Revolution\Processors\Processor.php(189): MODX\Revolution\Processors\System\PhpThumb->process() #5 

D:\site\site_www\core\src\Revolution\modX.php(1761): MOD in 

D:\site\site_www\core\vendor\james-heinrich\phpthumb\phpthumb.class.php on line 321

Error Log, set to Debug mode:

[2023-09-06 08:32:31] (DEBUG @ D:\site\site_www\core\src\Revolution\modLexicon.php : 526) Language string not found: "prop_file.skipExtensions_desc"
[2023-09-06 08:32:31] (DEBUG @ D:\site\site_www\core\src\Revolution\modLexicon.php : 526) Language string not found: "fred"
[2023-09-06 08:32:31] (DEBUG @ D:\site\site_www\core\src\Revolution\modLexicon.php : 526) Language string not found: "fredReadOnly"
[2023-09-06 08:32:31] (DEBUG @ D:\site\site_www\core\src\Revolution\modLexicon.php : 526) Language string not found: "prop_file.skipExtensions_desc"
[2023-09-06 08:32:31] (DEBUG @ D:\site\site_www\core\src\Revolution\modLexicon.php : 526) Language string not found: "fred"
[2023-09-06 08:32:31] (DEBUG @ D:\site\site_www\core\src\Revolution\modLexicon.php : 526) Language string not found: "fredReadOnly"
[2023-09-06 08:32:31] (DEBUG @ D:\site\site_www\core\src\Revolution\modLexicon.php : 526) Language string not found: "prop_file.skipExtensions_desc"
[2023-09-06 08:32:31] (DEBUG @ D:\site\site_www\core\src\Revolution\modLexicon.php : 526) Language string not found: "fred"
[2023-09-06 08:32:31] (DEBUG @ D:\site\site_www\core\src\Revolution\modLexicon.php : 526) Language string not found: "fredReadOnly"
[2023-09-06 08:32:31] (DEBUG @ D:\site\site_www\core\src\Revolution\modLexicon.php : 526) Language string not found: "prop_file.skipExtensions_desc"
[2023-09-06 08:32:31] (DEBUG @ D:\site\site_www\core\src\Revolution\modLexicon.php : 526) Language string not found: "fred"
[2023-09-06 08:32:31] (DEBUG @ D:\site\site_www\core\src\Revolution\modLexicon.php : 526) Language string not found: "fredReadOnly"
[2023-09-06 08:32:31] (WARN @ D:\site\site_www\core\vendor\james-heinrich\phpthumb\phpthumb.class.php : 314) PHP notice: Undefined variable: value
[2023-09-06 08:32:31] (WARN @ D:\site\site_www\core\vendor\james-heinrich\phpthumb\phpthumb.class.php : 314) PHP notice: Undefined variable: value

Expected behavior

Thumbnails should work.

Environment

3.1.0-dev
PHP 7.4.30
Apache 2.4
Mysql 5.7.17

ModX 2.8.4 works in the same environment as expected.

@DmitryFX DmitryFX added the bug The issue in the code or project, which should be addressed. label Sep 6, 2023
@halftrainedharry
Copy link
Contributor

It seems that there has been an update to the phpThumb library (that is used by MODX to create thumbnails) that is not compatible with PHP 7.4. But I believe this issue has to be fixed in phpThumb and not in MODX.

The problem is this pull request:
https://github.com/JamesHeinrich/phpThumb/pull/216/files

For a quick fix, you could either change the PHP version to at least 8.0, or delete the word mixed from the line here:
https://github.com/JamesHeinrich/phpThumb/blob/7ee966b38ddd7eb4d8091389aa514604710711c8/phpthumb.class.php#L317

@rthrash rthrash modified the milestones: v3.0.4, v2.8.6 Sep 6, 2023
@rthrash rthrash added the needs-backport Issues that need to be backported to the previous maintenance version. label Sep 6, 2023
@DmitryFX
Copy link
Author

DmitryFX commented Sep 6, 2023

halftrainedharry's solution worked. Many thanks!
SVG previews still not working, but it is another story.

@rthrash
Copy link
Member

rthrash commented Sep 6, 2023

SVGs maybe shouldn't try generating thumbnails and just be passed through w/o a thumbnail?

@DmitryFX
Copy link
Author

DmitryFX commented Sep 6, 2023

SVGs maybe shouldn't try generating thumbnails and just be passed through w/o a thumbnail?

Agree, but it looks, that phpthumb tries to make the preview and gives the following errors:

[2023-09-06 15:54:08] (DEBUG @ D:\site\site_www\core\src\Revolution\Sources\modMediaSource.php : 2376) assets/settings.svg has a mime type of: image/svg
[2023-09-06 15:54:08] (WARN @ D:\site\site_www\core\src\Revolution\Sources\modMediaSource.php : 2300) PHP notice: Undefined variable: image
[2023-09-06 15:54:08] (WARN @ D:\site\site_www\core\src\Revolution\Sources\modMediaSource.php : 2300) PHP notice: Undefined variable: image
[2023-09-06 15:54:08] (WARN @ D:\site\site_www\core\src\Revolution\Sources\modMediaSource.php : 1961) PHP notice: Trying to access array offset on value of type bool
[2023-09-06 15:54:08] (WARN @ D:\site\site_www\core\src\Revolution\Sources\modMediaSource.php : 1962) PHP notice: Trying to access array offset on value of type bool

Interesting:
Accessing .svg file directly via phpthumb connector gives the proper svg document in the browser.

http://localhost/site/connectors/system/phpthumb.php?src=assets%252Fsettings.svg&w=100&h=0&HTTP_MODAUTH=modx64e51c266752a2.30117216_164f338950c1714.28557021&f=png&q=90&wctx=mgr&source=1&t=1693977472&ar=x

But in the media browser svg preview looks as follows:

<img src="" data-src="" loading="lazy" width="100" height="80" alt="settings.svg" title="settings.svg">

@halftrainedharry
Copy link
Contributor

SVGs maybe shouldn't try generating thumbnails and just be passed through w/o a thumbnail?

This should already be implemented. There are exceptions in the code for SVGs:

protected function buildManagerImagePreview($path, $ext, $width, $height, $bases, $properties = [])
{
$size = $this->getImageDimensions($path, $ext);
if (is_array($size) && $size['width'] > 0 && $size['height'] > 0) {
if ($ext == 'svg') {
$size['src'] = $bases['urlAbsolute'] . ltrim($path, DIRECTORY_SEPARATOR);
return $size;
}

When I test it, it seems to work correctly (at least in MODX 3.0.3-pl).

@modxcommunity
Copy link
Collaborator

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

https://community.modx.com/t/modx-2-8-6-php-7-4-breaks-thumbnails-in-the-manager/7100/1

@opengeek opengeek modified the milestones: v3.0.4, v3.0.5 Oct 3, 2023
@opengeek opengeek changed the title Thumbnails not working in the media browser Thumbnails not working in the media browser in PHP 7.4 Oct 4, 2023
@rthrash
Copy link
Member

rthrash commented Oct 4, 2023

PR submitted to the upstream repo: JamesHeinrich/phpThumb#218

@modxcommunity
Copy link
Collaborator

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

https://community.modx.com/t/no-image-thumbnails-after-modx-3-upgrade/7353/2

opengeek added a commit that referenced this issue Feb 27, 2024
### 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
@Ibochkarev Ibochkarev reopened this Feb 28, 2024
@dimasites
Copy link
Contributor

dimasites commented Mar 26, 2024

Hi @opengeek sorry to bug you on this one but it'd be epic to get a release out with this fix.

People (like me and many other) stiil get updates websites and catches thumbnail bugs! Community chat warming with reports, manual codefixes coming to the wild...

But v2.8.7 milestone is only 58% complete, may be we can flush it faster?

@rthrash
Copy link
Member

rthrash commented Mar 26, 2024

Work is definitely happening @dimasites … some of the issues/PRs may not make the 2.8.7 release depending on testing and feedback, but the important phpThumb one definitely will.

@opengeek
Copy link
Member

Hi @opengeek sorry to bug you on this one but it'd be epic to get a release out with this fix.

People (like me and many other) stiil get updates websites and catches thumbnail bugs! Community chat warming with reports, manual codefixes coming to the wild...

But v2.8.7 milestone is only 58% complete, may be we can flush it faster?

We will be releasing it very soon. I would not pay much attention to the milestones—issues and PRs tend to get assigned to them with little regard for reality.

@dimasites
Copy link
Contributor

dimasites commented Mar 26, 2024

Thanks @opengeek and @rthrash for answers!

But for critical bugs please (it is only my point) lets make immidiate solutions! May be even hotfix-release with revert bad commit, because problems like this, make damage to friendliness and popularity of MODX.

I know about PHP 7.4 lifetime (yes, it is over), but we all knows: many and many MODX sites use PHP 7.4, much more then PHP8...

And I'll show you confirmation that it's critical.

See screenshots with stat, based on ~4.2k MODX Apps (components) install/upgrade/uninstall over last ~15 months (with only about 50% of Russian because its from MODX RSC data-source:
image

All additional screen for representation:
image

Based on this stats, we see over 95% MODX website's affected the bug, and even this stats not 100% accurate, even half of MODX users is overmuch

I think, we need (or must) to be more responsible with critical bug fixing! it is not security issue, but strog Developer UX issue...

P.S. I cant and dont want to make any pressure, but want stronly highlight problem!

Also i hope my examples and arguments were at least informative for you, thank you for your attention! And very thanks for working on such a wonderful project as MODX!

@denius-dev
Copy link

+1 for bump fixed version faster!

@wintik1
Copy link

wintik1 commented Mar 26, 2024

+2 for bump fixed version faster!
HELP!
I also faced this problem

@ShevArtV
Copy link

+1 for bump fixed version faster!

@Arahort
Copy link

Arahort commented Apr 9, 2024

Long time have this problem...

@Oksana-png
Copy link

Oksana-png commented Apr 9, 2024

+1 for bump fixed version faster!

1 similar comment
@biz87
Copy link

biz87 commented Apr 9, 2024

+1 for bump fixed version faster!

@opengeek opengeek modified the milestones: v3.0.5, v3.0.6 Apr 10, 2024
@smg6511
Copy link
Collaborator

smg6511 commented Apr 22, 2024

This issue is resolved with the vendor fix for phpthumb in both 2.8.7 and 3.0.5.

@smg6511 smg6511 closed this as completed Apr 22, 2024
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. needs-backport Issues that need to be backported to the previous maintenance version.
Projects
None yet
Development

No branches or pull requests