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

Command icon 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

anrikun created an issue. See original summary.

avpaderno’s picture

Issue summary: View changes
avpaderno’s picture

Component: user.module » documentation
anrikun’s picture

Issue summary: View changes
avpaderno’s picture

Title: Erroneous signature and documentation for user_login_finalize() » Erroneous signature and documentation for user_login_finalize() and hook_user_login()
Issue summary: View changes
avpaderno’s picture

Issue summary: View changes

avpaderno’s picture

Category: Bug report » Task
Status: Active » Needs review
anrikun’s picture

Thanks @apaderno
But shouldn't signature be updated as well?

<?php
function user_login_finalize(&$form_state = array()) {
//...
}
?>

I don't think replacing $edit by $form_state could break anything and it would make things clearer.

anrikun’s picture

Status: Needs review » Reviewed & tested by the community

Nice! :-)

poker10’s picture

Status: Reviewed & tested by the community » Needs review
Related issues: +#2704235: hook_user_login $edit array should be called $form_state to prevent confusion.

Thanks for working on this!

Should we also rename the $edit parameter in hook_user_login() to get it done completely?

If yes, I would prefer to do it here, so that we have all changes together.

avpaderno’s picture

I changed the parameter name given in the hook_user_login() definition too.

anrikun’s picture

Status: Needs review » Reviewed & tested by the community

Thank you!

poker10’s picture

Issue tags: +Pending Drupal 7 commit

There 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!

  • mcdruid committed fc873aa3 on 7.x
    Issue #3279652 by apaderno, poker10, anrikun: Erroneous signature and...
mcdruid’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Pending Drupal 7 commit

Looks good, thanks.

Does any of this still apply to D10/11?

poker10’s picture

I 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...

poker10 credited nicxvan.

poker10 credited Vinay15.

poker10’s picture

Adding credits from the duplicate issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

nicxvan’s picture

Updating credit