atomium_preprocess_link() changes $variables['text'] to be an array, violating expectations of other modules.

[donquixote]

atomium_preprocess_link() changes $variables['text'] to be an array like ['#markup' => 'the title'].

Some other modules that don't know about atomium expect this value to be a string.

Example: special_menu_items uses hook_theme_registry_alter() to replace the usual theme_link(), and then calls check_plain() on $variables['text'].

I would recommend to not modify the value types for $variables[$key] of theme hooks.

These arrays are fragile enough on their own (because no static type checking).

[donquixote]

Also this slows down theme('link', ..) because it involves an additional render() call in the template.

[donquixote]

If people need a link with a render element inside, they can use e.g. themekit_link_wrapper provided by themekit module.

This is a different theme hook, so no expectations are violated.

[donquixote]

Remember that theme('link', ..) is called within l(), except in some special case that was added there for optimization, see https://www.drupal.org/project/drupal/issues/318636. So this is really costly.

[drupol]

Hello,

I agree that this would be problematic if we would do that in a module. However, theme hooks are last in the stack so, I guess we can should just ignore this.

However, if phase callbacks are added through hook_theme_registry_alter(), then we might have issues if the are added AFTER the theme callbacks.

[donquixote]

We also get problems if

• A module implements hook_process_link(), which comes after hook_preprocess_link()
• A module replaces the theme function for 'link', like special_menu_items module does. (*)

(*) Imo special_menu_items should not do this. But neither should atomium :)