The documentation for user_login_finalize(&$edit = array()) states:
@param array $edit
The array of form values submitted by the user.
This is not true: user_login_submit() uses the following code.
global $user;
$user = user_load($form_state['uid']);
$form_state['redirect'] = 'user/' . $user->uid;
user_login_finalize($form_state);
$form_state is passed to user_login_finalize(), not $form_state['values'] as we could expect from the name $edit and documentation.
It's better that $form_state is passed and not $form_state['values'] as it allows to redirect user after login by changing $form_state['redirect'] in hook_user_login().
Both functions' signature and documentation should be updated to reflect the actual behavior.
Issue fork drupal-3279652
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
avpadernoComment #3
avpadernoComment #4
anrikun commentedComment #5
avpadernoComment #6
avpadernoComment #8
avpadernoComment #9
anrikun commentedThanks @apaderno
But shouldn't signature be updated as well?
I don't think replacing $edit by $form_state could break anything and it would make things clearer.
Comment #10
anrikun commentedNice! :-)
Comment #12
poker10 commentedThanks for working on this!
Should we also rename the
$editparameter inhook_user_login()to get it done completely?If yes, I would prefer to do it here, so that we have all changes together.
Comment #13
avpadernoI changed the parameter name given in the
hook_user_login()definition too.Comment #14
anrikun commentedThank you!
Comment #15
poker10 commentedThere was a test failure caused by a change in Drupal images. The commit from this issue #3116482: Duplicate X-Content-Type-Options headers both with the value nosniff [D7] was missing here (it was already committed to the core), so a rebase was needed. Now tests are green.
I think this looks good. Adding a tag for a final review. Thanks!
Comment #17
mcdruid commentedLooks good, thanks.
Does any of this still apply to D10/11?
Comment #18
poker10 commentedI do not think this applies to D10/11, as the arguments seems to have changed - both functions have only $account as a parameter now:
https://api.drupal.org/api/drupal/core!modules!user!user.api.php/functio...
https://api.drupal.org/api/drupal/core!modules!user!user.module/function...
Comment #21
poker10 commentedAdding credits from the duplicate issue.
Comment #23
nicxvan commentedUpdating credit