@tsteur opened this Issue on November 9th 2014 Owner

See #6435

I think most of the issues related to German locale (or any other locale that uses a comma instead of a dot to separate decimal values) are fixed but it is hard to tell as most parts of the code are not really covered by tests.

After working on #6435 there can be only one conclusion the more I think about it: We always have to set an en_* locale and format numbers in Twig templates and API (if we want so) etc using Twig filter or using a library or so.

It is easy to explain: While we can live with the fact or bug when a number is not formatted correct in the UI (eg 5.5 instead of 5,5 => easy to fix) we cannot let the platform track or calculate wrong values. It is so easy to cause a bug / wrong values if not a en_* locale is used and those bugs are in places where nobody would expect them see the changes and my comments. Even the PiwikTracker was sending wrong values with non en_* locale. Another thing is that we cannot make sure all the libraries work correctly. For instance the pdf library uses floats quite a lot but also Zend etc. Last but not least we cannot expect plugin developers to be aware of this problem. The worst part: As we do not use a German locale in our tests we would not even notice once we track or display wrong metrics.

Be aware that our code sets the German locale as soon as someone uses German language. Probably most of our users use a language where a comma is used instead of a dot. As soon as this locale is installed on the server we track wrong values.

We should also add a system check to make sure an English locale is installed.

@mattab commented on November 10th 2014 Owner

Be aware that our code sets the German locale as soon as someone uses German language.

I wonder the reason we set the locale, maybe there was a reason? If there was no reason, maybe we could set the en_US default locale instead?

@tsteur commented on November 10th 2014 Owner

As written in the other issue we need to set it to make sure it uses for example a comma instead of a dot in the UI to separate decimal

@mattab commented on April 8th 2015 Owner

@tsteur what would you reckon is the next step for this issue (what could we do to prevent data error bugs due to locale)? eg. adding more CI tests for locale != en or something else maybe...

@tsteur commented on April 20th 2015 Owner

The next step to do is what I wrote in the issue description I reckon. I'd probably try to fix the underlying problem. More tests won't help us at all. Those issues occur where you don't expect them. And even currently on Travis the tests that exist are skipped or so.

It would be a lot of work but I reckon to actually fix it we'd have to use always en_* locale or so. Then handle localization etc in views or so. Maybe there's a better solution but that's like the only one I can think of currently.

@mattab commented on April 21st 2015 Owner

After working on #6435 there can be only one conclusion the more I think about it: We always have to set an en_* locale and format numbers in Twig templates and API (if we want so) etc using Twig filter or using a library or so.

Now i'm wondering why we even set the locale in the first place. I didn't remember why I added this and reading the setlocale doc I can't see any feature that we actually need. it was likely added only for number formatting - so we can replace the setlocale call by a static setlocale('en... to fix this.

then to make Piwik look nice to all users in any of their language, we'd need to apply formatting to numbers before they are output in the UI and email reports (discussed in #4114)

@mattab commented on April 21st 2015 Owner

. I didn't remember why I added this and reading the setlocale doc I can't see any feature that we actually need.

well I wrote this too quickly... it is important to setlocale to the current language's locale because for example string operations the core piwik or plugins rely on, may use the locale info. For example in php when eg. uppercase / lowercase a given letter ( eg lcfirst), (and more string functions) use locale information.

Suggested steps:

  • leave the current setlocale using current selected language
  • add call to setlocale(LC_NUMERIC, 'en...'); to set the locale to english for numbers/floats data manipulation
  • add a system check to make sure an English locale is installed
  • (out of scope/next project: format numbers in UI and email reports #4114)
Powered by GitHub Issue Mirror