Publish error on Arabic locale caused by invalid date format

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS

https://www.silverstripe.org

BSD 3-Clause "New" or "Revised" License

721 stars 822 forks source link

Publish error on Arabic locale caused by invalid date format #8097

Closed hudhaifas closed 6 years ago

hudhaifas commented 6 years ago

Affected Version

4.1.0

Description

When I set the locale to Arabic, I got the following internal error on publishing pages and files [Emergency] Uncaught InvalidArgumentException: Invalid date: '٢٠١٨-٠٥-٢٦ ٢٣،١٢،٢١'. Use y-MM-dd to prevent this error.

Steps to Reproduce

CMS -> My Profile
Change Interface Language to: Arabic (Egypt)
Try to publish a page

PRs:

[ ] https://github.com/silverstripe/silverstripe-framework/pull/8170

hudhaifas commented 6 years ago

Also the wrong date format on Arabic locale causes showing blank $Date.Nice, $Date.Long, etc when used the template

robbieaverill commented 6 years ago

We've got a failing unit test in silverstripe/blog which reproduces this issue (with SS 4.2.x-dev): SilverStripe\Blog\Tests\BlogFunctionalTest::testBlogWithMultibyteUrl.

Blog test failures: https://travis-ci.org/silverstripe/silverstripe-blog/jobs/390546326

My local changes that caused this to start breaking are unrelated to dates but related to test fixtures: https://github.com/silverstripe/silverstripe-framework/compare/77a45c0...df8298f

hudhaifas commented 6 years ago

I think the problem originally caused by the Numbers format (numerals system), I noticed when set locale to Arabic, all Numeric fields in the CMS show Eastern Arabic numerals (also called Hindi numerals), which causes an Internal Error when try to save.

Arabic numerals | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 Hindi numerals | ٩ | ٨ | ٧ | ٦ | ٥ | ٤ | ٣ | ٢ | ١ | ٠

Same thing happened with the date format shown above exception: '٢٠١٨-٠٥-٢٦ ٢٣،١٢،٢١' instead of '2018-05-26 23,12,21'

So we need to look for the numeral system used with the Arabic locale and change from Hindi numerals to Arabic numerals.

tractorcow commented 6 years ago

I found out the cause of this. It's a bug in core. Our ISO8601 localisation formatter does set the format string correctly, but it fails to reset the locale, meaning that locale-specific numeric localisation rules are still being applied.

Hence, 2018-06-13 12:37:40 formatted "as iso8601" is coming out as "٢٠١٨-٠٦-١٣ ١٢:٣٧:٤٠" due to our ignorance of the arabic localisation.

Solution: Make sure the DBDate::getISOFormat() also resets the locale. ;)

tractorcow commented 6 years ago

Fix at https://github.com/silverstripe/silverstripe-framework/pull/8170

tractorcow commented 6 years ago

Sorry, I think I posted my earlier reply the same time as you @hudhaifas :) Good to see we came to the same conclusion.

tractorcow commented 6 years ago

@hudhaifas I hope that the fix works out for you.

I'd also love to hear any more feedback on how we can better localise SilverStripe.

hudhaifas commented 6 years ago

It fixed one use case; numerals in date object, which solved publishing problem and showing Nice date, but still:

Frontend: Numerics in dates are shown in a different numeral system (Eastern Arabic) than the remain numerics (Western Arabic) in the website, see difference between SS3 and SS4
Backend: when the user set the locale to Arabic all numerics are changed to Eastern Arabic system, which causes an Internal Error on save, see difference between SS3 and SS4

We need to find a way to set the default numerals system to Western Arabic, as it was in SS3

tractorcow commented 6 years ago

Yes the bug in non date numeric values is a problem. We could switch the number field to html5 default which omits localisation. That's just a workaround but not a real fix though.

tractorcow commented 6 years ago

Would it be preferable for us to support hindi numerals rather than trying to remove them?