pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
306 stars 445 forks source link

Date format isn't properly localized in OJS 3.4 #9303

Closed jonasraoni closed 1 month ago

jonasraoni commented 1 year ago

Describe the bug On OJS 3.4, a function to convert the deprecated date format to the new one was introduced (\PKP\core\PKPString::getStrftimeConversion()), and it's known to affect at least the Smarty templates.

Smarty: Depending on some factors, the Smarty's date_format modifier might use internally the strftime (deprecated) or the date function, whereas the latter isn't localized. One of the factors is the presence of a deprecated date format (e.g. "%B"), which happens at OJS 3.3, but not anymore on OJS 3.4 (due to the method mentioned previously).

Once you change the language to anything else than English, the localization for the interface will change, but localizable dates pieces will be kept in English, as shown in the screenshot below.

What application are you using? OJS 3.4

Additional information image

asmecher commented 1 year ago

Smarty currently either uses the deprecated strftime or non-localized date, and that's apparently not scheduled for fixing until Smarty adds PHP9 support; see https://github.com/smarty-php/smarty/issues/810 for details.

jonasraoni commented 1 year ago

Yeah, we might attempt to fix it on our end, but it will require the extension they've mentioned/3rd-party package.

asmecher commented 1 year ago

We already have Carbon in our Composer lockfile; it's probably a good candidate.

diegoabadan commented 5 months ago

A customer with OMP 3.4 reported the same problem.

asmecher commented 2 months ago

Here are some PRs to partially fix the issue (in cases where PHP's date() was being used to format dates):

I didn't replace all cases of date(), just the ones where date formats were supplied that might include translatable content (e.g. month names). I've tested each case individually, since the PRs are for stable-3_4_0; they'll need cherry-picking forward to main.

Edit: Here is another PR that adds the missing Smarty support, which will complete the issue;

asmecher commented 2 months ago

@defstat, would you mind reviewing these PRs?

asmecher commented 2 months ago

^ Quick note about the above PRs: I chose for now not to add the Carbon service provider for Laravel into our bootstrapping, even though it would have the benefit of configuring the locale for us (rather than having to call Carbon::locale(...) each time). This is because we have a bootstrapping problem where the service provider would attempt to determine the locale before the PKP environment is sufficiently configured. Stack trace to demonstrate:

[29-Aug-2024 00:51:01 UTC] PHP Fatal error:  Uncaught AssertionError: assert(false) in .../lib/pkp/classes/core/PKPRequest.php:861
Stack trace:
#0 .../lib/pkp/classes/core/PKPRequest.php(861): assert()
#1 .../lib/pkp/classes/core/PKPRequest.php(754): PKP\core\PKPRequest->_delegateToRouter()
#2 .../classes/core/Request.php(41): PKP\core\PKPRequest->getContext()
#3 .../lib/pkp/classes/i18n/Locale.php(495): APP\core\Request->getContext()
#4 .../lib/pkp/classes/i18n/Locale.php(262): PKP\i18n\Locale->_getSupportedLocales()
#5 .../lib/pkp/classes/i18n/Locale.php(139): PKP\i18n\Locale->isSupported()
#6 .../lib/pkp/classes/i18n/Locale.php(130): PKP\i18n\Locale->setLocale()
#7 .../lib/pkp/lib/vendor/nesbot/carbon/src/Carbon/Laravel/ServiceProvider.php(104): PKP\i18n\Locale->getLocale()
#8 .../lib/pkp/lib/vendor/nesbot/carbon/src/Carbon/Laravel/ServiceProvider.php(63): Carbon\Laravel\ServiceProvider->getLocale()
#9 .../lib/pkp/lib/vendor/nesbot/carbon/src/Carbon/Laravel/ServiceProvider.php(45): Carbon\Laravel\ServiceProvider->updateLocale()
#10 .../lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(36): Carbon\Laravel\ServiceProvider->boot()
#11 .../lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Container/Util.php(41): Illuminate\Container\BoundMethod::Illuminate\Container\{closure}()
#12 .../lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(93): Illuminate\Container\Util::unwrapIfClosure()
#13 .../lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(35): Illuminate\Container\BoundMethod::callBoundMethod()
#14 .../lib/pkp/lib/vendor/laravel/framework/src/Illuminate/Container/Container.php(661): Illuminate\Container\BoundMethod::call()
#15 .../lib/pkp/classes/core/PKPContainer.php(148): Illuminate\Container\Container->call()
#16 .../lib/pkp/classes/core/PKPContainer.php(135): PKP\core\PKPContainer->register()
#17 .../lib/pkp/classes/core/PKPApplication.php(228): PKP\core\PKPContainer->registerConfiguredProviders()
#18 .../lib/pkp/classes/core/PKPApplication.php(200): PKP\core\PKPApplication->initializeLaravelContainer()
#19 .../classes/core/Application.php(50): PKP\core\PKPApplication->__construct()
#20 .../lib/pkp/includes/bootstrap.php(37): APP\core\Application->__construct()
#21 .../index.php(18): require_once('...')
asmecher commented 1 month ago

All merged.