Per discussions in the development list. IMO, changing a menu weight (or expansion) is not really grounds for locking out hook_menu_alter from being able to do what my module needs to do. I put this in as a feature request because chx says this is the way it is supposed to work.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

NancyDru’s picture

BTW, this patch can be back ported to 6.x as-is. However, the damage to menu_links has already been done although an individual admin can easily undo it with "UPDATE `menu_links` SET customized = 0" (assuming there is no "real" customization in there).

Anonymous’s picture

Status: Active » Needs review
Issue tags: +DX (Developer Experience)

Well the patch needs a review.

chx’s picture

Status: Needs review » Needs work
Issue tags: -DX (Developer Experience)

Nancy, thanks for the patch. I think it's going in a good direction but comments go above the code line not besides. You are using some strange quotes, just use plain apostrophe. Also please check menu_edit_item too. You can change weight there too.

chx’s picture

Issue tags: +ux

Sorry, I have crossposted but this is indeed not developer experience. More like user experience.

NancyDru’s picture

Status: Needs work » Needs review
FileSize
1.82 KB

Status: Needs review » Needs work

The last submitted patch failed testing.

NancyDru’s picture

I have no idea why it should fail; the patch was created by CVS from a HEAD checkout.

NancyDru’s picture

Status: Needs work » Needs review
FileSize
1.84 KB

Status: Needs review » Needs work

The last submitted patch failed testing.

NancyDru’s picture

Are the tests, by any chance, flawed in this respect?

deviantintegral’s picture

It strikes me that modules/menu/menu.api.php should also be updated to contain documentation as to what constitutes a customized menu entry. The current docs make it seem like there are no restrictions, which I think caused much of the confusion in the first place.

NancyDru’s picture

It should also mention the ramifications of being "customized" - and that pretty much means that hook_menu_alter can no longer be relied on to make menu changes.

Anonymous’s picture

The tests didn't fail because it couldn't apply the patch, it failed during the 400+ tests it does for creating and deleting menu items.

Most of the API documentation resides in the body of the code. The API documentation that doesn't reside within the body of the code resides in one of the folders of http://cvs.drupal.org/viewvc.py/drupal/contributions/docs/developer/ so the proper way to change the documentation is to submit a patch.

Anonymous’s picture

And you may need to modify the tests to fit the new functioning of the API.

NancyDru’s picture

I have not gotten to where the tests make sense to me yet, so I can't do much about that.

themselves’s picture

Erm... Why doesn't hook_menu_alter change a customized menu item? That makes no sense at all. Drupal seems to be going out of its way to make life difficult for people who'd like to have dynamic menu titles. Surely it is quite reasonable to create a menu item with hook_menu and then want to change it later on with hook_menu_alter? Dynamic titles in hook_menu don't work (until you clear the cache), hook_menu_alter doesn't work if you've had the audacity to want to order your menu items... What happened to good ol' $may_cache from D5? That worked perfectly for this sort of situation!

NancyDru’s picture

Thanks for reminding me about this one. Now that I understand a bit about the testing now, maybe I can make this pass.

chx’s picture

What happened to this?

NancyDru’s picture

RealLife™ seems to have gotten in the way.

gagarine’s picture

Issue summary: View changes
Issue tags: -ux +Usability

Usability is preferred over UX, D7UX, etc. See https://www.drupal.org/issue-tags/topic

NancyDru’s picture

Assigned: NancyDru » Unassigned