Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#13411 closed (fixed)

Some improvement for query string built by utils.next_redirect if next parameter contains '#' anchor

Reported by: timesong Owned by: nobody
Component: contrib.comments Version: 1.1
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In the comments app, views.utils.next_redirect appends a '?' and query string key/value pairs to the '#comment', so for example in template add '<input type="hidden" name="next" value="{{ article.get_absolute_url }}#comment" /></td>' will get 'http://www.example.com/articles/2#comment?c=7', it's incorrect.

Attachments (4)

patch01.patch (704 bytes) - added by timesong 6 years ago.
comment-with-query-string-and-anchor-with-unittests.diff (1.8 KB) - added by David Novakovic 6 years ago.
comment-with-query-string-and-anchor-with-unittests-doc.diff (1.8 KB) - added by David Novakovic 6 years ago.
13411_comment_redirect_with_anchor.diff (2.4 KB) - added by Julien Phalip 6 years ago.

Download all attachments as: .zip

Change History (12)

Changed 6 years ago by timesong

Attachment: patch01.patch added

comment:1 Changed 6 years ago by Russell Keith-Magee

Has patch: set
milestone: 1.2
Needs documentation: unset
Needs tests: set
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

Not critical for 1.2.

Changed 6 years ago by David Novakovic

comment:2 Changed 6 years ago by David Novakovic

Patch now diffed against root of project and contains a test.

comment:3 Changed 6 years ago by David Novakovic

Fixed the docstring.

use the -doc patch.

comment:4 Changed 6 years ago by jonasnockert

Should/could this be marked as milestone 1.4?

For what it's worth, there's a little more information on Stack Overflow.

Changed 6 years ago by Julien Phalip

comment:5 Changed 6 years ago by Julien Phalip

milestone: 1.3
Needs tests: unset
Triage Stage: AcceptedReady for checkin

I've actually gotten bitten by this in one of my current projects. The patch looks good. I've just updated the test to make sure it works both with and without a query string.

comment:6 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

In [15720]:

Fixed #13411 -- Made sure URL fragments are correctly handled by the next_redirect utility of the comments apps. Thanks, timesong, dpn and Julien Phalip.

comment:7 Changed 6 years ago by Jannis Leidel

In [15721]:

[1.2.X] Fixed #13411 -- Made sure URL fragments are correctly handled by the next_redirect utility of the comments apps. Thanks, timesong, dpn and Julien Phalip.

Backport from trunk (r15720).

comment:8 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top