Resubmitting a user picture does show the first uploaded picture

I ran into this issue too. At first, I thought it was some other module messing something up, such as ImageCache. Thanks to Drupal user andypost and this post (http://drupal.org/node/615304#comment-2200248) I was able to come up with my own solution.

The problem is that the user's profile picture is named the same filename each time the same user uploads a new profile picture for themselves. I would guess the browser sees the same filename and doesn't bother to reload the newly-uploaded profile picture until a manual refresh.

The attached patch gets the current epoch timestamp, via PHP's time() function, and adds that before the file extension when the filename is composed. I have only tested it a little bit, but it does seem to work the way I want, and the old image gets properly removed after uploading a new user profile picture to replace it.

Hope that helps someone.

Whoops. I copied the patch text from the editor window, but the longer lines didn't get copied. Please use the attached patch instead.

Correcting status.

Perfect!! Thank you!

This issue fixed in D7 so this fix for 6 branch

Solution in #2 is good but it changes a way of file-naming so I think it cant go into D6

#2 is definitely a good idea, and doesn't change how existing images are named (only new filenames are affected).

Where/how was Drupal 7 fixed?

Patch in #2 works as expected

Ok, looking at current Drupal 7, this does not seem to be fixed at all. Let's get fixed in D7 and then in D6. Here is the relevant D7 code snippet.

      // Process picture uploads.
      if (!empty($edit['picture']->fid)) {
        $picture = $edit['picture'];
        // If the picture is a temporary file move it to its final location and
        // make it permanent.
        if (($picture->status & FILE_STATUS_PERMANENT) == 0) {
          $info = image_get_info($picture->uri);
          $picture_directory =  variable_get('file_default_scheme', 'public') . '://' . variable_get('user_picture_path', 'pictures');

          // Prepare the pictures directory.
          file_prepare_directory($picture_directory, FILE_CREATE_DIRECTORY);
          $destination = file_stream_wrapper_uri_normalize($picture_directory . '/picture-' . $account->uid . '.' . $info['extension']);

          if ($picture = file_move($picture, $destination, FILE_EXISTS_REPLACE)) {
            $picture->status |= FILE_STATUS_PERMANENT;
            $edit['picture'] = file_save($picture);
          }
        }
      }
      $edit['picture'] = empty($edit['picture']->fid) ? 0 : $edit['picture']->fid;

1) Naming user picture with pattern "picture-UID-REQUEST_TIME.EXT"
2) Fixed test, old one assumes that 'file_default_scheme' is always public

Changed return statement in test.

Improved test

Agreed, seem more reasonable

Me too.

Looks good

But we can't change the core module(user.module). May be this will get error when we upgrade the version.

Another way to change the user picture.

Use hook_user function, in switch case, rename the file path with timestamp and update user table as same file path

Example:

function hook_user($op, &$edit, &$account, $category = NULL) {
switch($op){
case 'after_update':
case 'after_update':
$old_filename = $account->picture;
$filename = basename($account->picture);
$fileArray = explode('.', $filename);
$finalFilename = $fileArray[0].'-'.time().'.'.$fileArray[1];
$pictureArray = explode('/', $account->picture);
$pictureArray[count($pictureArray)-1]=$finalFilename;
$picturePath = implode('/', $pictureArray);
$account->picture = $picturePath;
rename($old_filename, $picturePath);
db_query("UPDATE users SET picture='$picturePath' WHERE uid=%d", $account->uid);
break;
}
}

This will work fine.

Committed to CVS HEAD. Thanks.

Back RTBC for #2

The Drupal 6 patch does not use Drupal 6 coding standards. Also, the D7 patch renames the image instead of replacing (since an image with this name is unlikely). Is that not a good change? Why is it not in the D6 patch?

Re-roll with fixed code-style. Also changed FILE_EXISTS_REPLACE with FILE_EXISTS_RENAME to avoid race-conditions as proposed in #14

Also useful cleanup in #173493: User picture fixes and cleanup

This worked but I think it caused another problem.

1) User clicks to upload a new picture, browses for picture, and submits it.
2) User gets an error back after picture loads (picture does not have enough pixels, it's to big, etc.)
3) If the user decides to not upload a new picture their profile no longer has a picture at all. EVEN if profiles require it.

Is there a way to fix this?

The patch in #24 works for me.

Hi,
I need to use the patch in #2, but I'm not sure how or where to use this code. Please provide more detail.
Much Appreciated!

http://drupal.org/node/620014

http://drupal.org/patch/apply

Note: The current patch is in #24.

@dc1258: how do you make the picture required? Only options I see in D6 are no picture or an optional picture.

Check it out here:

http://drupal.org/project/reg_with_pic

@dc1258: then you have an issue with a contributed module, not Drupal core, submit the issue to the queue of that module please.

My issue isn't with the requirement of a picture. The issue is that, using the patch in 24, can cause other issues.

Suscribing and also a little help.

What if, instead of putting the $image_time variable as the name of the file, we just use a get value for that image, like www.example.com/sites/default/images/myimage.png?image_time=24566757.

I used to do that solution before in some Ajax webpages, but now, maybe it will create some troubles with other modules like lightbox. But who knows.

Just wondering

Suscribing

I am applying the changes suggested in #24 in a project that had just this problem ... if a user updates the image, it would not show up. Now it does ... great success.

