Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
When you are on a node page (for example node/10) and you submit a comment and get a validation error, you are taken to a different url (for example comment/reply/50).
A lot of things rely on the path, and because you are on a node page but the path is comment/reply/[nid] things break.
For example, you no longer have some classes that were based on the path, and themes relying on this class will break.
Comment | File | Size | Author |
---|---|---|---|
#46 | 1609256-46.patch | 875 bytes | manish.upadhyay |
#42 | 1609256-42.patch | 849 bytes | florianmuellerCH |
#39 | 1609256-39.patch | 859 bytes | acbramley |
Comments
Comment #1
superstar3826 CreditAttribution: superstar3826 commentedDrupal 7
i have recreated this error
first i installed standard version
Created a blank page node with comments enabled
Submitted comment with blank comment body
result sent me to "http://localhost:8082/comment/reply/2"
expected results should have been "http://localhost:8082/node/reply/2"
Comment #2
BrockBoland CreditAttribution: BrockBoland commentedComment #3
BrockBoland CreditAttribution: BrockBoland commentedI tried this out locally to confirm the behavior and did some thinking about the issue. It's no longer a concern in 8.x because of a JS check for required fields: one cannot submit the comment form without the required fields filled.
This is bigger than just the comment error, though: the permalinks for individual comments are things like
comment/2#comment-2
. If changing this path, we would also change any other comment/x path, and to do so at this point in the D7 product cycle would be abnormal. However, we could consider making that change for D8 (though I don't know how tightly-coupled node and comment will be in D8: there's no reason that comments need to be restricted to nodes only and not used on other entity types, I would think).I think that a better solution might be to do some preprocessing to add a class (or classes) to the comment/reply page for themers who need something to hook onto there, to style it like the node page.
Comment #4
rooby CreditAttribution: rooby commentedThanks for the investigation.
What if the user is one of the evil ones that has javascript disabled?
Agreed, comments definitely should not be restricted to nodes.
That sounds like an alright solution for pre D8.
Comment #5
Ivan Zugec CreditAttribution: Ivan Zugec commentedI have created a test which'll check the URL when the comment form is submitted without any body text. The test will fail if you are not redirected back to node/NID. I'm also changing the issue version to 8.x-dev, because I was able to replicate the issue in both 7.x and 8.x.
Comment #6
gremy CreditAttribution: gremy commentedComment #7
gremy CreditAttribution: gremy commentedI am currently working on a patch for this feature. It's 67% done.
I need to mention that since this is a feature request it will not be backported to Drupal 7.
Comment #8
gremy CreditAttribution: gremy commentedThis is as far as I got with the patch for today.
I replaced the menu items for comment/... with node/$nid/comment/....
I tried to cover all comment/... urls and callbacks.
I also need to update the Comments tests, because with the comment urls changing, they will fail.
I did not test this patch in any way, so please consider this.
So, if you have time before I do, please review the patch as it is now.
Comment #9
gremy CreditAttribution: gremy commentedComment #10
gremy CreditAttribution: gremy commentedComment #12
droplet CreditAttribution: droplet commented#8: comments-change.patch queued for re-testing.
Comment #14
rooby CreditAttribution: rooby commentedThis also affects things like views that take a node id argument from the path.
I will try to make some time to look further this effort soon as I think it is very important to fix.
I'd say if you went looking you would find an enormous number of drupal sites that have many bugs on the comment/reply page.
I will also look into making a contrib module for D7 that tries to work around as many of the issues surrounding this as possible.
Comment #15
rooby CreditAttribution: rooby commentedHey I didn't change that.
Comment #16
rooby CreditAttribution: rooby commentedActually my comment about views default nid argument is not valid.
It does handle that but my particular site had another related issue.
Comment #17
tamer.kamel CreditAttribution: tamer.kamel commentedif you need to stop redirect to comment/reply/%node
you just need to comment line 1875 file comment.module
from
if (empty($comment->cid) && empty($comment->pid)) {
$form['#action'] = url('comment/reply/' . $comment->nid);
}
to
if (empty($comment->cid) && empty($comment->pid)) {
// $form['#action'] = url('comment/reply/' . $comment->nid);
}
Comment #18
doliveros CreditAttribution: doliveros commentedWhy not just manually set the form's ['#action'] to an empty string?
I got this working adding this line on a custom comment-wrapper.tpl.php file, placed in my theme directory :
$content['comment_form']['#action'] = ''; print render($content['comment_form']);
I guess it's also achievable with form_alter.
Comment #19
gremy CreditAttribution: gremy commentedI updated the tests so that they match the new menu changes.
There are still some tests that are failing on my machine, so can someone please help me out with this.
Comment #21
gremy CreditAttribution: gremy commentedI changed the patch name to match the convention [project_name]-[description]-[issue_number]-[comment_id].patch.
Comment #22
gremy CreditAttribution: gremy commentedComment #24
gremy CreditAttribution: gremy commentedFixed patch format
Comment #26
gremy CreditAttribution: gremy commentedMy patch replaces all
comment/...
paths withnode/[nid]/comment/...
.Do you think this might be a correct solution?
Comment #27
gremy CreditAttribution: gremy commentedI just took another look at the issue queue, and saw this task: https://drupal.org/node/731724
I think it makes a lot of sense to decouple the Comment entity from the Node, and thus I believe that it makes the current issue irrelevant, and I believe this issue should be closed.
Comment #28
star-szrLet's mark this postponed for now then.
Comment #28.0
star-szrChanging the comment reply url to be correct.
Comment #29
mgiffordComment #30
SegementOfAnOrange CreditAttribution: SegementOfAnOrange commentedRe: #18
This did the trick for me:
Comment #31
mukila CreditAttribution: mukila commentedRe #30
If you use this one, you will need to redirect the page otherwise if you refresh the page you will get same error
i don't know it is the standard method but i use this one.. it works fine for me
Comment #33
milos.kroulik CreditAttribution: milos.kroulik commentedRe #31 This works, but I had to change redirection target to make it work with clean URL. Also, filled in field values are deleted...
EDIT: Simply doing
was enough to make it work for me.
Comment #34
mgifford#731724: Convert comment settings into a field to make them work with CMI and non-node entities is fixed, so no need to postpone this any more from what I can tell.
Comment #39
acbramley CreditAttribution: acbramley at PreviousNext for Transport for NSW commentedThis bit us when using reCaptcha on comment forms. If the user doesn't validate captcha, they're redirected to the reply form. The reply form hides the comment form from the main page build so it results in a comment form at the bottom of the page, potentially in a different position which is bad UX. I'm interested to see what tests fail without the action being set in CommentForm as removing it does fix the issue however I assume it will affect threaded comments.
Comment #42
florianmuellerCHPorted the patch to be usable in Drupal 8.8.2 (different line numbers).
Comment #43
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 for Drupal India Association commentedComment #46
manish.upadhyay CreditAttribution: manish.upadhyay as a volunteer and at Publicis Sapient commentedPatch for 8.9 Drupal release.