Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Comment module declares a path 'node/%node/%' which turns out to produce various artefacts, such as if you go to a path like 'node/66/xxx' where 66 is a valid nid. The node is displayed, but the tabs disappear. During earlier discussions with Karoly, I suggested making this path 'node/%node/comment/%'. On IRC he suggested rewriting comment module some so we wan use a load function and a path like 'node/%node/%comment'.
So, the first question is - are there ever any public-facing links like node/66/77 where 66 is a nid and 77 is a cid? If not, my suggestion is reasonable. This is similar to 'node/%node/edit' or 'node/%node/outline'.
Karoly's suggestion could also work, but would mean you can't pass params to a node page (perhaps a problem, for example, when embedding a view in a node). The reason for this is that you'd get a 'Not found' any time the second param is not a valid cid if comment module is enabled.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | path_despair_1.patch | 952 bytes | pwolanin |
Comments
Comment #1
pwolanin commentedComment #2
pwolanin commentedseems this path does not serve any function, since node module already passes any second argument on to comment_render. It should be removed.
Comment #3
chx commentedLet's investigate what happens when you remove this bothersome definiton and land on the path node/123/456. As node/123/456 and node/123/% is not defined, the next in order is node/123 which is defined, so you will call node_page_view(node_load(123), 456). So the page callback is the same. Let's examine the access callback -- we are checking whether a valid node is given but this is needless as the current _load handling call will check for you anyways... We also check for
&& $cid. This is pretty pointless, as the only way this can fail is that if you enter node/123/0 -- Drupal will show you the node and every comment and that's the same way it was in five. This was a pretty braindead conversion from me.The only functionality change is that node/123/456 does not show the node tabs with this definition. I can't fathom why that's good, it's very inconsistent.
Comment #4
chx commentedOpsie, cross posting. But yes the patch is good.
Comment #5
dries commentedCommitted to CVS HEAD. Thanks.
Comment #6
(not verified) commented