?
avatar smz
smz
12 Jul 2015

Description

Since when #7394 has been merged, we have what I consider a regression in the links associated to the logo in the Protostar template

Steps to reproduce the issue

  • Multilingual site
  • Default language = en-GB
  • SEF Off
  • Visit home page (assuming it is a "Single Article")
  • Examine the link in the logo image

Expected result

Well... this can be debated, but my expectation are one of the two (in the preferred order)

  • / (or baseurl + /) (the language cookie will handle redirection to current language)
  • /index.php?lang=en

Actual result

Something like this:
/index.php?option=com_content&view=article&id=8&Itemid=102&lang=en

Additional comments

As already commented in #7394, multilingual sites will not have a back-link to home page (/), not even in the logo and I'm afraid Google (and other crawlers / site mappers) will have a bit of hard time figuring out which is the home page.

Note that the declared intent of #7394 was to have links of the /index.php?lang=en type (see: #7394 (comment)) with which I disagree for the above SEO related reasons, but would anyway be better than the "fully specified URL" we have now: the published article in the home page could change in time and the link found in content cached by Google or elsewhere will be invalid.

avatar smz smz - open - 12 Jul 2015
avatar smz smz - change - 12 Jul 2015
Title
[Regerssion] Less than optimal URLs in Protostar's Logo
[Regression] Less than optimal URLs in Protostar's Logo
avatar smz smz - change - 12 Jul 2015
Title
[Regerssion] Less than optimal URLs in Protostar's Logo
[Regression] Less than optimal URLs in Protostar's Logo
avatar smz smz - change - 12 Jul 2015
The description was changed
avatar smz smz - change - 12 Jul 2015
The description was changed
avatar smz
smz - comment - 12 Jul 2015

One further comment: having the language code in the Logo link can be a good thing if we take the precaution of having the link for home page of default language to be /, plain and simple.

The reason why I say it could be a good thing is that in that case we could finally have multilingual sites that don't need the bloody language cookie. This is something for which I've been beated to death in the past, but I'm still convinced is totally feasible. Of course every time you'll visit the site anew you'll be presented the default language or the browser detected language.

avatar smz
smz - comment - 12 Jul 2015

There is also a mistake for non multilingual sites: the link will be (typically) empty as it is set to $this->baseurl, without the trailing slash.


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

avatar infograf768
infograf768 - comment - 12 Jul 2015

There is also a mistake for non multilingual sites: the link will be (typically) empty as it is set to $this->baseurl, without the trailing slash.

True for that part.
It should be
$logo_link = $this->baseurl . '/';
My mistake.

Not sure for the rest.

avatar infograf768
infograf768 - comment - 12 Jul 2015

Made a PR for the absent slash. #7418
Let's merge it and keep discussing the more general issue of the long link vs the short link for the Homepages.

avatar Bakual
Bakual - comment - 12 Jul 2015

The link in the logo should either be the domain name (without any index.php or language codes), or the same link that is created for the home menu item (the long one for non-SEF you currently see). Everything else makes no sense imho.

avatar infograf768
infograf768 - comment - 12 Jul 2015

@Bakual
@smz has a point though.
Can't we manage for JRoute to check if the url corresponds to a Home page and, in that case, simplify it when sef is off to avoid providing different urls for the same page?
It may not be worth as it is more and more considered that non-sef should disapear from future release, but until thenĀ ...

avatar Kubik-Rubik
Kubik-Rubik - comment - 12 Jul 2015

It is the same behavior as in the language switcher module, so it's actually not a "bug" because it works since many versions already. But you are right, it is not really the best possible solution...

I would not like to implement workarounds into JRoute at the moment, we will tackle the routing and also Hannes' contribution soon.

As only non-sef URLs are concerned, my proposal would be to use the base URL in the non-sef mode like we did before the change:

Change line 81 from

if (JLanguageMultilang::isEnabled())

to

if (JLanguageMultilang::isEnabled() && JFactory::getConfig()->get('sef'))

If this solution is feasible, could you please create a PR for it @infograf768? Thanks!

avatar smz
smz - comment - 12 Jul 2015

