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

Bug: Matching Locale with Negotiate Locale does not work in some cases as expected #9256

Open
crustamet opened this issue Nov 5, 2024 · 8 comments
Labels
missing feature Reported issue which is not a bug but needs to be implemented

Comments

@crustamet
Copy link
Contributor

PHP Version

8.3

CodeIgniter4 Version

4.5.5

CodeIgniter4 Installation Method

Manual (zip or tar.gz)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

No response

What happened?

public string $defaultLocale = 'en-US';
public bool $negotiateLocale = true;
public array $supportedLocales = ['en-US','en-GB'];

why like this ? Because i want to show different content for United Kingdom vs United States

Steps to Reproduce

image
to reproduce you must place UK first instead of the US

The app does not have {locale} specific routes

in your view just put it should always be equal to the $Request->getLocale()

In my Language Folder
I have them in separate folders en-US and en-GB

In the request header i have this
accept-language: en-US,en-GB;q=0.9,en;q=0.8,ro;q=0.7

and if i switch to English - United Kingdom the accept-language changes to this
accept-language:en-GB;en-US;q=0.9,en;q=0.8,ro;q=0.7

Expected Output

When i have the United Kingdom version as my default i should see

when i get the Locale from the Request the en-GB version instead of the en-US

echo $Request->getLocale(); always output en-US in both cases
it should return the en-GB version !

The problem is why is this happening is because of this file
https://github.com/codeigniter4/CodeIgniter4/blob/develop/system/HTTP/Negotiate.php#L181

There is a loop over the Supported languages and the first two are both english from my SupportedLocales so it returns only the first Locale matched and not the exact matched one.

I have test this it only works when i change the order from the
App Config
public array $supportedLocales = ['en-US','en-GB']; always shows the US page even if you have the default language en-GB
public array $supportedLocales = ['en-GB','en-US']; always shows the GB page even if you have the default language en-US.

Anything else?

No response

@crustamet crustamet added the bug Verified issues on the current code behavior or pull requests that will fix them label Nov 5, 2024
@michalsn
Copy link
Member

michalsn commented Nov 6, 2024

The problem here is that we want a loose comparison for locales most of the time.

Setting the last variable to false should help: https://github.com/codeigniter4/CodeIgniter4/blob/develop/system/HTTP/Negotiate.php#L130

Should we make that configurable? Is it worth creating another config file/property? Personally, I'm not convinced, although I would like to hear other people's opinions.

The instant solution would be to extend the Negotiate class and declare changed service in app/Config/Services.php file - which I recommend.

Anyway, not really a bug, maybe a missing feature.

@michalsn michalsn added missing feature Reported issue which is not a bug but needs to be implemented and removed bug Verified issues on the current code behavior or pull requests that will fix them labels Nov 6, 2024
@ALTITUDE-DEV-FR
Copy link
Contributor

Hello,
I also have an issue with {locale} in url_to() and route_to().

For example, if I create a page in French at /fr/mypage (/{locale}/mypage)
and use url_to('Mycontroller/', 'slug');, it returns the page as /en/mypage.

I'm not sure if this issue is related?

Thanks.

@michalsn
Copy link
Member

@ALTITUDE-DEV-FR No, I don't think so.

Please make sure you have updated the App::supportedLocales array with fr. If that's not it, please create a separate issue and provide a minimal code sample so we can reproduce the problem.

@crustamet
Copy link
Contributor Author

So any update on this, because from my POV the negotiate locale does not do what it says

"Once this is enabled, the system will automatically negotiate the correct language based upon an array of locales that you have defined in $supportLocales. If no match is found between the languages that you support, and the requested language, the first item in $supportedLocales will be used. In the following example, the en locale would be used if no match is found:"

It just does not do that

So it appears that it does not find en-GB but it find en-US

Entering from en-GB returns en-US because from the algorithm there who wrote that searches for the first segment of the string

"en" so it matches then returns the first from the array, even thoug i have exact match on en-GB still returns en-US

how come this is not a bug

@michalsn
Copy link
Member

As I explained in my first post:

The problem here is that we want a loose comparison for locales most of the time.

The intention here was quite clear if you look at the current implementation - locale subtypes are matched to a broad type. The examples in the user guide also fit into this scenario.

Feel free to send a PR with improvements.

@crustamet
Copy link
Contributor Author

Well i have managed to get it working but i don't know man how to PR !

can i just put the code here and you manage it ?

public function language(array $supported): string
{
	return self::getBestLanguageMatch($supported, $this->request->getHeaderLine('accept-language'));
}

public static function getBestLanguageMatch(array $supportedLocales, string $acceptLanguageHeader): string
{
    $acceptedLocales = self::parseAcceptLanguage($acceptLanguageHeader);

    // Sort accepted locales by their quality value in descending order
    usort($acceptedLocales, function ($a, $b) {
        return $b['q'] <=> $a['q'];
    });

    // Find the best match
    foreach ($acceptedLocales as $acceptedLocale) {
        foreach ($supportedLocales as $supportedLocale) {
            if (self::isLocaleMatch($acceptedLocale['locale'], $supportedLocale)) {
                return $supportedLocale;
            }
        }
    }

    // Default to the first supported locale if no match is found
    return $supportedLocales[0];
}

private static function parseAcceptLanguage(string $header): array
{
    $locales = [];
    $parts = explode(',', $header);

    foreach ($parts as $part) {
        $sections = explode(';', trim($part));
        $locale = $sections[0];
        $quality = isset($sections[1]) ? (float)str_replace('q=', '', $sections[1]) : 1.0;

        $locales[] = [
            'locale' => $locale,
            'q' => $quality,
        ];
    }

    return $locales;
}

private static function isLocaleMatch(string $acceptedLocale, string $supportedLocale): bool
{
    // Exact match or language-only match (e.g., 'en' matches 'en-US')
    return $acceptedLocale === $supportedLocale ||
        strpos($acceptedLocale, '-') === false && stripos($supportedLocale, $acceptedLocale . '-') === 0;
}

with this it returns the exact match if exist despite the order of SupportedLocales array

"loose comparison my ass"

getBestMatch execution time: 4.0054321289062E-5 seconds
getBestLanguageMatch execution time: 1.0967254638672E-5 seconds

@crustamet
Copy link
Contributor Author

while trying out these updates and playing with the supported langs
i accidently removed all supportedLangs from the app config
so that supportedLangs = [];

and then there was no 'en' language folder because i changed it to en-US

image

my server went down because of this with current implementation

@michalsn
Copy link
Member

Well i have managed to get it working but i don't know man how to PR !
can i just put the code here and you manage it ?

No, sorry - this is not how open-source works.

You may find this helpful: https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

However, I don’t think we will accept changes that reinvent the wheel and duplicate methods we already have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
missing feature Reported issue which is not a bug but needs to be implemented
Projects
None yet
Development

No branches or pull requests

3 participants