I have installed the Media module and its dependencies on a fresh Drupal 7.0 install as documented in the media_dev profile—that is:

cvs -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib checkout -d styles -r DRUPAL-7--1 contributions/modules/styles
cvs -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib checkout -d media -r HEAD contributions/modules/media
cvs -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib checkout -d wysiwyg -r HEAD contributions/modules/wysiwyg
cvs -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib checkout -d entity -r DRUPAL-7--1 contributions/modules/entity

I get the PHP strict warning in the subject when I visit admin/content/media and select the thumbnail view. This does not seem to affect the correct behaviour of the module, however.

This is on a development environment with PHP 5.3.3 and error_reporting = E_ALL | E_STRICT.

CommentFileSizeAuthor
#6 media-drupal_render-fix-1016232-5.patch3.19 KBlifepillar
#1 media-admin-inc-1016232-1.patch700 byteslifepillar

Comments

lifepillar’s picture

lifepillar commented
Status: Active » Patch (to be ported)
StatusFileSize
new media-admin-inc-1016232-1.patch700 bytes

The attached patch fixes the problem.

JacobSingh’s picture

JacobSingh commented
Status: Patch (to be ported) » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, media-admin-inc-1016232-1.patch, failed testing.

james.elliott’s picture

james.elliott commented
Title: Only variables should be passed by reference in media_admin_thumbnails() » drupal_render() is incorrectly called with the result of a function

This is a correct fix but the issue is more widespread than this one instance.

lifepillar’s picture

lifepillar commented
Status: Needs work » Patch (to be ported)
StatusFileSize
new media-drupal_render-fix-1016232-5.patch3.19 KB

The attached patch against the latest HEAD should fix all the (four) instances of drupal_render() having a function as an argument.

effulgentsia’s picture

effulgentsia commented
Status: Patch (to be ported) » Fixed

Committed to CVS HEAD. Thanks!

@druido: I hope you keep contributing in the issue queue. When you do, please read up on issue status definitions. The appropriate status for #6 would have been "needs review". Now that this has been committed, if there was another branch (say a functioning D6 version), and you wanted the fix ported to that branch too, you could set the status to "patch (to be ported)", and the "version" to the branch you wanted it on. If you then supplied the ported patch, you would set the status to "needs review" again. I hope you take this as information only, and not as criticism. I really do appreciate that you found this bug, reported it, and provided the fix for it. Once again, thank you!

lifepillar’s picture

lifepillar commented

Thank you for pointing out that page. I'll keep it in mind for the next time!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.