codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.4k stars 1.9k forks source link

Bug:`Locale::getDefault()` is different from `$this->request->getLocale()` when $negotiateLocale is true #7668

Closed kenjis closed 1 year ago

kenjis commented 1 year ago

version: develop (4.3.x)

Locale::getDefault() is different from $this->request->getLocale() when we use content negotiation.

--- a/app/Config/App.php
+++ b/app/Config/App.php
@@ -83,7 +83,7 @@ class App extends BaseConfig
      *
      * If false, no automatic detection will be performed.
      */
-    public bool $negotiateLocale = false;
+    public bool $negotiateLocale = true;

     /**
      * --------------------------------------------------------------------------
@@ -98,7 +98,7 @@ class App extends BaseConfig
      *
      * @var string[]
      */
-    public array $supportedLocales = ['en'];
+    public array $supportedLocales = ['en', 'ja'];

     /**
      * --------------------------------------------------------------------------
<?php

namespace App\Controllers;

use Locale;

class Home extends BaseController
{
    public function index()
    {
        return 'Request:' . $this->request->getLocale()
            . ', Locale:' . Locale::getDefault();
    }
}
--- a/app/Config/Routes.php
+++ b/app/Config/Routes.php
@@ -31,6 +31,8 @@ $routes->set404Override();
 // route since we don't have to scan directories.
 $routes->get('/', 'Home::index');

+$routes->get('{locale}', 'Home::index');
+
 /*
  * --------------------------------------------------------------------
  * Additional Routing

When we use only content negotiation:

$ curl -H "Accept-Language: ja,en-US;q=0.9,en;q=0.8" http://localhost:8080/
Request:ja, Locale:en

But If {locale} is used in a route, both locales are set to ja.

$ curl -H "Accept-Language: ja,en-US;q=0.9,en;q=0.8" http://localhost:8080/ja
Request:ja, Locale:ja
kenjis commented 1 year ago

This is because:

This bug was fixed in 4.4 branch.

michalsn commented 1 year ago

Why is this an issue?

  1. The content negotiation example is okay.

  2. If {locale} is used in the route, then treating it as default is logical. Nobody said that Locale::getDefault() has to be equal to Config\App::defaultLocale. Current behavior gives a nice option to distinguish lang used in the route and client preference.

kenjis commented 1 year ago

@michalsn Do you say the current behavior is expected?

Content negotiation: Request:ja, Locale:en

{locale} in the route: Request:ja, Locale:ja

michalsn commented 1 year ago

@kenjis I think yes.

Is there any other way (method) to find out what {locale} is being used? When we specify a language in a URL, it seems reasonable to treat it as a default locale.

kenjis commented 1 year ago

@michalsn Okay, I also think the case of {locale} in the route is okay.

But why is the Locale not changed when using Content Negotiation?

Content negotiation: Request:ja, Locale:en

michalsn commented 1 year ago

@kenjis The framework has no basis to set jp as the default Locale when we use Content Negotiation.

When the locale is in the URL, then we clearly specify the application to work in that language. So to speak, we overwrite the settings from the config file.

kenjis commented 1 year ago

I don't know the difference between the locale in the URL and the locale in Accept-Language header. When we use Content Negotiation, It seems to me it clearly specify the application to work in that language.

But if the current behavior is correct, we need to fix the behavior in 4.4 branch.

(4.4 *$=)$ curl -H "Accept-Language: ja,en-US;q=0.9,en;q=0.8" http://localhost:8080/
Request:ja, Locale:ja
(4.4 *$=)$ curl -H "Accept-Language: ja,en-US;q=0.9,en;q=0.8" http://localhost:8080/ja
Request:ja, Locale:ja
michalsn commented 1 year ago

@kenjis My bad. I think you're right.

Locale::getDefault() should always be the same as $this->request->getLocale() by default.

kenjis commented 1 year ago

@michalsn No problem! If you have any concern about it, feel free to say.

I think if these locale values are different, it is very confusing.

kenjis commented 1 year ago

This bug was fixed in 4.4 branch.