My Site has the same problem.
But I don't know how to use/ apply the Patch.
Please give detailed instruction..
Thanks in advance

I have the same problem.
Please give me the detailed instruction how to apply the PATCH. in user.module.
Thanks
PS:
I am newbie

@duyviet

see #28

@dc1258: How did that problem not happen before the patch? I can that maybe some code might look for files with the old pattern, but all the data is in the user table / object so if it is dependent on the file pattern, maybe that was not the best choice. I can see how this break backwards compatibility though :| Maybe we can link to the image as $filename?$change_timestamp - no? That way we can keep saving to the same filename, but the URL itself will change. So old modules will just find the file in place as usual. They need to change for Drupal 7 anyway, since dynamic file naming is there in Drupal 7.

I really don't think that it is a good idea to change the path of the user image in the database. The better solution should be to change the URL to the user image before sending it to the browser.

In hook_user on operation load and operation view simply attach the file last change date to the url in the $account['picture'] variable to get an url like in #33

sites/default/images/myimage.png?image_time=24566757

- I've updated a site to 7.0 and all users now have a "picture" that is a file (not necessary a picture, it can be a pdf, etc) attached to the a node they have submitted. The file names of pictures didn't change (no timestamp).

- I've updated another site locally, but in the local machine there was no pictures folder, because I thought I was upgrading the database only (not the files names). Now, in the live site, the pictures folder is there, but the pictures aren't shown in the profiles pages (I seems that there is no linking between the profiles and the files). If one users uploads a picture, the picture has the new format (timestamp at the end).

Have I lost the users pictures???

Is this another unrelated bug?? Should I create a new bug report for D7.0?

I've guided 3 new students through testing this bug. They are going to post their findings below - I have checked the issue on Google Chrome, Drupal 6.20 and PostgreSQL 8.1 and found the same as they did (unable to reproduce the issue with no patch applied).

I have reviewed the problem and have followed both sets of the instructions and was unable to reproduce the bug, I didn't apply any patches.

Drupal 6.20
Chromium 8.0.552.224.
Postgres 8.4
Apache2
Ubuntu 10.04

I tested this issue too (in Firefox-3.6.13) by following the instructions from the original submitter and I could not reproduce the problem. I did not apply any patches and I'm using Drupal-6.20(:

I am using Drupal 6.20 and I have tested this issue too, using the instructions above. I didn't come across any problems. I used Chromium 80.552.224 and I didn't apply any patches.

subscribe

I'm using the "me" module, and I experienced the same problem.

user/me/view - displays the new picture. Clicking the image links to the user profile page with the old picture.

users/userid - displays the old picture unless the user refresh the browser.

applying the patch broke the site.

http://drupal.org/project/unique_avatar

Regarding http://drupal.org/node/668058#comment-3317440

          // save picture to correct path and update the row in the user table
          $destination = variable_get('user_picture_path', 'pictures') .'/picture-'. $user->uid .'.'. $info['extension'];
          if (file_copy($file, $destination, FILE_EXISTS_REPLACE)) {
            db_query("UPDATE {users} SET picture='%s' WHERE uid=%d", $file->filepath, $user->uid);
          }

This is code from the reg_with_pic module. This should be better replaced by using user_validate_picture() internal.
@dc1258
Do you have problems on the user edit form or only on the registration page with this page? This information would be really helpful.

Regarding http://drupal.org/node/668058#comment-3844392
This is just changed for new uploaded pictures, so if someone uses "picture-$user->uid" as raw path in the template it's quite a
bug.

So, I'm confused. Is the patch in #24 the solution? I just tried running it on 6.22, but it failed. Probably because it was rolled for an earlier version. If this is the suggested solution, what I don't understand is why it didn't make it into core?

Are there other solutions out there that people are using? I've searched but not come up with much. This seems like too big and old of an issue to not already be solved.

I posted a link to a module solution in #48.

Once again... http://drupal.org/project/unique_avatar

Thanks mr.j, I saw that. But my question is about the core way of dealing with this.

The same distinction exists in #850292: This functionaly proposed for core.

any updates? bumping ...

subscribing

confirming that this works.

Thanks for the patch code. patch Its works fine for me. No more old image showing

This patch seems to work for me too:
http://drupal.org/files/issues/668058-user_picture-d6.patch

#24: 668058-user_picture-d6.patch queued for re-testing.

Sure would like to see this finally get solved before D6 is unsupported :-)

Reroll of #24. It wasn't rolled from the repo root.

OK.. somebody RTBC it *again*.

Randy, I scare that this approach would break BC, mostly I cares about contrib (imagecache_profiles, views for example)
#33 aproach seems a bit better but first we should understand why D7 works fine without this change

Wasn't D7 committed in #21 ?

I'd be happy for a D6 approach to this for imagecache_profiles. There's a D7 in the queue.

Thanks @andypost & @rfay, you save my day

I've seen this happen when we set ExpiresActive On for image files in httpd.conf. If you want to keep that setting, but disable it for user pictures, that can be done with something like:

<FilesMatch "picture?">
ExpiresActive Off
</FilesMatch>

Subscribing

#62 doesn't actually work... It works for the first user picture change, but after you do that, you have to delete the picture and reupload to get another picture in there.