1. CCK version:
6.x-3.x-dev

2. imagefield version
6.x-3.8

3. permissions for your tmp and files directory.
777 (I know its wrong, but hey! its localhost).

4. download method (public or private files)
FILE_DOWNLOADS_PUBLIC

5. imagefield configuration
To upload in a directory like: solutions/icons.

6. expected results
/sites/default/files/imagefield_thumb/solutions/icons/image-name.jpg

7. the unexpected actual results
/solutions/icons/image-name.jpg

8. Steps to reproduce the problem.

After an update from 6.x-3.7 to 6.x-3.8, I realize that my images where not showing !!
So, looking what a heck gad change on the last update, I sow a process of rawurlencode() thing..., but before that, the "sites/default/files" was removed, and never put it back in again.

Resuming, if you make an update lately and your images where all gone, check if the src="... of the images is missing the "/sites/default/files" before the path of your image.

I made a simple patch, but I think somebody need to review, etc...

Hugs! and thanxs for these awesome module!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sebas5384’s picture

FileSize
555 bytes

IGNORE THE OTHER PATCH !!!!

This is the right one :D

Sorry about that ¬¬

quicksketch’s picture

I'm unable to confirm this. My images still work perfectly fine in the latest code. I find it strange the you'd need to add file_directory_path() to the path, when it just gets stripped out again immediately in file_create_url(). How could this change make any difference?

sebas5384’s picture

I try to debug again to see a little more, and i realize this in the file_create_url() function:

...
// Path of the image
$path = 'solutions/icons/name_of_image.jpg';

