Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.If you look at the system_element_info the password confirm element looks like this:
<?php
$types['password_confirm'] = array(
'#input' => TRUE,
'#process' => array('form_process_password_confirm', 'user_form_process_password_confirm'),
'#theme_wrappers' => array('form_element'),
);
?>
I completely understand why form_process_password_confirm is there, but user_form_process_password_confirm probably doesn't belong there.
If anything this should be added, on the user_account_form callback.
This assumes the password_confirm element is always being used on the user register.
Comments
Comment #1
ericduran CreditAttribution: ericduran commentedTagging
Comment #2
pgrond CreditAttribution: pgrond commentedMade a patch for the removal of user_form_process_password_confirm
Comment #3
pgrond CreditAttribution: pgrond commentedComment #4
ericduran CreditAttribution: ericduran commented@pgrond, awesome.
But the user_form_process_password_confirm still needs to be added when on the user form.
Comment #5
pgrond CreditAttribution: pgrond commentedAh ok. Something like this? Not sure if you can just use #process => array('user_form_process_password_confirm') in the element declaration. I guess this would override the reference to 'form_process_password_confirm', so I'm checking if the element exists and add the function to the #process array.
Comment #6
pgrond CreditAttribution: pgrond commentedComment #8
rbayliss CreditAttribution: rbayliss commentedYou're right that adding a #process handler in the form declaration overrides the system level #process handler, but this will happen regardless of where the handler is added, so long as it is inside the user_account_form. How about switching user_form_process_password_confirm to a #pre_render handler, since it is adding the script for display purposes anyway.
Comment #9
ericduran CreditAttribution: ericduran commented@rbayliss yea, I think this is better.
Comment #10
roborn CreditAttribution: roborn commentedThis form now uses the new entity form controller.
I've tried the patch and everything looks good.
Comment #11
enhdless CreditAttribution: enhdless commentedPatch is in need of rerolling.
Comment #12
martin107 CreditAttribution: martin107 commentedReRoll
Comment #13
martin107 CreditAttribution: martin107 commentedComment #14
martin107 CreditAttribution: martin107 commentedComment #15
catchCommitted/pushed to 8.x, thanks!
Comment #16
David_Rothstein CreditAttribution: David_Rothstein commentedThis removed the client-side password-match validation and password strength indicator from the installer (which I consider critical because the user 1 account is the most important one to have a strong password).
I also don't think this change makes sense. Why wouldn't we want these things on all password confirm elements by default??? If someone really wants to remove it on a particular form, they can remove it there, but it's beneficial for security and usability (as well as developer experience) to have it there by default without the burden of developers having to remember to add it every time.
I would suggest rolling this back, then maybe following up by simply moving the code out of the User module and into form.inc (although keeping it #pre_render rather than #process seems reasonable), or alternatively having User module add it via hook_element_info_alter() (to address the original issue). The code isn't particularly specific to the User module at all.
Comment #17
catchDon't have a strong opinion on moving that code to form.inc vs. keeping it separate, however reverted the patch due to the regression regardless.
Comment #30
catchThis is no longer the case - PasswordConfirm (in core/lib) doesn't reference user module at all.