I have some questions:

  • Why this change? Haven't the plain / served us honourably for years?
  • Why only for multilingual?
  • Why only in Protostar and not in Beez3?
  • Are all template designers supposed to incorporate such code in their templates?
  • Why the link in the logo should match the entry in the menu system and/or the language switcher? Don't we have rel=canonical for handling such discrepancies?

BTW, having the domain name in that link would be a big mistake that could break many sites.

avatar Bakual
Bakual - comment - 12 Jul 2015

I don't see the point in trying to create "nice" or "short" URLs for non-SEF mode.
It's the whole point of SEF mode to have such URLs.

BTW, having the domain name in that link would be a big mistake that could break many sites.

I may have worded it wrong. I meant just having the base URL without any added query parameters. Be it www.example.com, /, /joomla or whatever. But no index.php and lang or itemid parameter.

avatar smz
smz - comment - 12 Jul 2015

@Bakual

I don't see the point in trying to create "nice" or "short" URLs for non-SEF mode.

I already explained that: the URL can change in time.

... Be it www.example.com, /, /joomla or whatever. But no index.php and lang or itemid parameter.

Anything containing the domain name or host name would be wrong. Which leave us with just / or /baseurl/. My point in case! :smile:

avatar smz
smz - comment - 12 Jul 2015

@Kubik-Rubik

I would not like to implement workarounds into JRoute at the moment ...

According to @Hackwar a call to JRoute::_('index.php?itemid=' . $home_page_id) should return (depending on actual configuration, i.e. multilingual or not, SEF mode or not) either:

  • /
  • /sef/
  • /index.php
  • /index.php&lang=sef

... and himself stated that if this is not the case than "... the routing code is broken".
He and others have also stated several times in the past that we should never need to differentiate between SEF mode or not.

So, if this is the case, the only way to solve this issue would be to fix the router and have just JRoute::_('index.php?itemid=' . $home_page_id) as the logo link (which IMHO is way better acceptable code to put in every template index.php).

But even that will leave us without a back-link to / for multilingual sites where the "Remove default language code" option is not set, and this is a major failure, IMHO.

avatar Kubik-Rubik
Kubik-Rubik - comment - 12 Jul 2015

@smz Yes, I'm also fine with this solution. Can you have a look whether an easy fix in the router is possible?

avatar smz
smz - comment - 12 Jul 2015

absolutely not! I'm not up to the task, really! :smile:

I've looked into that but couldn't really understand where to put my hands without breaking anything!!

And my objection of the lack of a back-link to / still holds true.

avatar Fedik
Fedik - comment - 12 Jul 2015

I agree with @Bakual , currently it the same behavior as for the home menu item, so I see no problem ...

But, I have a dream that someday joomla can recognize short links like index.php?Itemid=XXX when SEF off :smile:

avatar Hackwar
Hackwar - comment - 12 Jul 2015

It does understand index.php?Itemid=XXX. It was an intentional decision some LONG time ago to generate long URLs for non-SEF URLs based on the Itemid.

avatar smz
smz - comment - 12 Jul 2015

@Hackwar

It does understand index.php?Itemid=XXX. It was an intentional decision some LONG time ago to generate long URLs for non-SEF URLs based on the Itemid.

Then what we should use for a clean link to home page (both mono-lingual and multilingual)?

avatar Fedik
Fedik - comment - 12 Jul 2015

@Hackwar oh, you are right, sorry! ... how I missed this !? :smile:

avatar smz
smz - comment - 12 Jul 2015

@Hackwar

In https://github.com/joomla/joomla-cms/pull/7286/files#r34333995 you suggested that:

JRoute::_('Index.php?lang=' . $language->lang_code) is the correct code

but that returns link to current page, not home page...

But wait... couldn't be the case that the correct code is JRoute::_('Index.php?lang=' . $language->sef)? Let me try that...

avatar smz
smz - comment - 12 Jul 2015

Bingo! This is it: JRoute::_('index.php?lang=' . $language->sef), or at least so it seems at first sight...

avatar smz
smz - comment - 12 Jul 2015

