If user pictures are enabled and I edit my account, this is what is saved into the 'data' column of my users's db record:
a:4:{s:7:"contact";i:0;s:5:"block";a:0:{}s:14:"picture_delete";i:1;s:14:"picture_upload";s:0:"";}

The picture_delete and picture_upload do not need to be saved and for a site with a lot of users, that's a tiny less amount of processing for unserialize in user_load().

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Status: Active » Needs review
FileSize
3.39 KB

Attached patch that sets the values of those two fields to NULL in _edit_user_submit. Also fixed some FAPI code styles in the user picture account form.

Dave Reid’s picture

Since this does not implement an API change, a minimal version of this patch should be backported to 6.x.

Dave Reid’s picture

Even nicer formatting of the user_picture FAPI fields using #access. Yay FAPI!

EDIT: Patch has passed testingbot:
http://testing.drupal.org/pifr/file/1/2205

Dave Reid’s picture

Title: picture_upload and picture_delete should not be saved into $user->data » picture_upload and picture_delete should not be saved into $user->data + form cleanup
FileSize
3.62 KB

Revised patch that comments why there's #access properties on the form elements. Simply, if the user does not have a current user picture, it hides the picture preview and delete picture checkbox.

Instead of:

    $form['picture'] = array('#type' => 'fieldset', '#title' => t('Picture'), '#weight' => 1);
    $picture = theme('user_picture', (object)$edit);
    if ($edit['picture']) {
      $form['picture']['current_picture'] = array('#markup' => $picture);
      $form['picture']['picture_delete'] = array('#type' => 'checkbox', '#title' => t('Delete picture'), '#description' => t('Check this box to delete your current picture.'));
    }
    else {
      $form['picture']['picture_delete'] = array('#type' => 'hidden');
    }
    $form['picture']['picture_upload'] = array('#type' => 'file', '#title' => t('Upload picture'), '#size' => 48, '#description' => t('Your virtual face or picture. Maximum dimensions are %dimensions and the maximum size is %size kB.', array('%dimensions' => variable_get('user_picture_dimensions', '85x85'), '%size' => variable_get('user_picture_file_size', '30'))) . ' ' . variable_get('user_picture_guidelines', ''));

With the patch we have:

    $form['picture'] = array(
      '#type' => 'fieldset',
      '#title' => t('Picture'),
      '#weight' => 1,
    );
    // If the user does not currently have a picture, hide the preview and the
    // delete checkbox.
    $form['picture']['current_picture'] = array(
      '#markup' => theme('user_picture', (object)$edit),
      '#access' => $edit['picture'],
    );
    $form['picture']['picture_delete'] = array(
      '#type' => 'checkbox',
      '#title' => t('Delete picture'),
      '#description' => t('Check this box to delete your current picture.'),
      '#access' => $edit['picture'],
    );
    $form['picture']['picture_upload'] = array(
      '#type' => 'file',
      '#title' => t('Upload picture'),
      '#size' => 48,
      '#description' => t('Your virtual face or picture. Maximum dimensions are %dimensions and the maximum size is %size kB.', array('%dimensions' => variable_get('user_picture_dimensions', '85x85'), '%size' => variable_get('user_picture_file_size', '30'))) . ' ' . variable_get('user_picture_guidelines', ''),
    );

Much cleaner and easier to understand

Status: Needs review » Needs work

The last submitted patch failed testing.

Dave Reid’s picture

Status: Needs work » Needs review
FileSize
3.58 KB

Hmm...I tested the patch and it applied cleanly to HEAD (with just a tad bit of offset). Reposting to let testing bot give it another try.

Status: Needs review » Needs work

The last submitted patch failed testing.

Dave Reid’s picture

Dave Reid’s picture

Issue tags: +user pictures
Dave Reid’s picture

Version: 7.x-dev » 6.x-dev

#357403: Move user picture to managed files fixed this in 7.x, so moving back to 6.x.

Dave Reid’s picture

Status: Needs work » Closed (duplicate)