Skip to content
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

Multilingual: correcting alternates when associated menu items are com_users items #13152

Merged
merged 3 commits into from
Dec 11, 2016

Conversation

infograf768
Copy link
Member

@infograf768 infograf768 commented Dec 10, 2016

Issue: #13142

See description there.

Summary of Changes

Adding a special case when associating com_users menu items (login. logout, password reset, reminder, registration, etc.)
EDIT: as we only need the itemid and lang, simplified code.

Testing Instructions

Install a multilingual site with associations.
Make sure Add Alternate Meta Tags is set to Yes in the languagefilter parameter.
Set SEF to on.
Create various menu items with different aliases using com_users menu item in each language and associate them.
Display these menu items in frontend.
Check the source of the page.

Before patch, the alternate will always display the same url in the alternate for all languages.
(I have also set here one of the login menu item as child of another different menu item)

screen shot 2016-12-10 at 08 33 51

After patch
Now the alternate are correct

screen shot 2016-12-10 at 08 33 05

Documentation Changes Required

None. This is solving a bug due, I guess, to the com_users router

@ioweb-gr @mbabker @andrepereiradasilva

Thank you for testing.

@gwsdesk
Copy link

gwsdesk commented Dec 10, 2016

Trying to understand the testing instructions. Don't sure that I understand them so is here a more 'practical' explanation so me dumb blond can understand it and test? I simply do not know what to look for at present guidelines. Thanks JM

@@ -774,7 +774,7 @@ public function onAfterDispatch()
// Heads up! "$item = $menu" here below is an assignment, *NOT* comparison
case (isset($associations[$i]) && ($item = $menu->getItem($associations[$i]))):

$language->link = JRoute::_($item->link . '&Itemid=' . $item->id . '&lang=' . $language->sef);
$language->link = JRoute::_('index.php?&Itemid=' . $item->id . '&lang=' . $language->sef);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just index.php?Itemid= (ie, remove the '&')

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops.. done

@infograf768
Copy link
Member Author

infograf768 commented Dec 10, 2016

@gwsdesk Forgot to say that SEF should be on to test this.
Also the menu items alias should be different depending on language.

@AlexRed
Copy link
Contributor

AlexRed commented Dec 10, 2016

I have tested this item ✅ successfully on 9a67290

Patch ok.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13152.

@gwsdesk
Copy link

gwsdesk commented Dec 10, 2016

I have tested this item ✅ successfully on 9a67290

"Also the menu items alias should be different depending on language" Did it for me...sorry for being blond.

Thanks for clarifying. Works4me JM as well now.

Thanks for the patch !


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13152.

@roland-d
Copy link
Contributor

RTC as we have 2 successful tests


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13152.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Dec 10, 2016
@zero-24 zero-24 added this to the Joomla 3.7.0 milestone Dec 10, 2016
@rdeutz rdeutz merged commit 8e20e66 into joomla:staging Dec 11, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 11, 2016
@infograf768 infograf768 deleted the com_users_alternate branch December 11, 2016 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants