The menu system allows custom callbacks for menu item titles, with t() being the default.
Title_callbacks other than t() are passed insufficient information. Because - unlike code-based strings - admin-entered ones can be updated, we need a way to update existing locale source records, rather than creating new ones each time a change is made and thus orphaning existing records. An example is custom menu items as created via the menu module. To be able to update, we need a unique identifier for a menu item. But currently menu item title callbacks are passed only the string to be translated.
So it looks like a small change is needed: pass mlid as a second argument to title callbacks other than t().
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | menu-title-callback-id.patch | 3.3 KB | nedjo |
| #6 | menu-title-callback-id.patch | 3.62 KB | nedjo |
| #3 | menu-title-callback-id.patch | 746 bytes | nedjo |
| #1 | menu-title-callback-id.patch | 716 bytes | nedjo |
Comments
Comment #1
nedjoPatch.
One place this could be used is the i18nmenu module, which references menu source strings by mlid.
Comment #3
nedjoForgot to test for isset().
Comment #4
catchLooks very reasonable, probably small enough a change that we don't need a test for it, but I'm wondering if there's either somewhere in core we could use it or extend one of the existing menu tests to pick it up.
Comment #5
nedjoComment #6
nedjoHere's the patch with a test.
In the menu_test module we create a menu callback declaring a title callback but not options. This is needed to trigger a callback where the ID might be needed. (If there are callbacks, the menu system loads and feeds an object.)
In menu.test, we fetch the test page and determine if it has the title produced by the title callback, which includes the ID passed to the callback.
Comment #8
nedjoComment #9
nedjoRefreshed the patch after a conflicting change to menu tests. Updated the db query to used in the new test to DB api.
Comment #10
jose reyero commentedThis feature is badly needed. Still this leaves out the item description. Could we think of a 'localize' callback that is passed the full menu item?
Comment #11
nedjoGood question. It would be a large rewrite though of how menus are handled. Currently it's only the title that has a callback, and that callback is used in ways that may not apply to descriptions.
We *could* consider e.g. changing the title callback to return an array of title and description, or adding a description callback, or completely changing the approach. But meantime, I think it's worth fixing this issue in the current implementation.
Comment #13
stella commentedPatch passes all tests on a clean installation of HEAD on my local machine. May be related to #381004: locale test is failing
Cheers,
Stella
Comment #14
stella commentedTo summarise this patch:
It's a trivial enough change and works well, so I feel it's ready to be marked RTBC.
Cheers,
Stella
Comment #16
stella commentedComment #17
stella commentedRe-setting to RTBC now that testbot has passed the final patch again.
Comment #18
pwolanin commentedthis looks incomplete, and possibly just wrong. Are you counting on this for translation of user-entered links? You will never reach this code because of:
If nothing else, it's only going to work for the case where there are no title arguments specified.
I agree this is pretty trivial as a patch, but for D7 we should be looking for a more fundamental fix if this is an important issue.
Comment #19
muriqui commentedComment #26
lauriii_menu_item_localize has been removed in #2301319: MenuLinkNG part5: Remove dead code; and party!, and the way menu links get generated has changed significantly in Drupal 8, so I'm closing this as outdated. Thanks for all the contributors and sorry that we didn't get to fix this before this occurred.