Instead of menu_get_names(), this module could use menu_get_menus/menu_list_system_menus (menu.module/menu.inc) to display human-readable names. At the same time, performance might be improved by using book_get_books() instead of parsing nids from book-toc-$nid and loading entire nodes.

Note also that #473240: menu_get_names() crashes when called. currently breaks the settings page of this module until the bug in core is fixed.

Comments

cburschka’s picture

Status: Active » Needs review
StatusFileSize
new14.93 KB

This patch does the following.

a) Privatize dhtml_menu_menus to _dhtml_menu_menus. There's no hook_menus, but as this function is internal, it should be named accordingly.
b) Use menu_get_menus (if menu.module is enabled), book_get_books (if book.module is enabled) and menu_list_system_menus (always) to build a human-readable list of menu names.
c) Add the code to filter these names through menu_get_names(), which will remove empty (unused) menus and add the internal names of any menu perhaps created by some other contrib module. This code is not called now (the function returns first) until #473240 is fixed.

cburschka’s picture

StatusFileSize
new2.65 KB

Didn't clean my local copy properly. Here is the patch without other changes.

cburschka’s picture

StatusFileSize
new2.6 KB

I asked around, and module_exists() is completely redundant here as far as performance is concerned.

cburschka’s picture

Status: Needs review » Fixed

Tested and read over repeatedly, and found nothing to improve (reviewing your own patches is tough). Committed to HEAD.

Status: Fixed » Closed (fixed)

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

cburschka’s picture

Status: Closed (fixed) » Active

#473240: menu_get_names() crashes when called. has long since been fixed. This workaround should now be reversed.

cburschka’s picture

Category: feature » task
Status: Active » Needs review
StatusFileSize
new945 bytes

Here's a patch.

cburschka’s picture

Status: Needs review » Fixed

Committed to HEAD and to DRUPAL-6--4.

Status: Fixed » Closed (fixed)

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