When making a new locale the default (and/or when editing strings in the locale and/or when importing new translations), the menu cache is not invalidated. This makes the menu stay stale...

Note that this is Goba speaking, but is unable to log in or create a patch. I consider this a serious usability issue.

CommentFileSizeAuthor
#6 book.module_menu_caching.patch3.08 KBpuregin
#4 locale_11.patch3.79 KBkilles@www.drop.org
#1 locale_10.patch878 byteskilles@www.drop.org

Comments

killes@www.drop.org’s picture

killes@www.drop.org commented
Assigned: Unassigned » killes@www.drop.org
Priority: Critical » Normal
StatusFileSize
new locale_10.patch878 bytes

Here is a patch.

dries’s picture

dries commented

Committed to HEAD. Shouldn't we do the same after a string has been translated through the web interface? A small code comment wouldn't hurt.

dries’s picture

dries commented

Another related bug: when a user changes his or her locale setting from the 'my account' page, his or her cached menu does not appear to be invalidated. The menus are in the previously selected language.

killes@www.drop.org’s picture

killes@www.drop.org commented
StatusFileSize
new locale_11.patch3.79 KB

The attached patch addresses all the mentioned issues and also converts a few " to '.

dries’s picture

dries commented

Committed to HEAD.

puregin’s picture

puregin commented
Component: menu system » book.module
Assigned: killes@www.drop.org » puregin
StatusFileSize
new book.module_menu_caching.patch3.08 KB

In book.module (4.5.0-rc) the book menu is not updated when changes to the structure of
a book occur and caching is enabled.

The attached patch calls cache_clear_all() and menu_rebuild() for each change which changes
book outlines.

Anonymous’s picture

(not verified) commented

The book menu is not generated using the menu system, but internally by book.module. Therefore, the unchanging results you're seeing are a result of page caching only and have nothing to do with menu caching. In your fix, you should just use a cache_clear_all (only)... no need for a menu_rebuild.

puregin’s picture

puregin commented

book_menu() is called with $may_cache, which if unset causes the book menu to not be
updated with changes to the book outline. I have tried removing the menu_rebuild() calls,
keeping only cache_clear_all(), and changes to books do not result in updates to the menu.
Am I missing something?

Anonymous’s picture

(not verified) commented

At first, I thought you were referring to the block generated by book.module as the "book menu." After looking at the code, I see that you're referring to the book administration pages and that they are indeed cached. Sorry... my mistake.

moshe weitzman’s picture

moshe weitzman commented

I think a menu_rebuild() is a reasonable solution. An alternative would be to move the definition of those menu items into !$may_cache but that will cause unnecessary queries on pages viewed by node admins. onte that menu_rebuild() does a cache_clear_all() so you don't need both of those calls.

i think that these book outline links should be defined as MENU_DYNAMIC_ITEM.

dries’s picture

dries commented

I did not apply the patch and moved the logic to the $may_cache clause and added some checks. I think that is the most performant behavior.

Anonymous’s picture

(not verified) commented
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Component: book.module » locale.module

There is still some issues with locale module and menu caching. If I change the default locale on the locale admin interface, the menu cache should be invalidated. The locale cache could probably be kept intact, since it is per language. But since the default locale change is done after the menu items are collected and even the page help is computed, to not require another submit or page refresh, Drupal should issue a redirect after invalidating the menu cache, so I get the new language view instantly after changing the default language.

Lacking experience in how the redirect wizardry is done currently, I am afraid I am unable to provide a patch, although this would be nice to fix before 4.5.1.

dries’s picture

dries commented

I committed a fix for this to both HEAD and DRUPAL-4-5. Thanks Goba. Marking this fixed.

Anonymous’s picture

(not verified) commented