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

Comments

mpaler’s picture

mpaler CreditAttribution: mpaler commented

subscribing

drupalina’s picture

drupalina CreditAttribution: drupalina commented

subscribing

it doesn't work with Drigg module either

rkn-dupe’s picture

rkn-dupe CreditAttribution: rkn-dupe commented

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?

Logi83’s picture

Logi83 CreditAttribution: Logi83 commented

Subscribing

vegeneric’s picture

vegeneric CreditAttribution: vegeneric commented

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;
  }
}
vegeneric’s picture

vegeneric CreditAttribution: vegeneric commented

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.

vegeneric’s picture

vegeneric CreditAttribution: vegeneric commented
Status: Active » Needs review
christefano’s picture

christefano CreditAttribution: christefano commented
Status: Needs review » 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

paulnem’s picture

paulnem CreditAttribution: paulnem commented

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 :

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.

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;
  }
}
paulnem’s picture

paulnem CreditAttribution: paulnem commented

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.

bejam’s picture

bejam CreditAttribution: bejam commented

Slight mod to get this working on drupal 6.

if (module_exists('login_destination')) {
$form['#action'] = url('user/login', array('query' => array('destination' => 'login_redirect')));
}
else {
$form['#action'] = url('user/login', array('query' => array('destination' => $_GET['destination'])));
}