The redirect
return $this->redirect('user.page', [], ['query' => drupal_get_destination()]);
in ubercart/uc_cart/src/Controller/CheckoutController.php on line 85 results in multiple redirections.

The correct redirect is
return $this->redirect('user.register', [], ['query' => drupal_get_destination()]);

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

mathiasgmeiner created an issue. See original summary.

mathiasgmeiner’s picture

Issue summary: View changes
TR’s picture

Category: Bug report » Task
Priority: Normal » Minor
Status: Active » Postponed (maintainer needs more info)

Anonymous users can be either users who are not logged in or they can be users who do not have an account on the site yet. user.register is the tab for creating new accounts, and we don't want that as the landing page for the redirect. Better would be user.login.

user.page, which we use now, is less specific than user.login but it does bring you to the same page.

mathiasgmeiner’s picture

But if the user is not logged in, this path results in multiple redirects and the browsers (Firefox in my case) throw an error.

TR’s picture

Status: Postponed (maintainer needs more info) » Active

Sorry I wasn't clear - I didn't mean we won't change it, but it really shouldn't be causing errors for you right now as it is - a redirect isn't really problem. user.page (/user) is always a valid route and should always bring you to the same page as user.login (/user/login) for an anonymous user. Throwing errors IS a problem, but I can't reproduce that in Firefox or Chrome - perhaps you're using some contributed module like logintoboggan that modifies the log in process?

mathiasgmeiner’s picture

Thanks for the clarification!

I have Email Registration installed, but uninstallation doesn't change the behaviour of the redirects.

With Email Registration:
Screencast #1

With Email Registration:
Screencast #2

It is the same thing in Chrome and Safari.

But you are absolutely right that user.login instead of user.register, it's way more user friendly!

TR’s picture

What happens if you try to visit /user on your site while you are anonymous?

mathiasgmeiner’s picture

Just /user is ok, everything is fine. But with /user?destination=/cart/checkout the multiple redirects happen.

keshav.k’s picture

Assigned: Unassigned » keshav.k
Status: Active » Needs review
FileSize
923 bytes

Same problem is happened.
When checkout as anonymous . the page isn't redirect to login. I will test it when url is "/user" , then working fine .
but when the url is with query string 'base_url/user?destination=/root/cart/checkout' then it will not working . and URL like 'base_url/user/login?destination=/root/cart/checkout' will working fine.
So i think changing in ubercart/uc_cart/src/Controller/CheckoutController.php on line 85 results in multiple redirections.
return $this->redirect('user.register', [], ['query' => drupal_get_destination()]);
Will be Great!

keshav.k’s picture

Status: Needs review » Needs work

The last submitted patch, 9: redirect_2789977_1.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 11: redirect_2789977_2.patch, failed testing.

keshav.k’s picture

keshav.k’s picture

Status: Needs work » Needs review
FileSize
907 bytes

Status: Needs review » Needs work

The last submitted patch, 15: redirect_2789977_3.patch, failed testing.

TR’s picture

Status: Needs work » Needs review

Testbot is working again.

Aporie’s picture

Another thing to investigate is when using pathauto changing the url for /user.

@mathiasgmeiner, Are you using pathauto to redirect the /user path ?

You can have a look here.

TR’s picture

@Aporie: If you use the patch in #15, does that fix the issue for you?

mathiasgmeiner’s picture

@aporie: no, I'm not using pathauto for a /user redirect. But I have not tested this patch! Thanks for reminding!

  • TR committed b8aca89 on 8.x-4.x authored by keshav.k
    Issue #2789977 by keshav.k, mathiasgmeiner: Wrong redirect path for...
TR’s picture

Status: Needs review » Fixed

I don't believe this has anything to do with Pathauto.

Ubercart redirects from /cart/checkout to /user?destination=/cart/checkout so that when the login or registration is complete the customer will be taken back to /cart/checkout.

Apparently, for anonymous users, core Drupal tries to redirect /user?destination=/cart/checkout to /user/login, but in the process reads the destination key and figures that you really want to go back to /cart/checkout. Then Ubercart sends the customer back to /user?destination=/cart/checkout, etc., resulting in a redirect loop.

This must be something with how Drupal 8 and/or Symfony treat the destination key, because this works as intended in Drupal 7 even though the same redirection is happening in both Ubercart and Core.

Anyway, redirecting to user.login (/user/login) instead of user.page (/user) seems to fix the issue, so I committed #15.

Aporie’s picture

Sorry for the delay,

Yeah I confirm that the patch fixed the issue.

Redirecting to user.login or user.register do the tricks.

Status: Fixed » Closed (fixed)

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