A password_confirm field won't display the error highlight (or in other words, won't include the 'error' class the element class attribute on the two password elements). Problem here is caused by form_get_error() returning FALSE when there is no error message associated with the element.

I looked at two ways of fixing this:

  1. Modify the theme_* functions to have this operate properly.
  2. Modify the form_get_error() function so all current calls work as desired and change the single call for _form_set_class() to call it in a slightly different manner.

I chose #2 because it is a more global solution to the problem. If a form element has been flagged by form_set_error(), it should highlight regardless of it having a message associated with it. This may have the consequences that module developers have worked around this bug and they might get double highlights.

Patch is attached.

Sammy Spets
Synerger Pty Ltd
http://www.synerger.com

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

Status: Active » Needs work

Something sets $form[$key] to FALSE, right? Fix that instead.

sammys’s picture

Thanks for responding quickly. The following two lines in form.inc [password_confirm_validate()] contribute to this problem.

form_error($form, t('The specified passwords do not match.'));
form_error($form['pass1']);
form_error($form['pass2']);

As you can see the errors for pass1 and pass2 are flagged here. However you can't set any messages for the second or third calls to form_error() or you'll get duplicate messages displayed. Really are only those two ways to fix it, which I stated in my original post.

sammys’s picture

Status: Needs work » Needs review
FileSize
2.86 KB

A third solution to the problem exists and is, IMO, a far superior solution to those proposed earlier. It means there is only need for one single form_set_error() call to flag both boxes. This solution will most likely solve the problem given in http://drupal.org/node/56278 and _does_ solve the problem I posted in http://drupal.org/node/59532.

The solution requires no further changes to core for it to work. Modules implementing some password validation of their own will need to add one small bit of code to each use of $form_values['pass1'] or $form_values['pass2'].

Patch is attached to this message. I leave it up to the powers that be to determine which fix they want to use.

chx’s picture

Status: Needs review » Needs work

Nice approach but needs work. A space is mandatory after comma (never before), I would prefer $key instead of $e, why the $element['#tree'] = TRUE; when you already set the #parents of the siblings, and there is a { } after if even for one line.

sammys’s picture

Status: Needs work » Needs review
FileSize
2.61 KB

Here is the updated patch. You asked why the 'tree' when i've set the parents. Good question. :)

I've added a space after comma, changed $e to $key and added braces around single line if blocks. Let me know if there's any other changes you wish me to make.

chx’s picture

Status: Needs review » Needs work

yes, drop the #tree TRUE. If it does not work, then leave it in but it will work.

chx’s picture

Status: Needs work » Needs review

Oh wait, you dropped #parents, that's a better idea :)

sammys’s picture

See the patch. the parent setting has been removed and tree fulfills the duty.

sammys’s picture

lol

chx’s picture

Status: Needs review » Needs work

Sorry, but after this change the foreach loop became so simple that it looks better without a loop...

sammys’s picture

Status: Needs work » Needs review
FileSize
2.67 KB

changed and tested.

Robin Monks’s picture

Status: Needs review » Reviewed & tested by the community

Tested to work on:

WindowsXP + Apache + MySQL 5 + PHP 5
and
WindowsXP + Apache + MySQL 5 + PHP 4

Patch applied cleanly, and code looks clean.

+1, ready to commit.

killes@www.drop.org’s picture

Status: Reviewed & tested by the community » Fixed

applied

Anonymous’s picture

Status: Fixed » Closed (fixed)