-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[3.9] Making the alias menu item multilingual aware (for core menu module only) #22579
Conversation
modules/mod_menu/helper.php
Outdated
// Use language code if not set to ALL | ||
if ($language && $language !== '*') | ||
{ | ||
$item->flink = $item->flink . '&lang=' . $language; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$newItem
sometimes may not exists (NULL).
.=
would be better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t get it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to $item->flink .= '&lang=' . $language;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If $newItem
will be NULL, then you get PHP Warning. It may be when the target item has been deleted, but the alias still exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add if ($newItem != null)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or better if ($newItem != null && $newItem->language && $newItem->language !== '*')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or if (!empty($newItem->language) && $newItem->language !== '*')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I will leave it now as it is. ;)
I have tested this item ✅ successfully on 8704980 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22579. |
1 similar comment
I have tested this item ✅ successfully on 8704980 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22579. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22579. |
What does happen? http://prntscr.com/lg8r9y |
Pull Request for Issue #22515
Thanks @csthomas for the code to not use the db query
Summary of Changes
Adding
$lang
parameter to the target menu item when site is multilingualTesting Instructions
Create a multilingual site. 2 languages are enough.
Use multiple menu items in both languages
Among them create an Alias menu item to a menu item tagged to the other language.
Click on the Alias menu item in frontend.
Before patch
The menu item may display but with the wrong language interface
After patch
Issue solved. One is redirected to the menu item in the other language.
Warning
This will NOT work when not using the core menu module.
It does not break 3rd party menu solutions but they will lack this improvement.
@B3nito
@HLeithner
@csthomas