Forget it: I tested wrong code, the "hardcoded" '/index.php?lang=' . $language->sef; I have in my language switcher PR, without the JRoute stuff. That's why it was working.... :smile:

Sorry...

avatar Bakual
Bakual - comment - 12 Jul 2015

I still fail to see the issue you try to solve here.

You're talking about non-SEF URLs and care about how crawlers index them. Right?

Keep in mind that SEF (Search Engine Friendly) URLs are meant to take care of that.
There is no point trying to get non-SEF URLs work fine for search engines when all you need to fix this is enable SEF URLs. That's why we have them.

avatar mbabker
mbabker - comment - 12 Jul 2015

By that argument you would say that Joomla should not support non-SEF URLs. Sadly, there are sites deployed to production that use them, so their behavior MUST be accounted for until support for them is otherwise removed.

avatar smz
smz - comment - 12 Jul 2015

By that argument you would say that Joomla should not support non-SEF URLs. Sadly, there are sites deployed to production that use them, so their behavior MUST be accounted for until support for them is otherwise removed.

This! :+1:

I still fail to see the issue you try to solve here.

TBH, I fail to see the issue solved by #7394

... and nobody has yet answered to my questions above: #7417 (comment)

avatar Kubik-Rubik
Kubik-Rubik - comment - 12 Jul 2015

By that argument you would say that Joomla should not support non-SEF URLs. Sadly, there are sites deployed to production that use them, so their behavior MUST be accounted for until support for them is otherwise removed.

Yes, the merged PR has not changed it and those sites are still accounted as usual. We just have the same behavior as in the menu and language switcher modules, so I agree with @Bakual and @Fedik.

To make everybody happy again, please make a PR with my suggested change in #7417 (comment).

avatar mbabker
mbabker - comment - 12 Jul 2015

We should not need to check the site configuration to determine if SEF URLs are enabled unless we are in code that deals with URL building or parsing. Anything outside that really should care less about the SEF config. IMO your suggestion, while it would work, is just yet another bandage on top of a system that needs stitches to stop its bleeding.

avatar Kubik-Rubik
Kubik-Rubik - comment - 12 Jul 2015

@mbabker Yes, this is right. The best solution would be a change directly in the router. Well, I will take a look when I have more time again. My proposal was meant as a temporary solution until we have improved the routing in general.

avatar Bakual
Bakual - comment - 12 Jul 2015

TBH, I fail to see the issue solved by #7394

It's written there in the PR. If you clicked the link in a multilang site under some circumstances you would loose the selected language and get the default language instead.
The fix is correct to create a link to the proper home menu item for the active language.
The link is the same that is in the home menu item for that language.

So I still don't see the issue. There is no additional URL for the same page as the same link already exists for the home menu item. So Google will have no issue at all.
Also this only applied to the Protostar template.

The best solution would be a change directly in the router.

For non-SEF URLs there isn't much done in the router as far as I know. I wouldn't start changing things there.

avatar Bakual
Bakual - comment - 12 Jul 2015

Btw: Google doesn't determine the homepage based on a link in the logo. It does it by requesting the / and sees what comes. Like any human does. It absolutely doesn't matter for them what link is in the logo. That link will be indexed like any other and Google will detect that it's the same link as the homepage, ignoring it then. I would be very surprised if Google isn't smart enough to handle this.

avatar smz
smz - comment - 12 Jul 2015

... If you clicked the link in a multilang site under some circumstances you would loose the selected language and get the default language instead. ...

I may be blind, but I don't see that written in the PR. What I see is:

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

but what is lacking from the sentence above is: ... if a language cookie is not set

So again, sorry, I may be dumb, but I don't see the problem.

And if there actually is a problem, shouldn't we advice all template developers to fix their templates?

avatar smz
smz - comment - 12 Jul 2015

Btw: Google doesn't determine the homepage based on a link in the logo.

You sure? Don't you think that with all his heuristics Google is not giving a particular weight to a link in an element with class "site-title", at the top of the page?

But once again, the main issue is that the URL could change in time and thus what Google indexed will be no more valid.

