View Issue Details

IDProjectCategoryView StatusLast Update
0007188OXID eShop (all versions)4.09. SEO, SEO URLpublic2020-11-09 13:20
Reportersimon.runer 
PrioritynormalSeverityminorReproducibilityalways
Status acknowledgedResolutionopen 
Product Version6.1.5 
Target VersionFixed in Version 
Summary0007188: language abbreviation might get added multiple times if lower case urls are used
DescriptionSet config parameter blSEOLowerCaseUrls and use a language abbreviation like nl_BE

This check
https://github.com/OXID-eSales/oxideshop_ce/blob/39dca71033876bcf0797748f4ae6f9a2cb3d427c/source/Core/SeoEncoder.php#L107
will fail similar to what was the problem in issue 0006407

Changing the function from

    public function addLanguageParam($sSeoUrl, $iLang)
    {
        $iLang = (int) $iLang;
        $iDefLang = (int) $this->getConfig()->getConfigParam('iDefSeoLang');
        $aLangIds = \OxidEsales\Eshop\Core\Registry::getLang()->getLanguageIds();

        if ($iLang != $iDefLang &&
            isset($aLangIds[$iLang]) &&
            // 0006407 bugfix, we should not search for the string saved in the db but for the escaped string
            getStr()->strpos($sSeoUrl, $this->replaceSpecialChars($aLangIds[$iLang]) . '/') !== 0
        ) {
            $sSeoUrl = $aLangIds[$iLang] . '/' . $sSeoUrl;
        }

        return $sSeoUrl;
    }

to

    public function addLanguageParam($sSeoUrl, $iLang)
    {
        $iLang = (int) $iLang;
        $iDefLang = (int) $this->getConfig()->getConfigParam('iDefSeoLang');
        $aLangIds = \OxidEsales\Eshop\Core\Registry::getLang()->getLanguageIds();

        $langUrlParam = $this->replaceSpecialChars($aLangIds[$iLang]);

        // if we work with lowercase urls, also compare lower case language params
        if ($this->getConfig()->getConfigParam('blSEOLowerCaseUrls')) {
            $strUtility = Str::getStr();
            $langUrlParam = $strUtility->strtolower($langUrlParam);
        }

        if ($iLang != $iDefLang &&
            isset($aLangIds[$iLang]) &&
            // 0006407 bugfix, we should not search for the string saved in the db but for the escaped string
            getStr()->strpos($sSeoUrl, $langUrlParam . '/') !== 0
        ) {
            $sSeoUrl = $aLangIds[$iLang] . '/' . $sSeoUrl;
        }
        return $sSeoUrl;
    }

Should solve this
Additional Information- es -
TagsNo tags attached.
ThemeAll
BrowserAll
PHP Version7.1
Database VersionNot defined

Activities

QA

2020-10-28 14:27

administrator   ~0013339

have tried to recreate the behavior. Config parameter set:
$this->blSEOLowerCaseUrls = true;
and tried to create language with the language abbreviation en_AU a second time

-> Error: A language with this language abbreviation already exists

Seolowercase.JPG (67,405 bytes)
Seolowercase.JPG (67,405 bytes)

simon.runer

2020-10-28 22:41

reporter   ~0013340

The problem is, that function addLanguageParam() sometimes gets called with a
$sSeoUrl = en-au/product.html

Then this check will be true
getStr()->strpos($sSeoUrl, $this->replaceSpecialChars($aLangIds[$iLang]) . '/') !== 0
and you will get back
$sSeoUrl = en-AU/en-au/product.html
which later is saved to the database as en-au/en-au/product.html

oxid is basically checking if the $sSeoUrl passed to the function addLanguageParam() already contains the language param.
en_AU becomes en-AU because of the function replaceSpecialChars()
but that still is not the same as en-au

QA

2020-10-30 08:27

administrator   ~0013341

As you write, the function addLanguageParam() is sometimes called with a $sSeoUrl = en-au/product.html and appended the language abbreviation again.
I could not reproduce this. In my case the SEOURL was always stored in the DB as follows:
en-au/product.html

Can you please tell me how to reproduce the behavior?

simon.runer

2020-11-08 09:27

reporter   ~0013346

unfortunately I am not able to reproduce it. I suspect the function (eventually when other modules are installed) gets called twice. But I am not sure.

What I am sure about is:

As Oxid Standard does this check

getStr()->strpos($sSeoUrl, $langUrlParam . '/') !== 0

it expects $sSeoUrl already having a language parameters in it. Otherwise this check could be removed.

This check will fail if
$sSeoUrl == "en-au/product.html";
and
$langUrlParam == "en-AU";

QA

2020-11-09 13:20

administrator   ~0013347

You are right, if Oxid Standard performs this check, it is also expected that $sSeoUrl might already contain a language parameter, otherwise this check might be removed.

getStr()->strpos($sSeoUrl, $langUrlParam . '/') !== 0

Thus your objection is justified and the bug entry is aknowledged. Thanks for your feedback.