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

Only first browser locale is matched, and others ignored (most cases) #68

Closed
wq9578 opened this issue Jan 8, 2024 · 5 comments · Fixed by #69
Closed

Only first browser locale is matched, and others ignored (most cases) #68

wq9578 opened this issue Jan 8, 2024 · 5 comments · Fixed by #69

Comments

@wq9578
Copy link

wq9578 commented Jan 8, 2024

Due to a bug only the first browser locale is matched.

In most cases the second browser locale is relevant (en in en-US,en;...).
The buggy foreach loop then returns null, which means the browser locale is ignored.

Fixed source file below, see function getBrowserLocale.
Also some minor optimization added (LanguageSwitch::make()->getLocales() only called once).

<?php

namespace BezhanSalleh\FilamentLanguageSwitch\Http\Middleware;

use BezhanSalleh\FilamentLanguageSwitch\LanguageSwitch;
use Closure;
use Illuminate\Http\Request;

class SwitchLanguageLocale
{
    public function handle(Request $request, Closure $next): \Illuminate\Http\Response | \Illuminate\Http\RedirectResponse
    {
        $locale = session()->get('locale')
            ?? $request->get('locale')
            ?? $request->cookie('filament_language_switch_locale')
            ?? $this->getBrowserLocale($request)
            ?? config('app.locale', 'en');

        if (in_array($locale, LanguageSwitch::make()->getLocales())) {
            app()->setLocale($locale);
        }

        return $next($request);
    }

    private function getBrowserLocale(Request $request): ?string
    {
        $userLangs = preg_split('/[,;]/', $request->server('HTTP_ACCEPT_LANGUAGE'));

        /* Previous code (bug: foreach returns already in first loop)
        foreach ($userLangs as $locale) {
            return in_array($locale, LanguageSwitch::make()->getLocales())
                ? $locale
                : null;
        }
        */

        // New code begin
        $language_switch_locales = LanguageSwitch::make()->getLocales();

        foreach ($userLangs as $locale) {
            if (in_array($locale, $language_switch_locales)) {
                return $locale;
            }
        }

        return null;
        // New code end
    }
}
@wq9578 wq9578 changed the title Only first browser locale matched, and others ignored (most cases) Only first browser locale is matched, and others ignored (most cases) Jan 8, 2024
@UtkuDalmaz
Copy link

I was having an issue. It reverted to the browser locale even though I set it to another. Is that related to this bug?

@wq9578
Copy link
Author

wq9578 commented Jan 8, 2024

I don't think so. It looks like that your session data / cookie was lost.
Once set, it clearly has precedence over the browser locale:

        $locale = session()->get('locale')
            ?? $request->get('locale')
            ?? $request->cookie('filament_language_switch_locale')
            ?? $this->getBrowserLocale($request)
            ?? config('app.locale', 'en');

@UtkuDalmaz
Copy link

Sometimes when I open the page, it shows different languages on the page and the plugin. For example page loads in Turkish but it shows EN (English) in the up-right switcher.

@bezhanSalleh
Copy link
Owner

Sometimes when I open the page, it shows different languages on the page and the plugin. For example page loads in Turkish but it shows EN (English) in the up-right switcher.

As @wq9578 said if session and cookies are lost then it will default to the browser locale then the app locale.

@bezhanSalleh
Copy link
Owner

Due to a bug only the first browser locale is matched.

In most cases the second browser locale is relevant (en in en-US,en;...).

The buggy foreach loop then returns null, which means the browser locale is ignored.

Fixed source file below, see function getBrowserLocale.

Also some minor optimization added (LanguageSwitch::make()->getLocales() only called once).


<?php



namespace BezhanSalleh\FilamentLanguageSwitch\Http\Middleware;



use BezhanSalleh\FilamentLanguageSwitch\LanguageSwitch;

use Closure;

use Illuminate\Http\Request;



class SwitchLanguageLocale

{

    public function handle(Request $request, Closure $next): \Illuminate\Http\Response | \Illuminate\Http\RedirectResponse

    {

        $locale = session()->get('locale')

            ?? $request->get('locale')

            ?? $request->cookie('filament_language_switch_locale')

            ?? $this->getBrowserLocale($request)

            ?? config('app.locale', 'en');



        if (in_array($locale, LanguageSwitch::make()->getLocales())) {

            app()->setLocale($locale);

        }



        return $next($request);

    }



    private function getBrowserLocale(Request $request): ?string

    {

        $userLangs = preg_split('/[,;]/', $request->server('HTTP_ACCEPT_LANGUAGE'));



        /* Previous code (bug: foreach returns already in first loop)

        foreach ($userLangs as $locale) {

            return in_array($locale, LanguageSwitch::make()->getLocales())

                ? $locale

                : null;

        }

        */



        // New code begin

        $language_switch_locales = LanguageSwitch::make()->getLocales();



        foreach ($userLangs as $locale) {

            if (in_array($locale, $language_switch_locales)) {

                return $locale;

            }

        }



        return null;

        // New code end

    }

}

It was a PR didn't notice the issue with the loops. Nice catch 👍
Looks good to me could you pr it. Thanks.

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 a pull request may close this issue.

3 participants