Please answer to that.

avatar Bakual
Bakual - comment - 12 Jul 2015

And if there actually is a problem, shouldn't we advice all template developers to fix their templates?

Most templates probably don't have such a link in the logo anyway :)

You sure? Don't you think that with all his heuristics Google is not giving a particular weight to a link in an element with class "site-title", at the top of the page

Yes, I'm quite sure. Because it would make no sense to determine a homepage based on a link in an element with a specific class when you can just access the site root instead just fine. Like anyone does.

But once again, the main issue is that the URL could change in time and thus what Google indexed will be no more valid.

That is true for any URL. Google can handle that just fine.
In this case, there is actually no issue. Assuming you have a single article as homepage and you change it to another article, what will happen? Someone searching for content of the old article (old homepage) will be directed to that old article instead of the new homepage. I'd say that is actually exactly what the user searching for content would expect. He gets to see the article containing the info he wants and he doesn't care if that is the homepage or not.

avatar smz
smz - comment - 12 Jul 2015

Yes, I'm quite sure. Because it would make no sense to determine a homepage based on a link in an element with a specific class when you can just access the site root instead just fine. Like anyone does.

In other words you're saying is that Google is not able to identify local home pages for multilingual non-sef sites. I'd really be cautious about that...

That is true for any URL. Google can handle that just fine.

But these are (semantically) special URLs: they are the local home pages for different languages.

... Someone searching for content of the old article (old homepage) will be directed to that old article instead of the new homepage. ...

"instead of the new homepage", that's the point, and of course assuming it is still there...

avatar Bakual
Bakual - comment - 12 Jul 2015

In other words you're saying is that Google is not able to identify local home pages for multilingual non-sef sites. I'd really be cautious about that...

If they do, they surely give more weight to a menu item called "home" (in the respective language) over a logo link with a specific class which is only present in Protostar anyway. And this one had that link for a long time already.
However I really don't think they store language specific "homepages". They store the site root and link to that, and Joomla will then make sure the user gets to see the correct homepage based on its language preferences.

"instead of the new homepage", that's the point, and of course assuming it is still there...

It's no issue. The user gets to see what he searched. If he wants to see the homepage instead, he will be able to press the navigation or logo link.
If the article is no longer there or the URL is invalid, Google will delete it from the index on its next crawl, like any other link.

Keep in mind that previously, the logo link always pointed to the site root. Google certainly didn't need that logo link to determine site root. The home menu items are unchanged. They are like they are for a very long time already. The issue you see would be present there for the same time. But I didn't see such an issue raised.
I don't know why the logo link should now change that.

avatar smz
smz - comment - 12 Jul 2015

OK, let's assume for a moment that you're right here and you are willing to put your an PLT's face on this.

Now: what is the problem solved in Protostar and not solved in Beeze3 and thousand of other templates?

avatar Bakual
Bakual - comment - 12 Jul 2015

Now: what is the problem solved in Protostar and not solved in Beeze3 and thousand of other templates?

Beez3, like probably most other templates, doesn't have a logo link to the homepage.

avatar smz
smz - comment - 12 Jul 2015

well, I haven't checked Beez3, TBH, but all other templates I know of have a logo link to the Home Page: /

avatar smz
smz - comment - 12 Jul 2015

... but the problem was?

avatar Bakual
Bakual - comment - 12 Jul 2015

On a sidenote: The link to the homepage has the classes brand pull-left. I doubt Google will give those classes any weight at all.

avatar Bakual
Bakual - comment - 12 Jul 2015

I don't know many templates myself. The ones I use don't have such a link (and I find it stupid myself anyway).
But for those templates nothing changed. And as long as nobody complains they don't need to fix anything.

avatar smz
smz - comment - 12 Jul 2015

On a sidenote: The link to the homepage has the classes brand pull-left. I doubt Google will give those classes any weight at all.

the item contained by the <a> link is $logo = '<span class="site-title" title="' . $sitename . '">' . $sitename . '</span>';

avatar smz
smz - comment - 12 Jul 2015

... and Beez3 does have a logo:

<h1 id="logo">
<?php if ($logo) : ?>
    <img src="<?php echo $this->baseurl; ?>/<?php echo htmlspecialchars($logo); ?>"  alt="<?php echo htmlspecialchars($templateparams->get('sitetitle')); ?>" />
<?php endif;?>
// ...
</h1>
avatar smz
smz - comment - 12 Jul 2015

Yes. But do you really think that Google will weight a link based on CSS classes contained within the link? I really doubt this. Sorry.

Yes, I do.

Edit:

But it doesn't have a link.

Correct, you're right on this.

... and the problem fixed by #7394 was?

avatar infograf768
infograf768 - comment - 13 Jul 2015

The problem was that the logo link was not using the cookie when sef was off, always directing to the setting in "Language Selection for new Visitors."
Easiest way to solve this was that PR, but it means for sure we have an issue somewhere else.

avatar infograf768
infograf768 - comment - 13 Jul 2015

I think I have found the solution.
We could revert #7394 and correct the languagefilter plugin where we do NOT check the cookie when sef is off.
Please test this patch:

diff --git a/plugins/system/languagefilter/languagefilter.php b/plugins/system/languagefilter/languagefilter.php
index a223e85..33bd8a6 100644
--- a/plugins/system/languagefilter/languagefilter.php
+++ b/plugins/system/languagefilter/languagefilter.php
@@ -309,4 +309,19 @@
        }
        // We are not in SEF mode
+       else
+       {
+           $lang_code = $this->getLanguageCookie();
+
+           if ($this->params->get('detect_browser', 1) && !$lang_code)
+           {
+               $lang_code = JLanguageHelper::detectLanguage();
+           }
+
+           if (!isset($this->lang_codes[$lang_code]))
+           {
+               $lang_code = $this->default_lang;
+           }
+       }
+
        $lang = $uri->getVar('lang', $lang_code);

diff --git a/templates/protostar/index.php b/templates/protostar/index.php
index e438d3f..6de3447 100644
--- a/templates/protostar/index.php
+++ b/templates/protostar/index.php
@@ -77,15 +77,4 @@
    $logo = '<span class="site-title" title="' . $sitename . '">' . $sitename . '</span>';
 }
-
-// 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 . '/';
-}
 ?>
 <!DOCTYPE html>
@@ -148,5 +137,5 @@
            <header class="header" role="banner">
                <div class="header-inner clearfix">
-                   <a class="brand pull-left" href="<?php echo $logo_link; ?>">
+                   <a class="brand pull-left" href="<?php echo $this->baseurl; ?>/">
                        <?php echo $logo; ?>
                        <?php if ($this->params->get('sitedescription')) : ?>

avatar smz
smz - comment - 13 Jul 2015

@infograf768
I was about saying the same: a problem like that could had only been in the LS! :smile:

Please also try my code review of the LS in #7286 on which I can swear!

avatar infograf768
infograf768 - comment - 13 Jul 2015

I think it would be simpler to just correct this known issue and think about refactoring at another time.
I will create a PR now.

avatar smz
smz - comment - 13 Jul 2015

@infograf768
I've spent tens of hours on that PR and I can swear on it. Also note that my version fixes other issues...

avatar smz
smz - comment - 13 Jul 2015

Oops!, sorry, still not totally awake.. :-)

Of course I intended to talk about the Language Filter (not Switcher) and the correct PR is #7271 (which correct other issues too...)

avatar infograf768
infograf768 - comment - 13 Jul 2015

@smz
I prefer to solve issues separately. We have here a regression, it is not a refactoring.
Please test #7427 and confirm solution. Thanks

avatar smz
smz - comment - 13 Jul 2015

why?

avatar infograf768
infograf768 - comment - 13 Jul 2015

A refactoring needs full tests of all aspects. A regression (at least such a simple one) just needs testing the issue concerned.

avatar smz smz - change - 13 Jul 2015
Status New Closed
Closed_Date 0000-00-00 00:00:00 2015-07-13 08:17:48
Closed_By smz
avatar smz smz - close - 13 Jul 2015

Add a Comment

Login with GitHub to post a comment