The password and password_confirm fields do not handle empty values very well (at least in D6). When you submit a settings form that contains a password / password_confirm field, you have to enter the password every time you make a change or it will reset the variable entry to empty string. It seems like this applies to any situation where you have module settings that automatically get saved to variables when the form is submitted.

This is resulting in bugs (issue 3 below) and also questionable practices (the passwords and some API keys in the other ubercart payment gateway modules are textfield instead of password because of this).

Although the User edit account form behaves well, it is not the same kind of form as these settings forms, so the same solution doesn't really apply very well. Really, it shouldn't require any (or at least any significant) work to make your password field retain its old selection if you don't enter a new value. I'm fine with adding a new field parameter that indicates the field is "optional" in this sense, where a new empty value results in no change.

Some relevant issues: 1 2 3

#1 password_test.tar_.gz737 byteskwinters
Members fund testing for the Drupal project. Drupal Association Learn more


kwinters’s picture

Title: password / password_confirm fields do not handle empty values very well » system_settings_form does not support password / password_confirm fields well
Version: 6.x-dev » 7.x-dev
Status: Active » Needs work
737 bytes

Installed D7 and wrote a tiny module to confirm that the problem exists there as well (attached).

If someone can give me some guidance about what an appropriate fix for this would be, I can do the legwork. The two approaches I can think of right now are modifying system_settings_form not change the value if the POST for that field was empty and it was type password / password_confirm OR modifying the form system to return the default value in some case (probably requires a new option).

Also related: password_confirm does not support #default_value at all (Fatal error: Unsupported operand types in includes/ on line 1312)

kwinters’s picture

Forgot to leave instructions on using the module.

After enabling it, go to admin/settings/password_test and fill out the form, then submit. The 3 variables will be saved correctly to the database. You've been redirected back to the form, which should now be filled in with the actual data. Submit the form again, and it will blow out your password settings.

phayes’s picture

I think the correct behavior would be to edit the system_settings_form processor and not update any password fields that are left blank.

alexpott’s picture

Had this issue myself on a new module I'm building...
here's a way to do this without changing core Drupal...

function example_admin_settings() {
  $form = array();

  /* Some settings form with password field */

  $form['example_password'] = array(
    '#type' => 'password',
    '#title' =>  t('Password'),
    '#default_value' => variable_get('example_password', ''),
    '#description' => t('Enter the password to connect'),

  $form = system_settings_form($form);
  //Add custom submit handler so that passwords will not be overwritten if saved with no value
  array_unshift($form['#submit'], 'example_admin_settings_submit');

  return $form;

function example_admin_settings_submit($form, &$form_state) {
  if ($form_state['values']['example_password'] == '') {
DamienMcKenna’s picture

@phayes: then how would you erase an existing non-required value? I've a gut feeling that the password field needs additional logic, looking at the bare field it just seems its creators expected implementors would intuit the need for further custom logic.

gdd’s picture

Status: Needs work » Closed (won't fix)

system_settings_form() is being removed in D8 (see #1696224: Remove system_settings_form()) so I'm marking this wont fix.

DamienMcKenna’s picture

@heyrocker: Is it not worth pursuing for D7?

gdd’s picture

Status: Closed (won't fix) » Active

Ack I just noticed this was a D7 issue, not a D8 one. My bad.

kwinters’s picture

Also, does whatever replaces it in D8 actually fix the issue? The problem may be at the form layer instead of unique to system_settings_form().

gdd’s picture

There is no replacement in D8, you just build your forms as normal and write your own submit functions.

deepcore’s picture

Issue summary: View changes

@alexpott: Works well in D7.x. Thanks :) After 7 years, LOL :)