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

[2.3] phpthumb image thumbnails in media browser #11700

Closed
absent42 opened this issue Jul 16, 2014 · 14 comments
Closed

[2.3] phpthumb image thumbnails in media browser #11700

absent42 opened this issue Jul 16, 2014 · 14 comments
Assignees
Labels
area-core bug The issue in the code or project, which should be addressed. urgent The issue requires attention and has higher priority over others.
Milestone

Comments

@absent42
Copy link

With the new 2.3 release I'm getting image thumbnails failing to render in the media/file browser. This didn't happen with a dev release install on the same server.

Error in the log is:
2014-07-16 00:58:24 (ERROR @ /manager/index.php) [OnManagerPageBeforeRender]1

@pixelchutes
Copy link
Contributor

This appears to be a harmless (but annoying) entry in the error log. I've fixed plugins exhibiting this same behavior in 2.3 by simply adding a return; at the end.

Tthat said, not sure what's going on that the thumbs would no longer be rendering. Have you tried clearing cache / error log?

@absent42
Copy link
Author

Yes, manually.

Extracting the thumbnail url (connectors/system/phpthumb.php?src=images etc) and going to it manually produces the error:

Fatal error: Cannot redeclare phpthumb::__destruct() in /home/danielg5/public_html/core/model/phpthumb/phpthumb.class.php on line 254

@Mark-H
Copy link
Collaborator

Mark-H commented Jul 16, 2014

I think I saw a destruct method in the master-2.3 merge but didn't think
much of it. Merge error after all?
Op 16 jul. 2014 02:38 schreef "Daniel Gibson" notifications@github.com:

Yes, manually.

Extracting the thumbnail url and going to it manually produces the error:

Fatal error: Cannot redeclare phpthumb::__destruct() in
/home/danielg5/public_html/core/model/phpthumb/phpthumb.class.php on line
254


Reply to this email directly or view it on GitHub
#11700 (comment)
.

@absent42 absent42 changed the title phpthumb image thumbnails in media browser [2.3] phpthumb image thumbnails in media browser Jul 16, 2014
@pixelchutes
Copy link
Contributor

I can confirm this issue.

I verified on a 2.2.14 upgrade to modx-2.3.0-pl-advanced (via .zip, not git)

UPDATE:
Yep, I can confirm there are multiple, duplicate function declarations in phpthumb.class.php

https://github.com/modxcms/revolution/blob/master/core/model/phpthumb/phpthumb.class.php#L238-L268

@pixelchutes
Copy link
Contributor

As a temporary solution, I replaced core/model/phpthumb/phpthumb.class.php
with the contents of: https://raw.githubusercontent.com/modxcms/revolution/develop/core/model/phpthumb/phpthumb.class.php

...and that resolved the thumbnail issue. To your point Mark, there may be other merge issues to look into.

@GarretOverstreet
Copy link

I was getting a similar error when upgrading to 2.3. I found that there were a couple functions that were duplicated. Look for the following and you'll see both twice (in a backwards order.) After removing the duplicates below, it seems to work fine now.

// public:
function purgeTempFiles() {
foreach ($this->tempFilesToDelete as $tempFileToDelete) {
if (file_exists($tempFileToDelete)) {
$this->DebugMessage('Deleting temp file "'.$tempFileToDelete.'"', FILE, LINE);
@Unlink($tempFileToDelete);
}
}
$this->tempFilesToDelete = array();
return true;
}

function __destruct() {
    $this->purgeTempFiles();
}

@Mark-H
Copy link
Collaborator

Mark-H commented Jul 16, 2014

I'd send a PR for this, but it seems the problem is only in the master/master-2.3 branch. Definitely result of the merge. One instance was cherry picked into 2.2, causing the merge issue. Commit d2d7e9d

@jesseshowalter
Copy link

This was causing an issue with the "Clear Cache" function in the top menu. as soon as I removed duplicate code, the media manager and cache issued were repaired. If anyone if having a clear cache issue this could be you too.

@NunoBentes
Copy link

Yep, I can also confirm there are multiple, duplicate function declarations in phpthumb.class.php
I just replaced this file by the one that exist on the Modx Rev 2.2.14. This solved the problem

@sebastian-marinescu
Copy link
Contributor

No PR ready yet? Should I provide one by tomorrow?

@absent42
Copy link
Author

@sebastian-marinescu PR here: #11712

@opengeek opengeek added this to the v2.3.1-pl milestone Jul 19, 2014
@opengeek opengeek self-assigned this Jul 19, 2014
opengeek added a commit that referenced this issue Jul 19, 2014
Merge remote-tracking branch 'origin/pr/11712'

* origin/pr/11712:
  Get rid of weird character in phpthumb class
  Fix issue with duplicate method declarations in phpthumb.class.php
@jacobkball
Copy link

I can confirm that deleting the duplicate functions cleans things up nicely. I upgraded from 2.2.9 and everything looked terrible (manager completely borked, galleries not working)! Removed the duplicate functions, and voila, all good again. I didn't do what Legues did (replace with 2.2.14 version - a file comparison shows more changes between these two versions) - I just edited the file that was there.

@NunoBentes
Copy link

Probably could be better just delete lines 254 to 268. I´ve updated my comment on MODX forum.

@opengeek
Copy link
Member

This should be resolved now in master and develop branches. Re-open if this cannot be confirmed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core bug The issue in the code or project, which should be addressed. urgent The issue requires attention and has higher priority over others.
Projects
None yet
Development

No branches or pull requests

9 participants