Posted by mlncn on December 28, 2007 at 12:54pm
| Project: | Hidden |
| Version: | 5.x-2.0 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | ekes |
| Status: | closed (fixed) |
Issue Summary
After reporting content that should be hidden, a regular authenticated user is redirected to hidden/node/1976 (where 1976 is the node ID), which is access denied for non-admins (and page not found for admins).
The path I got redirected to: http://wsf.grassrootsconnection.org/es/hidden/node/1976
This is a multilingual site but I don't think that's introducing the error.
I think it should just redirect back to the reported node? I can make a patch but I am looking to make sure I have your intentions correct.
Comments
#1
Yes. If the node/comment is just being reported it should return to the page it came from with a drupal_set_message confirming that it has been reported.
If you have time to make a patch it would be great, if not I'll do it.
Cheers!
#2
The logic here appears reversed. This is the way it is now:
<?phpif ($access) {
$url = 'node/'. $nid;
if ($cid != 0) {
$fragment = 'comment-'. $cid;
}
}
else {
$url = "hidden/$type/$id";
}
?>
But it's people who don't have hidden access who should be directed back to the node, like so:
<?phpif ($access) {
$url = "hidden/$type/$id";
}
else {
$url = 'node/'. $nid;
if ($cid != 0) {
$fragment = 'comment-'. $cid;
}
}
?>
So the attached patch fixes that. However...
1. In most situations, people should be redirected back to the node regardless of their access.
2. I don't think hidden/$type/$id actually gets you anywhere-- there's no there, there. I think it's only part of a path
_hidden_goto_hidden is taking in too much for me to figure this out though.
#3
Yes the _hidden_goto_hidden was getting unnecessarily complicated dealing with (or trying to deal with) both hide and report redirects. I've simplified it using more of the drupal inbuilt goto functions. I've left in, but commented, the slightly complicated logic testing if cid===NULL, it is also redundant now - it was intended for times when you call but don't know the nid - I'll check to see if it is a future posibility.
I'll review the code when I've not got a horrid cold and related slow brain. But if you can test the patch that would be cool.
#4
OK a clear headed patch, with a mind on making redirects flexible in the future too
#5
Committed to DRUPAL-5--2 will be in next -dev created and I'll make a bug fix release as well soon.
#6
Awesome.
#7
(Apparently, hitting reload on a page like this, that's been open in one's browser window a couple days, will add new comments, but leave form values as they are. Setting back to fixed!)
#8
Fix in release 5.x-2.1