Problem/Motivation

menu_tokens function changes the node object with menu_node_prepare() if it has not a menu variable.

<?php
   
if (!isset($node->menu)) {
      
// Nodes do not have their menu links loaded via menu_node_load().
      
menu_node_prepare($node);
    }
?>

A loaded node object (with node_load()) has not menu variable, even if the node has a menu item. menu_node_save() does nothing, if the node object has no menu variable, but if it has, it will create, change or delete a menu item. In this case it will delete the menu item, if it exists.

So if a module loads a node, prepare it, generate node tokens then save it, menu_tokens will remove the node's menu item. Like in this case: #1230034: Menu link disappears when publishing a node by the moderation dropdown or by the moderation view link

Proposed resolution

The menu_tokens() function needs an 'mlid', and I think, the best way to get it in this case is still the menu_node_prepare() function, so my patch just using a new variable for the mlid, and then unset the menu variable, so node object will be unchanged.

I attach also a patch just for the tests, this patch should fail.

Remaining tasks

Review needed.

User interface changes

none

API changes

none

Files: 
CommentFileSizeAuthor
#5 menu_tokens-1317926-5.patch1.16 KBpp
PASSED: [[SimpleTest]]: [MySQL] 296 pass(es).
[ View ]
#4 menu_tokens-test_only-1317926-4.patch721 bytesDésiré
FAILED: [[SimpleTest]]: [MySQL] 295 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
#4 menu_tokens-1317926-4.patch1.65 KBDésiré
PASSED: [[SimpleTest]]: [MySQL] 296 pass(es).
[ View ]
#3 menu_tokens-1317926-3.patch1.57 KBDésiré
PASSED: [[SimpleTest]]: [MySQL] 296 pass(es).
[ View ]
#2 menu_tokens-test_only-1317926-1.patch723 bytesDésiré
FAILED: [[SimpleTest]]: [MySQL] 284 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
#2 menu_tokens-1317926-1.patch1.59 KBDésiré
PASSED: [[SimpleTest]]: [MySQL] 285 pass(es).
[ View ]
menu_tokens.patch1.59 KBDésiré
PASSED: [[SimpleTest]]: [MySQL] 285 pass(es).
[ View ]
menu_tokens-test_only.patch723 bytesDésiré
FAILED: [[SimpleTest]]: [MySQL] 284 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

Comments

Désiré’s picture

Status:Active» Needs review
Désiré’s picture

StatusFileSize
new1.59 KB
PASSED: [[SimpleTest]]: [MySQL] 285 pass(es).
[ View ]
new723 bytes
FAILED: [[SimpleTest]]: [MySQL] 284 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

(reupload patches for testing)

Désiré’s picture

StatusFileSize
new1.57 KB
PASSED: [[SimpleTest]]: [MySQL] 296 pass(es).
[ View ]

better tests

Désiré’s picture

StatusFileSize
new1.65 KB
PASSED: [[SimpleTest]]: [MySQL] 296 pass(es).
[ View ]
new721 bytes
FAILED: [[SimpleTest]]: [MySQL] 295 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

After some consultation and code review I've made a new solution and better tests.
(test only patch should fail on tests)

pp’s picture

StatusFileSize
new1.16 KB
PASSED: [[SimpleTest]]: [MySQL] 296 pass(es).
[ View ]

I think it's enough to cloning the $node.

How reproduce this bug?

Désiré’s picture

OK, you're patch is cleaner. :)

How reproduce this bug?

It not so simple, because it occurs only if the node object will saved with node_save() after token generation (token_generate()), like in my test:

<?php
// Test [node:menu] tokens for a loaded node.
$loaded_node = node_load($node->nid);
$this->assertTokens('node', array('node' => $loaded_node), $tokens);
// Save the loaded node after token generation.
node_save($loaded_node);
?>

There is how was I reproduce it:
1 - create a node, check the 'Provide a menu link' option
2 - then run the following PHP code:

<?php
  $node
= node_load(1); // replace the correct nid
 
token_generate('node', array(), array('node' => $node));
 
node_save($node);
?>

3 - reload the page -> the menu item was deleted
(sorry I don't fount simpler reproduction method, but you can see, it's not impossible that a module need implements the steps above)

pp’s picture

Status:Needs review» Reviewed & tested by the community

ok, I think it is good enough to understand the problem.

Dave Reid’s picture

Status:Reviewed & tested by the community» Fixed

I added a couple comments and committed this to 7.x-1.x. Thanks Désiré and pp! http://drupalcode.org/project/token.git/commit/12eb465

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Frederic wbase’s picture

Super! I was waiting for this fix for a long time.

Thanks for your hard word :-)

Ciraxis’s picture

hm can't find the patch in token.tokens.inc or it is just out of date?

E: version 7.x-1.0-rc1 or 7.x-1.x-dev

Ciraxis’s picture

Issue summary:View changes

just some correction