Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverted to previous behaviour when deleting links. #112

Closed
wants to merge 1 commit into from

Conversation

pikzen
Copy link

@pikzen pikzen commented Feb 15, 2015

OK, I've had a bit of feedback concerning #90. While this simplifies the
logic, when clearing out a bunch of tags or from a search, it's not really
practical.
What this does is that it checks if we're on a page that needs to redirect
to self. so ?searchtags does, but ?Eox6ze (a permalink) does not.
Also, thanks to some magic, HTTP_REFERER is always set and therefore we
don't need to grab the current URL if redirecting to self.
Tested on every page with a delete link

OK, I've had a bit of feedback concerning sebsauvage#90. While this simplifies the
logic, when clearing out a bunch of tags or from a search, it's not really
practical.
What this does is that it checks if we're on a page that needs to redirect
to self. so ?searchtags does, but ?Eox6ze (a permalink) does not.
Also, thanks to some magic, HTTP_REFERER is always set and therefore we
don't need to grab the current URL if redirecting to self.
Tested on every page with a delete link
}
}

header('Location: ' . $location); // After deleting the link, redirect to the home page.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is wrong, should be "redirect to appropriate location"

@nodiscc
Copy link
Member

nodiscc commented Feb 15, 2015

Everything is fine except the minor comment problem. The commit message should also be "redirect to previous search when deleting a link"

isset($_GET['searchtags'])) {

if (isset($_POST['returnurl']))
$location = $_POST['returnurl']; // Handle redirects given by the form
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use curly brackets here as well, as you're doing with the HTTP_REFERER case just below.

@e2jk
Copy link

e2jk commented Feb 17, 2015

Adding to @nodiscc 's comments (comment and commit message), please fix the brackets as well.
It would be great if the commit message would also include "Fixes #110" to have this documented well, and that issue closed automatically.

Thanks for your work.

@nodiscc
Copy link
Member

nodiscc commented Feb 17, 2015

Corrected in #114

@nodiscc nodiscc closed this Feb 17, 2015
@pikzen pikzen deleted the back-to-before branch February 19, 2015 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants