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

Fix format_date with "x" format for Windows #9250

Merged
merged 3 commits into from
Dec 10, 2023

Conversation

mvorisek
Copy link
Contributor

the 1st param of IntlDateFormatter::__construct is Intl locale, not system locale

on Windows and PHP 8.3 and I getting Found unconstructed IntlDateFormatter exceptions - php/php-src#12912

@alecpl
Copy link
Member

alecpl commented Dec 10, 2023

The intention of the code was to return "date representation based on locale" (i.e. %x from strftime()). Using English US date format is just wrong.

That being said, we dropped strftime-compatible format support in 1.6. Also, the 'x' formatter does something completely different in date().

So, we have tho options:

  1. remove this whole if branch
  2. fix the issue properly (and use another letter?)

@mvorisek
Copy link
Contributor Author

What about new IntlDateFormatter(null) to use intl.default_locale?

docs: https://www.php.net/manual/en/intldateformatter.create.php#refsect1-intldateformatter.create-parameters

@alecpl
Copy link
Member

alecpl commented Dec 10, 2023

Maybe, if you set intl.default_locale in rcmail::set_user().

@alecpl
Copy link
Member

alecpl commented Dec 10, 2023

In other words, this has to be a user locale, not system locale.

@alecpl
Copy link
Member

alecpl commented Dec 10, 2023

The tests pass, but we should set intl.default_locale there too. I.e. in Rcmail_Rcmail::test_format_date().

@alecpl alecpl merged commit f9a9ec4 into roundcube:master Dec 10, 2023
7 checks passed
@mvorisek mvorisek deleted the fix_IntlDateFormatter_locale branch December 10, 2023 14:30
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 this pull request may close these issues.

2 participants