The API documentation for menu_link_save() could be improved to add the information that it does not play well with path aliases. It is very tricky because when creating a menu link on node creation (programmatically) it accepts aliases, but it does not accept using menu_link_save(). Then, this is a valuable information for developers.

Moreover, the list of elements in the $item array is not complete in the documentation. In order to add a translatable item it must contain 'language' and 'customized' => 1.

#5 fix-menu_link_save-documentation-1922896-5.patch1.42 KBGoddamnNoise
#3 fix-menu_link_save-documentation-1922896-3.patch1.42 KBGoddamnNoise
PASSED: [[SimpleTest]]: [MySQL] 40,750 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more


jhodgdon’s picture

Issue tags: +Novice

OK... Regarding aliases, the documentation already says:

"The path of the menu item, which should be normalized first by calling drupal_get_normal_path() on it."

And if you look at drupal_get_normal_path(), that is the function that translates an alias to a real path (which is what "normalize" means in this context). So... I believe this is already documented, but maybe the term "normalized" is confusing, and we should make it say:

The real (not aliased) path of the menu item, which should be normalized first by calling drupal_get_normal_path() on it.

Regarding language and customized, those sound like good additions to the documentation.

I guess this doesn't apply to Drupal 8, since it uses a class as the argument and presumably all of this will be taken care of by the class.

GoddamnNoise’s picture

Assigned: Unassigned » GoddamnNoise
Issue summary: View changes

I'll try to fix it!.

GoddamnNoise’s picture

Assigned: GoddamnNoise » Unassigned
Status: Active » Needs review
1.42 KB
PASSED: [[SimpleTest]]: [MySQL] 40,750 pass(es). View

Let me know if it is ok or you see any way to improve it.

jhodgdon’s picture

Status: Needs review » Postponed (maintainer needs more info)

Pretty close, thanks! A couple of things to fix:

+ *   - language: (optional) The language code (e.g. 'en'). If yout want to make
+ *     the item translatable, then it is required to set the language code and
+ *     you must set customized to true.
+ *   - customized: (optional) Boolean that must be true in order to not display 
+ *     the link in any language. 

- First line: "yout" -- oops!
- In docs, when referring to TRUE and FALSE, they should be in ALL CAPS.

So... I am not sure about the documentation of -customized either. The issue report here asserts that you need to set customized to TRUE in order to have a translatable menu item, but I do not know if that is true or not. Where is the code that sets that up, so we can verify? It seems a bit odd, and presumably if your module set up a menu item and didn't set it to be "customized", then it would have put text in for the menu link using t() so that text would be translatable? I just don't know if I believe this necessarily, and it's not obvious from the code in that this would be the case.

And assuming this is correct, I think the wording in the patch for 'customized' is confusing...

Anyway, we need more information before we can proceed with this. Sorry for not noticing that earlier!

GoddamnNoise’s picture

I've fixed the things you pointed out, but I'm sorry I can't help with the "customized" issue. I don't have enough knowledge about Drupal core to know that. Just let me know if i can help you with that.

jhodgdon’s picture

Yeah, I don't know either. Maybe the original reporter of this issue can explain why?