i18n_menu_block_view_alter overrides custom block titles

[herbdool]

If someone sets a custom block title on a menu block, then i18n_menu_block_view_alter() ignores that custom title and assumes the title is same as the menu name and then changes it to that. See https://github.com/backdrop-contrib/i18n/blob/1fa318c320ea5e480dc27e9d13a0b07eea0d5f52/i18n_menu/i18n_menu.module#LL95C54-L95C54

The function passes the textgroup and context for the menu title itself. For example, menu:main-menu:title. It should probably check the title somehow to see if the source title matches it.

UPDATE: It also overrides core when it sets the title with system_menu_block_set_title() which will set the title to be menu item's parent.

[herbdool]

I don't know if something like this makes sense:

/**
 * Implements hook_block_view().
 */
function i18n_menu_block_view_alter(&$data, $block) {
  if (($block->module == 'menu' || $block->module == 'system') && (i18n_menu_mode($block->delta) & I18N_MODE_MULTIPLE)) {
    $menus = menu_get_menus();
    if (isset($menus[$block->delta])) {
      if (!empty($data['title']) && $data['title'] == $menus[$block->delta]) {
        $data['title'] = i18n_string_plain(
          array('menu', 'menu', $block->delta, 'title'),
          $menus[$block->delta]
        );
      }
    }
  }
}

I added this condition: $data['title'] == $menus[$block->delta]. But I'm not sure if this hook is even needed. At this point the menu name was already translated. Perhaps it depends on what kind of translation is used : translation set or just locale translation? I haven't tried all the settings.

[indigoxela]

Many thanks for reporting. We'll need to check all circumstances, of course, to prevent regressions.

[indigoxela]

Weird enough... after some recherche and testing it appears to me, we can completely drop i18n_menu_block_view_alter() here.

Module i18n_block has been dropped in B, as this functionality went into core. If I also skip this hook, block titles do stay translatable - still have to look closer into details.

[indigoxela]

For sure this needs more testing, but I just provided a PR for discussion. 😏

Based on my findings, I dropped the (seemingly obsolete) block hook.

[herbdool]

I agree we can probably drop it. When I have a chance I'll test the pr with different settings.

[herbdool]

With this hook removed, then the only way (as far as I can see) to translate the menu block title is with the User Interface Translation:

But then at least it respects custom block titles.

It also means that this interface won't translate the block title, just the menu title for other contexts:

Maybe some help text on that page?

[indigoxela]

With this hook removed, then the only way (as far as I can see) to translate the menu block title is with the User Interface Translation

That might be because of the different concept in B re block translation. Blocks in general are pretty different (in Layout), so I'm not astounded, that things behave differently here.

The interface provided by i18n does translate the menu title itself, but the block title gets handled by core (Layout). It could be that we need some further adaptions - not sure, where these changes should go to.

[herbdool]

I think your PR is good enough for now since it does fix the main bug. And deal with any UI improvements later, if needed.

[indigoxela]

I'd like to - at least - add some help text, where this translation is relevant and what's not overridden (anymore). Otherwise it's too confusing. Especially, but not only for people coming from D7 - where this always translates the block title.

Struggling a bit with wording, though. @herbdool any idea how to phrase that?

[herbdool]

So on this tab: admin/structure/menu/manage/MYMENU/translate?

Maybe something like:

This provides a translation for the menu title to be used in various administrative contexts. However, if a menu title is being displayed for block in a layout, you will need to use the main User Interface Translation page (/admin/config/regional/translate) to find the menu block title and translate it. Blocks are part of the layout configuration and can have dynamic or custom titles set in the layout.

[indigoxela]

@herbdool to me it seems, like you're in the same boat... 😃 The description is technically correct, but too wordy.

[olafgrabienski]

A less wordy suggestion:

Provides a translation for the menu title to be used in administrative contexts. If a menu title is being displayed for block in a layout, translate it on the User interface translation page (/admin/config/regional/translate).

Linking to the translation page, instead of mentioning the path, would make the text even shorter.