The recent CVS commit cleaning up the node_page() function broke the new ability to have modules define multiple node types. Specifically, when adding new nodes, arg(3) is not passed on to node_add(), so modules cannot differentiate between their multiple types. Patch attached.

CommentFileSizeAuthor
#8 node_2.diff1.44 KBjonbob
#4 node_0.diff792 bytesjonbob
#3 forum-module-fix.patch482 bytesTDobes
node.diff633 bytesjonbob

Comments

dries’s picture

dries commented

Committed to HEAD. Thanks.

calanya’s picture

calanya commented
Title: Recent node_page() cleanup breaks multiple content types » It broke forum module for me

If I try to create a new forum topic from a forum page (for example mysite.com/forum/5), I get to this page: mysite.com/node/add/forum/5. The patch you applied makes node module think the new node's type is "forum/5", when in fact it should be "forum".

TDobes’s picture

TDobes commented
Title: It broke forum module for me » forum.module posts no longer appear in forum
Component: node.module » forum.module
Assigned: Unassigned » TDobes
Priority: Normal » Critical
StatusFileSize
new forum-module-fix.patch482 bytes

As calanya noted, the multiple node type fix introduces a bug in forum.module causing it to no longer display forum nodes in the forum because they are assigned the wrong type. (Compare behavior with node.module 1.325 vs. 1.326) This more-or-less breaks forum.module, so I'm setting the priority to critical.

The attached patch fixes the problem by resetting a node type of 'forum' from within forum_form(). I have tested it, and it seems to work consistently (for forum.module anyway... some contrib modules might have issues as well, though)

Can someone more familiar with the multiple-node-type-per-module functionality please look at this patch and advise as to whether this is the best way to solve the problem?

jonbob’s picture

jonbob commented
Assigned: TDobes » jonbob
StatusFileSize
new node_0.diff792 bytes

I suppose that would be me. :-)

To clarify the problem, it seems that forum.module relies on arg(3) to make new node add forms for a particular taxonomy selection. The node_pager() code sees this as a new content type instead.

The proposed method should work fine, but as stated would only fix forum.module, not other modules which may play with other arguments in node/add. As an alternate, the attached patch changes the behavior of node_add to check whether a node type exists before using arg(2)/arg(3) as a node type specifier. If not, it falls back on arg(2).

Steven’s picture

Steven commented

Wouldn't a better solution be to use a different separator than / to separate module name from subtype? We treat the entire node-type as a single string throughout Drupal. By using a slash, we are splitting it up, because URL arguments behave as separate variables.

I don't think using module.subtype would be any dirtier in the URL.

dries’s picture

dries commented

I don't like using a . (point) but I'd be OK with a - (dash) ...

dries’s picture

dries commented

I think we are best of using a dash (-) to keeps the code simple and to avoid unexpected problems.

jonbob’s picture

jonbob commented
StatusFileSize
new node_2.diff1.44 KB

The attached patch changes the multiple type delimiter from / to -. Modules that use multiple content types will have to be updated. I will update hook documentation in contrib.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: forum.module posts no longer appear in forum » Multiple node types with a dash (and not a slash)

Change title to reflect current state of the issue (and bring it to attention).

dries’s picture

dries commented

Committed to HEAD. Thanks.

jonbob’s picture

jonbob commented

Tested for a while; seems fine.