Login_destination / Thickbox redirection issues

paulnem - February 29, 2008 - 20:16
Project:Thickbox
Version:5.x-1.1
Component:Code
Category:support request
Priority:normal
Assigned:Unassigned
Status:patch (code needs work)
Description

Hi

I'm having an issue with the login_destination module and thickbox. It seems with login_destination enabled and no thickbox, users logging in are redirected to the correct destination. With thickbox enabled, login_destination is bypassed and thickbox directs everyone to the front page.

It seems this function, within thickbox is controlling where logging in users go. It doesn't seem to play nice with login_destination.

function thickbox_form_alter($form_id, &$form) {
if ($form_id == 'user_login' && arg(0) == 'thickbox_login') {
$form['#action'] = url('user/login', 'destination='. $_GET['destination']);
$form['name']['#size'] = 25;
$form['pass']['#size'] = 25;
}

So I'm really not sure at this point. I can change that destination to the page I want, but that seems like a dodgy situation to be in.

As I've already said, without thickbox, login_destination works fine.

I've also logged a support request at the login_destination queue.

http://drupal.org/node/226277

Any help would be great.

Thanks

#1

mpaler - March 5, 2008 - 18:56

subscribing

#2

drupalina - March 30, 2008 - 13:30

subscribing

it doesn't work with Drigg module either

#3

rkn - April 9, 2008 - 18:53

I'd be interested to see an an option in thickbox which doesnt redirect you back to the index when you login. Clearly users should stay on the page where they clicked the thickbox link. Otherwise this defeats the point of having thickbox in the first place. Why have a simple pop up when it does the same thing as a separate login page?

#4

Logi83 - May 17, 2008 - 22:34

Subscribing

#5

vegeneric - June 3, 2008 - 17:55

here's an updated function thickbox_form_alter that fixes this issue... it just checks if login destination is enabled and if so defers to that module's default destination behavior. perhaps not the best way but it definitely works.:

thickbox.module (~ line 145)

function thickbox_form_alter($form_id, &$form) {
  if ($form_id == 'user_login' && arg(0) == 'thickbox_login') {
    if (module_exists('login_destination')) {
      $form['#action'] = url($_GET['q'], 'destination=login_redirect');
    } else {
      $form['#action'] = url('user/login', 'destination='. $_GET['destination']);
    }
    $form['name']['#size'] = 25;
    $form['pass']['#size'] = 25;
  }
}

#6

vegeneric - June 3, 2008 - 18:13

honestly i'm not sure why thickbox needs this customized redirection anyway... it doesn't serve any purpose that i can tell. maybe it would be simpler to just remove this line from thickbox_form_alter:

$form['#action'] = url('user/login', 'destination='. $_GET['destination']);

and let other modules do their own thing? i cut that line out and everything seemed to work just fine.

#7

vegeneric - June 3, 2008 - 18:17
Status:active» patch (code needs review)

#8

christefano - June 25, 2008 - 00:13
Status:patch (code needs review)» patch (code needs work)

@vegeneric, can you post a patch please? I'll be happy to mark it RTBC if your patch comments out the line you mention in #6. http://drupal.org/create/patch

#9

paulnem - July 3, 2008 - 21:39

I've tried the options from both #5 and#6 above on my thickbox 2.0 install but there are issues when the user isn't valid, either wrong username or wrong password. it sends them to a page (rather than a thickbox) outside the theme to try again.

I've reverted (for my specific purposes) it to this :

<?php
function thickbox_form_alter($form_id, &$form) {
  if (
$form_id == 'user_login' && arg(0) == 'thickbox_login') {
   
$form['#action'] = url('user/login', "destination=login_redirect");
   
$form['name']['#size'] = 25;
   
$form['pass']['#size'] = 25;
  }
?>

Post # 5 will work with a slight change, though I don't know login_destination is big enough to make this part of thickbox.

<?php
function thickbox_form_alter($form_id, &$form) {
  if (
$form_id == 'user_login' && arg(0) == 'thickbox_login') {
    if (
module_exists('login_destination')) {
     
$form['#action'] = url('user/login', "destination=login_redirect");
    } else {
     
$form['#action'] = url('user/login', 'destination='. $_GET['destination']);
    }
   
$form['name']['#size'] = 25;
   
$form['pass']['#size'] = 25;
  }
}
?>

#10

paulnem - August 5, 2008 - 15:00

Any other thoughts around this issue? It's working for me but I'm having to patch with my own workaround each time I upgrade thickbox, which isn't ideal.

 
 

Drupal is a registered trademark of Dries Buytaert.