Navigating to www.drigg.org/submit url (url used in bookmarklets) redirects user to www.drigg.org/user/register?destination=...
When existing user then clicks on "login" tab, the destination querystring is lost.

Site should redirect to www.drigg.org/user/login with destination querystring preserved, so that user can login and populate the submit form.

CommentFileSizeAuthor
#6 bookmarklet-fix-254092-6.patch2.38 KBsikjoy

Comments

mercmobily’s picture

mercmobily commented

Hi,

Please provide:

* A link to a bookmarklet with Drigg stories
* Exactly how to reproduce this, step by step, describing what happens and what _should_ happen

Be as precise as possible. Thanks!

Merc,

pileo’s picture

pileo commented
Title: Navigating to www.drig.org/submit redirects to "user/egister" page -- should be "user/login" page » Navigating to www.drig.org/submit redirects to "user/register" page -- should be "user/login" page

This bug repros without the bookmarklet code...it is just very prominent when using with a bookmarklet, since it requires user to click to "Login" and looses the destination querystring. Basic repro steps are listed below.

Repro steps:
1. Make sure user is not logged in at the drigg site (www.fxscoop.com for this example)
2. Navigate to http://www.fxscoop.com/submit?url=http://www.drigg.org&title=Drupal%20ba...

Results:
-User is redirected to http://www.fxscoop.com/user/register?destination=submit%253Furl%253Dhttp%25253A%25252F%25252Fwww.drigg.org%2526title%253DDrupal%252Bbased%252Bdigg%252Bclone%2526body...
-After user clicks "login" tab to enter credentials, the ?destination=.... querystring is lost, and after login user is redirected to default page

Expected Result:
- User should be redirected to http://www.fxscoop.com/user/login?destination=submit%253Furl%253Dhttp%25253A%25252F%25252Fwww.drigg.org%2526title%253DDrupal%252Bbased%252Bdigg%252Bclone%2526body...
-User should then enter credentials, and be redirected to "submit" page, with URL and TITLE fieild pre-populated with values from querystring

pileo’s picture

pileo commented
Title: Navigating to www.drig.org/submit redirects to "user/register" page -- should be "user/login" page » Navigating to www.drigg.org/submit when signed out redirects to "user/register" page -- should be "user/login" page
pileo’s picture

pileo commented

Link to bookmarklet can be found here: http://www.fxscoop.com/partner

mercmobily’s picture

mercmobily commented
Category: bug » feature

Hi,

This is not a bug, this is a feature request.
Basically, the Grigg administrator should be able to pick whether users should be directed to the SIGNUP page, or the "register" page.

The current behaviour is the most common one -- it's rare that people already have a drigg account, and if they have it, they aree probably already logged in and therefore don't need to register.

However, I think it's a good idea to implement this as a feature request.

Bye,

Merc.

sikjoy’s picture

sikjoy commented
Status: Active » Needs review
StatusFileSize
new bookmarklet-fix-254092-6.patch2.38 KB

Here is a patch written by cedricfontaine.

It stores the redirect address in the user's session variable. That way it does not get lost if the user needs to click the register tab.

Nice work Cedric!

Pileo, can you please test this patch?

--sicjoy

mercmobily’s picture

mercmobily commented

Hi,

Sicjoy, the percentage of people actually applying the patch and testing it is so small, I wouldn't count on it.
I would apply this patch. Since thanks to Cedric we are basically fixing the lot, I vote for actually applying the patches, and then tell everybody to test carefully Drigg in the next release before deploy. This is hardly gonna be a showstopper anyway...

Unless pileo is able to test it right away, please apply. That way, we can all see it on www.drigg.org anyway.

Bye,

Merc.

mercmobily’s picture

mercmobily commented

Hi,

Guys... how come we unset the cookie in function drigg_taxonomy_form_term() ...?!? :-D

No normal user will ever go there... that's an admin-only page!

Merc.

cedricfontaine’s picture

cedricfontaine commented
Assigned: Unassigned » cedricfontaine
Status: Needs review » Needs work

You're right... My error. Working on fix it.

cedricfontaine’s picture

cedricfontaine commented
Status: Needs work » Patch (to be ported)

You're wrong Merc ! We unset values in drigg_form. That's a bad reading form you of the patch...

function drigg_taxonomy_form_term() { appears in it cause we modify the function just before (drigg_form_alter($form_id, &$form) {)

Patch is ok and running ! Please apply to source code.

mercmobily’s picture

mercmobily commented
Status: Patch (to be ported) » Fixed

Hi,

Patch applied!!!

Merc.

sikjoy’s picture

sikjoy commented

In the future, to eliminate confusion, please use the -up switch with diff when creating patches. It adds the function name to the context of each change in the file.

I also like to include the module name, description, issue number, and comment number in the patch's filename. For example if I were adding a patch here I would use:

cvs diff -up > drigg-redirect_fix-254092-12.patch

Here is an awesome tutorial for creating patches for drupal.

--sicjoy

mercmobily’s picture

mercmobily commented
Status: Fixed » Closed (fixed)