User tests: Successful: Unsuccessful:
In Protostar, the link used for the logo header is always a direct link to the home page.
Code is
<a class="brand pull-left" href="<?php echo $this->baseurl; ?>/">
This means that we get an url of the type http://mysite.com/
In a multilanguage site it means the url will display the site default language or browser default language home page (depending on "Language Selection for new visitors") when sef is off
This patch, depending if sef is on or off, creates the good url.
In each case, clicking on the logo will display the site home page in the language in use.
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
There has got to be a way to do this with JRoute::_()
. Our routing API cannot be so bad that we manually piece together URLs.
Tested the new file index.php of the template Protostar template in a multilingual site (that had three languages en-GB, fr-FR, and ta-IN) with the following Global Configuration Options combinations:
(a) Search Engine Friendly URLs - No
(b) Search Engine Friendly URLs - Yes & Use URL Rewriting - No
(c) Search Engine Friendly URLs - Yes & Use URL Rewriting - Yes
In all the above combinations the link in the logo pointed to the URL of the appropriate language's Default menu item (i.e., Home Page of that language) in the proper way.
The URLs created for the language fr-FR are:
Case (a) => http://localhost/joomla_34_3_ml/index.php?lang=fr
Case (b) => http://localhost/joomla_34_3_ml/index.php/fr/
Case (c) => http://localhost/joomla_34_3_ml/fr/
The result was consistent across other languages (en-GB and ta-IN) as well.
@mbabker
I first indeed tested with JRoute
$logo_link = JRoute::_('index.php?lang=' . $this->sef . '&Itemid=' . $homemenu->id);
it works fine but the issue is that we get full long links when sef is off
http://mysite.com/index.php?option=com_content&view=featured&Itemid=153&lang=it
instead of the clean:
http://mysite.com/index.php?lang=it
Sometimes I think the only way to love our router is after the consumption of
Haven't tested, but shouldn't JRoute::_('index.php')
work? It should apply the language and itemid automatically. Or am I wrong?
As for the non-SEF URL, technically http://mysite.com/index.php?option=com_content&view=featured&Itemid=153&lang=it
is the correct link to the homepage. it's the same link that will be generated for the "home" menuitem.
@infograf768 I had some time to check your PR. As @Bakual said, JRoute is completely fine to solve this issue.
Just change the line 138
<a class="brand pull-left" href="<?php echo $this->baseurl; ?>/">
to
<a class="brand pull-left" href="<?php echo JRoute::_('index.php'); ?>">
Thanks!
Tested the code above from @Kubik-Rubik and it works correctly with all 3 sef settings on a multilingual site. Also tested on a monolingual site and it works there as well
The code suggested by @Kubik-Rubik brings/shows the URL of the current page viewed (in the browser). It does not show the Default Menu Item's (i.e., Home Page's) URL.
The logo's link is suppose to take the user to the Home Page - not the current page being viewed (unless it is the home page that is being viewed :-))
The original code change suggested by @infograf768 shows the URL of the Home Page as the Logo's link irrespective of the page being viewed.
Hope this test helps.
Category | ⇒ | Multilanguage |
@Bakual @Kubik-Rubik @brianteeman
Indeed, I should have added to the instructions that the test should also be done on a page which is not the home page.
<?php echo JRoute::_('index.php'); ?>
just reloads the current page.
Therefore, it is a matter of preference:
Do you prefer the long link or the short one?
whatever you do, the proposed code so far is definitely wrong. It will not work for sites that drop the default language SEF. We might indeed have to add the default menu itemid, but you don't need to add the language tag and especially not the SEF to the URL. Everything other than JRoute::_('index.php?Itemid='); is unacceptable.
It will not work for sites that drop the default language SEF.
Wrong. It works fine. I suggest you test for yourself.
Will test though JRoute::_('index.php?Itemid=');
It will work, because the language switcher jumps through hoops to get this stuff right, the router parses about every URL twice, etc. We had to introduce all that code in the SEF plugin and the language switcher to account for exactly such wrong code.
Tested with
// Get the correct logo link in multilingual
if (JLanguageMultilang::isEnabled())
{
$homemenu = $app->getMenu()->getDefault(JFactory::getLanguage()->getTag());
$logo_link = JRoute::_('index.php?Itemid=' . $homemenu->id);
}
else
{
$logo_link = $this->baseurl;
}
and further
<a class="brand pull-left" href="<?php echo $logo_link; ?>">
It works indeed. We get the long link when sef is off.
Would that be satisfactory to all here?
Oh, JRoute, how can you deceive me like this?! :-)
@Hackwar's suggestion works but then we have the ugly query (?Itemid=) in the URL... If we don't set the query, the option variable (=component) is set as var which will be also processed in the URL creation process.
@infograf768 Your last proposal is good and the long link if SEF is off is not a problem, this is the same behavior like we have in the language switcher module. Go for it!:-)
@Kubik-Rubik I have no idea what you are talking about...
@Kubik-Rubik
Not sure I got you.
My last proposal would be #7394 (comment)
Is that OK?
@infograf768 Yes.
@Hackwar I meant the check for an Itemid in the function createURI (libraries/cms/router/site.php) :-)
Milestone |
Added: |
Modified PR. Needs new tests. Thanks.
The newly proposed code works fine in both monolingual and multilingual sites.
Labels |
Added:
?
|
Thank you @infograf768! Merged.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-07-10 10:20:19 |
Closed_By | ⇒ | Kubik-Rubik |
@Kubik-Rubik where are the tests.
@brianteeman Set...
NO this is not right - the committer should not be a tester. You are the final check not part of the testing
@brianteeman Okay, then could you please also check this PR?
By the way, we also accept merging by "code review"... maybe we should also change this rule? :-)
Committer can be tester. It is the PR author that should (normally) not commit...
It's helpful if the person who commits wasn't one of the two testers; the committer should serve as a final layer of review and testing on a patch and not just merge because it has a label. This is why I typically don't commit things I was a tester for.
I'm sorry but I object to this PR and having it been merged
Same problem as in my language switcher code review: now, with SEF disabled, the logo link can become something like that:
/index.php?option=com_content&view=article&id=8&Itemid=102&lang=en-us
This is very bad for SEO: the home page could change in time and the indexed link will be no more valid.
The link obtained (what I called the long link) is the one we get now for the Home menu item.
It is true that it is equivalent to index.php?lang=xx-XX
But I remind you that this is exactly the same behavior when sef is off for a monolanguage site... i.e. without the lang code...
... If that is what is currently happening, the routing code is broken.
For crying out loud, Hannes, try it...
But I remind you that this is exactly the same behavior when sef is off for a monolanguage site...
Exactly: mono/multi-lingual doesn't matter here...
Actually not, and mono-lingual has his issues too, here (missing trailing slash)
Guys, with this patch having been merged, our (non-SEF) sites will not have a back-link to home page (/
), not even in the frigging logo... I'm afraid Google will have a bit of hard time figuring out which is the home page...
Labels |
Removed:
?
|
Thanks to Mig Manickam for pointing this issue