// Shipped files.
  if (strpos($path, file_directory_path() . '/') !== 0) {
    return base_path() . $path;              ///==> RETURN HERE WITH: '/solutions/icons/name_of_image.jpg' (that's wrong ¬¬)
  }
  // Created files.
  else {
    switch (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC)) {
      case FILE_DOWNLOADS_PUBLIC:            ///==> SUPPOSED TO RETURN HERE, RIGHT?
        return $GLOBALS['base_url'] . '/' . $path;
...

So passing the full path to $path = '/sites/default/files/solutions/icons/name_of_image.jpg'
get this thing work, but, i don't know if it just me or what.... maybe its the PHP 5.3 ....lately everything its because of that, haha!

Can you make a debug with a and see if the $path it's different than me?

Soren Jones’s picture

http://drupal.org/cvs?commit=461444 breaks thumbnails for newly uploaded images in node/add and node/edit forms with download method set to public files.

Commenting out:
// Strip off the file directory path if present.
if (strpos($thumb_path, file_directory_path() .'/') === 0) {
$thumb_path = trim(substr($thumb_path,
strlen(file_directory_path())), '\\/');
}

Resolves this issue, but doesn't help #931540: Images have wrong path when using private files and absolute Windows file directory.

EDIT: Or not. I can't reproduce this on a clean install.

Soren Jones’s picture

OK. I can duplicate the issue with http://drupal.org/cvs?commit=461444 and theme_imagefield_admin_thumbnail() in a fresh install of Pressflow.

Soren Jones’s picture

Priority: Major » Normal
Status: Needs review » Active
Issue tags: +Pressflow

And the issue is with Imagefield and Pressflow's file_create_url(). Pressflow returns a path of $GLOBALS['base_url'] . '/' . $path for FILE_DOWNLOADS_PUBLIC, whereas Drupal returns a path of $GLOBALS['base_url'] .'/'. file_directory_path() .'/'. str_replace('\\', '/', $path) for FILE_DOWNLOADS_PUBLIC.

quicksketch’s picture

Status: Active » Postponed (maintainer needs more info)

My guess is this may be because your site uses Pressflow's hook_file_url_alter() function? This code is at the top of file_create_url() in Pressflow, but otherwise the function is the same.

  // Clean up Windows paths.
  $old_path = $path = str_replace('\\', '/', $path);

  drupal_alter('file_url', $path);

  // If any module has altered the path, then return the alteration.
  if ($path != $old_path) {
    return $path;
  }

Down below in the same function the same code to strip out the file directory path still exists, just like in Drupal 6 core:

  // Shipped files.
  if (strpos($path, file_directory_path() . '/') !== 0) {
    return base_path() . $path;
  }

So my theory here is that this doesn't even happen a fresh install of Drupal 6 Pressflow with ImageField (even on a Windows machine). I think the problem is likely a module using hook_file_url_alter() and causing the image to break.

Soren Jones’s picture

So my theory here is that this doesn't even happen a fresh install of Drupal 6 Pressflow with ImageField (even on a Windows machine)
I'm testing on a Mac with a clean install of Pressflow with only Content, Filefield, and Imagefield enabled after install and an image field added to the default story content. I first noticed it on an Ubuntu server, but that wasn't a clean install of Pressflow.

After uploading an image theme_imagefield_admin_thumbnail() produces the link /imagefield_thumbs/file.jpg?1234567890 when it is expected to produce the link /sites/default/files/image/imagefield_thumbs/file.jpg?1234567890.

As sebas5384 said in #3...
Pressflow's file_create_url() is returning "base_path() . $path" from:

// Shipped files.
if (strpos($path, file_directory_path() . '/') !== 0) {
  return base_path() . $path;
}

So replacing return base_path() . $path; in the Pressflow 6.19.96 file_create_url() with return $GLOBALS['base_url'] .'/'. file_directory_path() .'/'. str_replace('\\', '/', $path); from the Drupal 6.19 file_create_url() resolves the issue. But that's no good. :-/

Especially because it should be returning from case FILE_DOWNLOADS_PUBLIC, right? But even if it did return from case FILE_DOWNLOADS_PUBLIC, it would return $GLOBALS['base_url'] . '/' . $path, which would still result in an incorrect link.

Applying a patch that reverses http://drupal.org/cvs?commit=461444 also resolves the issue. But that's no good for Windows users. :-/

quicksketch’s picture

Status: Postponed (maintainer needs more info) » Needs work

Huh, okay you're right. Hrmmmm I don't know what to do here. This sounds like PressFlow is breaking its golden rule of "100% API Compatibility".

A cheap fix could be doing an if (!function_exists('file_directory_strip')) { (another function that I don't think should even exist in Drupal 6 Pressflow), but that just makes it seem even more hacky. I'm open to suggestions here.

Soren Jones’s picture

Hrmmmm. I agree that it seems super hacky to do an explicit check for Pressflow. I suppose it would be hackier to have an imagefield_create_url(). ^^; There's a discussion thread at https://bugs.launchpad.net/pressflow/+bug/597718. CDN's solution is a Drupal 6 core patch because "file_create_url() in Drupal 6 is broken" (https://bugs.launchpad.net/pressflow/+bug/597718/comments/10).

See #839282: Problems with shipped files and Drupal 6 file_create_url() behavior for that CDN core patch.
See #877982: Fix pressflow incompatibility and standardize image object URL creation. for a Taxonomy Image patch.

I'm happy to patch Imagefield for my Pressflow Drupal 6 sites (just reversing http://drupal.org/cvs?commit=461444). If I correctly understand what I've read at CDN, it won't be an issue in Drupal 7. Maybe some others will weigh in.

Thanks for looking at it.

quicksketch’s picture

Status: Needs work » Needs review
FileSize
1.8 KB
2.49 KB

Okay here's a revised approach, which essentially url encodes only the latter part of the path (after the file_directory_path part). Because ImageField needs this in multiple places and FileField uses it also, this patch makes it so that ImageField depends on the new field_file_urlencode_path() function in FileField. A nice bonus is that we remove a lot of code from theme_* functions in ImageField, which probably shouldn't have been there to begin with.

I definitely have to agree that file_create_url() is quite broken in Drupal 6, as it's silly that private file URLs are encoded automatically (a side-effect of using the url() function internally), yet public file URLs are not. But while I appreciate PressFlow's performance enhancements, I don't really think it should be attempting to fix its problems (which it doesn't). The problem we have here is mostly an unintentional side effect of "shortening up" the code in PressFlow's file_create_url().

Soren Jones’s picture

Awesome! Thank you!

I tested both patches on the Mac Pressflow Drupal 6 clean install and on the Ubuntu Pressflow Drupal 6 site.
It works on both sites without any immediately apparent issues.

Testing on the Mac... With a breakpoint at switch (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC)) { in file_create_url(), I'm now seeing the correct value in $path, and continuing on to case FILE_DOWNLOADS_PUBLIC: and then to return $GLOBALS['base_url'] . '/' . $path; as expected.

I'll leave it to you to decide how many reviews are necessary before changing the issue status.
I suppose I'm just imagining a small performance gain?!

Thanks again!

Soren Jones’s picture

Also tested and works as expected with Private Files on the Mac Pressflow Drupal 6 clean install.

sebas5384’s picture

Thanks for the help!! and i wanna know if the .path that i made its going to give me future problemas on my pressflow installation.

I must use the patches from quicksketch in the #11 ?

Thanks!!

quicksketch’s picture

Title: Not showing images after latest update of ImageField » ImageField 3.8 not showing images when used with PressFlow
Status: Needs review » Fixed

I went ahead and committed these patches. Because a lot of users are upgrading now anyway because of the Chrome bug fixed in 3.8, I'm going to make a 3.9 release. The last thing I want is people to have to worry about PressFlow screwing up images.

AntiNSA’s picture

subscribe

so is everything patched in imagefeild/ Must I do anything to pressflow?

quicksketch’s picture

No, you just need to use ImageField 3.9.

Status: Fixed » Closed (fixed)
Issue tags: -Pressflow

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