When a user logs in using a the user login block on, say, http://example.com/node/1, the form submits to http://example.com/node/1?destination=node/1. If the login succeeds, this user is redirected to http://example.com/node/1, i.e. the same page but now the user is logged in.

The redirect happens when the login block is rendered, i.e. via drupal_render_page() in index.php, through drupal_alter() → block_page_alter() → block_list() → _block_render_blocks() → user_block_view() → drupal_get_form() → drupal_process_form() → drupal_redirect_form().

If the page e.g. does some expensive database lookups, it is a waste of resources to generate it twice. This patch makes the form submit to a small menu callback that redirects to the referring page.

CommentFileSizeAuthor
#15 user-login-5.patch3 KBc960657
#10 user-login-4.patch3.02 KBc960657
#8 user-login-3.patch3.01 KBc960657
#4 user-login-2.patch2.96 KBc960657
user-login-1.patch2.8 KBc960657

Comments

c960657’s picture

c960657 commented
Issue tags: +Performance

+Performance

moshe weitzman’s picture

moshe weitzman commented
Priority: Normal » Minor

Status: Needs review » Needs work

The last submitted patch failed testing.

c960657’s picture

c960657 commented
Status: Needs work » Needs review
StatusFileSize
new user-login-2.patch2.96 KB

Reroll.

Status: Needs review » Needs work

The last submitted patch failed testing.

c960657’s picture

c960657 commented
Status: Needs work » Needs review

Test bot glitch?

Status: Needs review » Needs work

The last submitted patch failed testing.

c960657’s picture

c960657 commented
Status: Needs work » Needs review
StatusFileSize
new user-login-3.patch3.01 KB

Reroll.

Status: Needs review » Needs work

The last submitted patch failed testing.

c960657’s picture

c960657 commented
Status: Needs work » Needs review
StatusFileSize
new user-login-4.patch3.02 KB

Reroll.

z1rra’s picture

z1rra commented

I actually somehow got rid of this problem by converting all my template files in my theme to ANSI, instead of UTF-8. I have no idea how that works but there it is. Also, it somehow made my redirect to URL from login block not work.

c960657’s picture

c960657 commented

#10: user-login-4.patch queued for re-testing.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Patch looks good, it'd also help with #2946: Login fails and no warning is issued if cookies are not enabled by making the redirect option there cheaper.

dries’s picture

dries commented 
+++ modules/user/user.pages.inc	19 Oct 2009 20:18:28 -0000
@@ -138,18 +138,30 @@ function user_pass_reset($form, &$form_s
+function user_login_redirect() {
+  global $user;
+  // If the login succeeds, FAPI redirects based on $form['#redirect'];
+  // otherwise drupal_goto() redirects to the URL specified using the
+  // ?destination= parameter.
+  drupal_get_form('user_login_block');
+  drupal_goto();
+}

Seems like the global $user could be removed?

c960657’s picture

c960657 commented
StatusFileSize
new user-login-5.patch3 KB

You're right. Fixed.

marcingy’s picture

marcingy commented

#15: user-login-5.patch queued for re-testing.

dries’s picture

dries commented

Reading up on this patch again ...

1. I don't think this custom callback is that much cheaper.

2. I don't really understand this code comment, and how it is relevant to the underlying code. I'm sure I can figure it out, but it would be better if the comment made sense without additional research.

+++ modules/user/user.pages.inc	15 Apr 2010 16:18:06 -0000
@@ -153,18 +153,29 @@ function user_pass_reset($form, &$form_s
+  // If the login succeeds, FAPI redirects based on $form['#redirect'];
+  // otherwise drupal_goto() redirects to the URL specified using the
+  // ?destination= parameter.
+  drupal_get_form('user_login_block');
+  drupal_goto();
dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work
c960657’s picture

c960657 commented

The patch has a problem when login fails: The username field that is reported as the problematic field using form_set_error() is not highlighted due to the redirect.

owen barton’s picture

owen barton commented
Version: 7.x-dev » 8.x-dev
klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

...just saying that the issue title makes it look more like a bug rather than a task.

Status: Needs work » Needs review

dcine queued 15: user-login-5.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 15: user-login-5.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue summary: View changes

Looks this issue makes no sense for d8

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Needs work » Closed (won't fix)

Doesnt seem to be an issue in D7 or 8