Once a page has been created with a path and a normal menu item, modifying the menu item in anyway under Site building > Menus (including moving it) causes changes to the page path to disconnect the page from the menu item.

Steps to the reproduce the problem:

  1. Create a Page Manager page.
    1. Use "test" as title and machine name.
    2. Select Visible menu item as an optional feature.
    3. In the Menu settings, select Normal menu entry, enter "Test" as title, and select "Navigation" as menu.
    4. Use any normal settings for the rest of the creation process to finish creating the page (I used single column and everything else as is) and press "Save".
  2. Modify the menu item.
    1. Go to Site building > Menus.
    2. Find the item "Test" you created under "Navigation".
    3. Move the item to another position such that it is highlighted as changed.
    4. Save.
  3. Change the page path.
    1. Go back to the page editing interface for page "test".
    2. Go to the "Basic" settings form.
    3. Change the path to test2.
    4. Update and save.

Observed effect: the menu item has been inexplicably moved (often to the bottom). In fact, it is a new menu item. Moving the item around, saving it, and changing the page path back to "test" brings the menu item back to its old position.

Expected effect: the menu item under Menus > Navigation now points to the new path, and you can modify it further.

Workarounds: None, other than deleting the page and creating it again on its new path.

CommentFileSizeAuthor
#4 899066.patch1.54 KBzhangtaihao
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

This is an effect...possibly a bug...of the Drupal menu system.

Page Manager uses hook_menu() to provide an initial menu block. When you change this, this is an override. When a second item appears, that is very likely Drupal's built in menu system loses track of the item since it keys on path.

I'm not sure how possible it is to keep these values in sync.

zhangtaihao’s picture

zhangtaihao CreditAttribution: zhangtaihao commented

I'm working on the solution right now and almost have it.

Also, probably a stupid question, what function fetches a page from the database (possibly statically cached) in a similar fashion to node_load(), if such a function exists?

zhangtaihao’s picture

zhangtaihao CreditAttribution: zhangtaihao commented
Priority: Major » Normal

Never mind, I have it.

Just for reference though, is it page_manager_page_load()?

zhangtaihao’s picture

zhangtaihao CreditAttribution: zhangtaihao commented
FileSize
899066.patch1.54 KB

I also discovered the same issue with deleting: menu_rebuild refuses to delete the item with customized = 1.

I've managed to patch it and it works. However, I don't quite like it. The menu link deletion happens in page_manager_page_delete(), but the updating happens in page_manager_page_save_subtask() instead of page_manager_page_save(). The problem is that on "Update and save" (but not "Update" AND "Save", I think), querying {page_manager_pages} for the path given a page name somehow returns the new path as opposed to the old one. So, I've only managed to trap the old path from the subtask definition.

In a way, this patch makes logical sense. The function page_manager_page_save() saves to that table, leaving other concerns in the page_manager_page_save_subtask(). However, that doesn't account for the deletion. This probably requires a look into the "Update and save" workflow, which admittedly I'm ill-prepared to peruse. Where this patch goes from here, I will leave it in your capable hands, merlinofchaos.

N.B. As you can see in the patch, I've limited the actions to "customized = 1". I intended to let core handle the rest.

zhangtaihao’s picture

zhangtaihao CreditAttribution: zhangtaihao commented
Status: Active » Needs review
merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

Actually, this is one place you should not need to do a database lookup at all. Page Manager watches to see if the path changes.

In page_manager_save_page_cache there is already a flag, $cache->path_changed -- that lets us know we need to do a menu rebuild.

Now, that $cache isn't being sent down to page_manager_save_task_handler(), but it's being set in page_manager_page_form_basic_submit() which means that where it's set could also be set on the handler in a way of marking the handler so it can know to do something with the menu_links table.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Status: Needs review » Needs work

I think I should've set this needs work

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented

looks interesting. Will try to find some time to take a look.

nathan573’s picture

nathan573 CreditAttribution: nathan573 commented

This still seems to be an issue in Drupal 7 and caused me major problems. Is there a workaround other than re-creating the Page at the new path? My workaround was exporting the variant and then importing it into the new page that I created at the path that I needed it to be at - not a big deal but it was quite an ordeal to find out that things weren't as broken as they seemed at first. Thanks for your work on this complex, critical code.