Remember me wipes out user data (file field) after login

Here's another patch that properly saves $account data without wiping out everything. You have to include all of $account's properties in the $edit array, otherwise it all gets set to NULL.

see http://api.drupal.org/api/drupal/modules--user--user.pages.inc/function/...

Bad patch, don't try it.

Need to work out another way to save that data, and only that data.

Fixed by implementing hook_user_presave to properly attach the field to the data collection.

You have the following code in the module.

$account->data = array_merge($account->data, array('remember_me' => (bool) $remember_me));
user_save($account, (array) $account, NULL);

You need a similar change in remember_me_disable(). Perhaps:

if ($user->uid) {
     if(isset($user->data['remember_me'])) {
         unset($user->data['remember_me']);
     }
      user_save($user);
}

Thank you for raising your concerns, all comments are welcome and appreciated.

Rest assured, the reason for the array_merge at login and the bug-ger that got this issue raised was because the previous method, which was inherited from D6, replaced the whole collection in D7. When we clean up we are purely looking if the key exists and only removing the remember_me key and nothing else. That method will not harm anything else that might reside in the collection, so the kittens are safe.

I had an issue with user avatars being wiped out whenever the user logged in. It took a lot of time to finally figure out what was causing it.
Looking at the docs for accessing the $account object, I see the following;

$clonedaccount = clone $account

I altered the function remember_me_user_login function to first clone the $account object, then run user_save() on the cloned object, not $account. This did the trick for me.

The remember_me_user_login() function before;

function remember_me_user_login(&$edit, $account) {
  if ($account->uid) {
	user_save($account, array('remember_me' => !empty($edit['input']['remember_me'])), 'remember-me');
  }
}

After:

function remember_me_user_login(&$edit, $account) {
  if ($account->uid) {
    //user_save($account, array('remember_me' => !empty($edit['input']['remember_me'])), 'remember-me');
	$accountclone = clone $account;
	user_save($accountclone, array('remember_me' => !empty($edit['input']['remember_me'])), 'remember-me');
  }
}

Patch is attached.

Just to clarify, I was referring to http://api.drupal.org/api/drupal/modules%21user%21user.pages.inc/functio..., which includes this line;

$account_unchanged = clone $account;

I used the same approach. Somehow, using $account causes values in $account to change. Cloning the object avoids that, in my case anyway.

I don't get it, according to the link you are supposed to keep a copy for user_save and make changes on another. Yet all this patch is suggesting is to send a clone for modification.

This is equivalent to just going:

user_save(clone $account, array('remember_me' => !empty($edit['input']['remember_me'])), 'remember-me');

Since we are not doing anything with the original, how does that solve anything? Or am I missing something obvious?

Will make some time to investigate closer, glad it helped you to a resolve. Can you please give me a test scenario by which to verify that it solves a problem. Please provide the usual list of installed modules, what you expect to get, what you really got and how I can get to have it happen too please.

Yeah, we're not modifying the original. I don't really get it either, but it solved it for me.

As far as test scenarios, here was the situation;

User would log in, edit account, upload avatar, sign out.
User returns to the site later, logs in, and voila! Avatar is removed.

At first I thought maybe some modules were conflicting, and perhaps they are. However, the site in question uses only core profile fields (no Profile2). Logintoboggan is installed, but was not to blame. I deactivated it to make sure.

I wound up opening the database in phpMyAdmin and seeing what happened to the user's data right after the user logged in. This way I determined the avatar field was being reset as soon as the user logged in. It was being set to 0 I believe. By trial and error and based on what I saw in the documentation link cited earlier, I arrived at the suggested code, which worked, to my surprise.

Your code is simpler;

user_save(clone $account, array('remember_me' => !empty($edit['input']['remember_me'])), 'remember-me');

I just prefer to declare my variable first, rather than doing stuff inline.

I know it can be frustrating when you don't know what is to blame, this is how I adopted our beloved module you should have seen the disasters back then. The whole session would just vanish out of thin air to such an extent that it was impossible to filter the content whatsoever. But only when you didn't tick remember me =)

I'm happy if it does the trick then we won't ask for reasons why, thank you for taking the time to come back with your results and from a personal note, thank you for your patience I've been swamped in from all directions these last few weeks.

Your explanation is detailed enough I feel confident that I should be able to reproduce. What a relief it's no stack of 60 modules! I want to give it the attention it deserves and be thorough, I am sure I speak for everyone saying that we want to be sure it hits the target between the eyes.

