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: Tests Time::humanize() does not work with ar locale #9478

Closed
neznaika0 opened this issue Mar 6, 2025 · 5 comments · Fixed by #9481
Closed

Bug: Tests Time::humanize() does not work with ar locale #9478

neznaika0 opened this issue Mar 6, 2025 · 5 comments · Fixed by #9481

Comments

@neznaika0
Copy link
Contributor

PHP Version

8.4

CodeIgniter4 Version

4.6

CodeIgniter4 Installation Method

Git

Which operating systems have you tested for this bug?

Linux

Which server did you use?

cli-server (PHP built-in webserver)

Database

No response

What happened?

PHP 8.4.4 has made some changes.

1) CodeIgniter\I18n\TimeLegacyTest::testHumanizeWithArLocale
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'١ week ago'
+'1 week ago'

/development/tests/system/I18n/TimeLegacyTest.php:1106

2) CodeIgniter\I18n\TimeTest::testHumanizeWithArLocale
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'١ week ago'
+'1 week ago'

/development/tests/system/I18n/TimeTest.php:1191

Steps to Reproduce

Run the tests on local PC: Arch, PHP 8.4.4.
List locales:

$ localectl list-locales
C.UTF-8
ar_AE.UTF-8
en_US.UTF-8
ru_RU.UTF-8

Expected Output

Successful completion of the tests is expected

Anything else?

I don't use specific languages, maybe @datamweb can tell us.

Old PR here: #6120

@neznaika0 neznaika0 added the bug Verified issues on the current code behavior or pull requests that will fix them label Mar 6, 2025
@datamweb
Copy link
Contributor

datamweb commented Mar 6, 2025

I am not an Arabic speaker, but in any case, I don’t see any issue. The output for me is as follows:

$time->humanize() UTF-8 string (20) "منذ ١ أسبوع"

which seems correct.

@michalsn
Copy link
Member

michalsn commented Mar 7, 2025

  1. This is your local ICU database problem, not a framework-level issue.
  2. The results of the localectl list-locales command are unrelated to the ICU locale database.
  3. The intl extension depends on ICU locales and not system-level locale settings.

@michalsn michalsn closed this as completed Mar 7, 2025
@michalsn michalsn removed the bug Verified issues on the current code behavior or pull requests that will fix them label Mar 7, 2025
@neznaika0
Copy link
Contributor Author

neznaika0 commented Mar 8, 2025

I found solution. Update locale as ar@numbers=arab
See https://unicode-org.github.io/icu/userguide/locale/#variant-code
and https://www.php.net/manual/en/locale.getkeywords.php here it is shown that it is possible to have these keywords in a locale.

Do I need to update the tests, or will we rely on everyone's system (ICU) being configured as on Github Actions?

My version intl:

ICU version	        76.1
ICU Data version	76.1
ICU TZData version	2024b
ICU Unicode version	16.0

@michalsn
Copy link
Member

michalsn commented Mar 8, 2025

Does the ICU version impact the default settings for the ar? From what I know, most Arabic countries use Arabic-Eastern numbers, but there are some exceptions, and they use Arabic-Western (Latin) numbers (UAE, Morocco, and maybe some more).

If ar may be "linked" differently depending on the ICU version, we should change the test for something more predictable, like ar-SA.

@michalsn michalsn reopened this Mar 8, 2025
@michalsn
Copy link
Member

michalsn commented Mar 8, 2025

Okay, I did some deeper digging, and it turns out this change was introduced in ICU 76 (released on October 24, 2024).

The modification came from CLDR 46, where the default numbering system for ar was changed from arab (Arabic-Eastern numerals) to latn (Latin numerals). You can see the exact change here: CLDR #3678.

If you're interested in this, please send a PR which will change the locale from ar to ar-SA - this will guarantee the Arabic-Eastern numbers for everyone.

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