On the simpler code you made reference to, there is usually a 1 to 1 correlation IMO between the number of code lines used and the quality of the code. If you are not using the reference then you are saving memory as well as the extra cycle it costs to assign it in the first place not to mention the extra work required to clean it up nor the space it occupies in the interim. I also find less code has less room for error and I consider it a win if a problem can be solved without the need for braces at all. If you are vigilant with writing less code you will be astonished at how much we waste.

This does not have to degrade legibility on the contrary, use the features that the language provide and reap the benefits instead of letting uncertainty foster a false sense of safety. Safe is boring lets not kid and boring makes you lazy and unfocused so you make mistakes. I would go a step further and claim its safer for everyone if we stay on our toes and focus on what we're doing, get the adrenalin pumping be excited about your work. Test it several times if your not sure, again and again, until you're certain, you only need to do that once, now you know and we all reap the benefit in leaner, tighter, snappier, bug free solutions as a result. So it is not about simpler anymore, do you see?

remember me used to delete the profile picture and throw an integrity constraint violation error evry time i logged in. that's why i had to remove this module, although i needed it badly. I was cluless about the error untill i removed the remember me module and things are back to normal now..hope this helps others caught in darkness also and in fixing this issue in this otherwise useful module.
error text:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '' for key 2: INSERT INTO {file_managed} (filesize, status, timestamp) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array ( [:db_insert_placeholder_0] => 0 [:db_insert_placeholder_1] => 1 [:db_insert_placeholder_2] => 1346747158 ) in drupal_write_record() (line 6776 of /home/www/mikipo.com/includes/common.inc).

@ironhit786

Would you please try the fix suggested by plethoradesign, that would really help!

Just simply change the call to user_save to

user_save(clone $account, array('remember_me' => !empty($edit['input']['remember_me'])), 'remember-me');

Let us know if it solves the problem.

Otherwise could you assess which module uses the file_managed database table? Since the error indicates that this table is receiving a duplicate entry which may mean that this module should consider that it might have inserted the record already. We do not use this table file_managed.

Reviewed not reviewed?... I am not to proud to admit we obviously need some help around here.

nickl, it seems you and I are the only ones interested in this? Either of us could propose a patch, but I'd rather the maintainer sees this before reviewing any patch, because this could affect other things in the module (?)

plethoradesign

nickl == maintainer =)

Ha, good point ;) So, has anyone else tried my fix? I haven't had occasion to use this module recently.

@plethoradesign no feedback received as of yet...

Hmm, OK. You could put it in dev and then you should automatically get some feedback when people pull the dev branch ..

From my investigation, the _remember_me_set_lifetime function is to blame.

When it starts the session, all the original data that was in the global $user object is discarded and the hooks don't re-run. Why would they even need to? The data is already correct as far as I'm concerned.

My solution so far was to clone $user before the session gets (re)started and then reinstate the cloned data into the clobbered $user variable.

Sorry, does this problem still exist? Does it only concern profile picture (the core one?)
Could it be, that there is a parallel to https://www.drupal.org/node/2390715 ?
In that post the author also seems to have problems with the user_save function (but I am not sure, if it is the same line of code ...)

-    user_save($account, array('remember_me' => !empty($edit['input']['remember_me'])), 'remember-me');
+    $account->data['remember_me'] = !empty($edit['input']['remember_me']);
+    drupal_write_record('users', $account, 'uid');

Could this be helpful?

Has there been any resolution regarding this issue?

Attaching a new patch that seems to solve both this issue and #2390715: Using save_user on user login cause problem with other modules by:
- reloading the user entity without caching on hook_user_login to ensure all properties are properly populated
- using db_update instead of user_save to inject the $user->data remember_me marker and remove the need for user_presave hook
Will try and keep posted when we've conducted more tests, feedback welcome in the meantime.

I updated to version 7.x-1.1, and users were losing their profile picture on login as described above, with an error message

Notice: Object of class stdClass could not be converted to int in drupal_write_record() (line 7340 of /home/[cPanelname]/public_html/includes/common.inc)

The profile pictures of users who did not login since the upgrade were still there.

I reinstalled 7.x-1.0 (stable), and the profile picture is remembered, with no error on login.

You have attached a new patch to fix issue commented in #27 (version 7.x-1.1). I hope it works for you.

Hi @oneblankspace, if you could test the patch that @vengador has provided and mark this as RTBC if you are happy then we can get this committed ready for a new release

I had a similar issue with 7.x-1.1 where the update would revert the picture of users in Drupal to the picture with id of 1.
The patch from @vengador in #28 resolved this issue for me, marking as RTBC.

#28 worked for me too.

#28 worked for me too. Thanks, lot of time went into debugging this issue!!! :(

#28 works

#28 works
+1

#28 works for me, thanks.

Thanks everyone and sorry for the delay - this has been pushed and included in the new release!