CRM-5554 Civicrm should check for a false return value on calling setlocale()

    Details

    • Type: Improvement
    • Status: Done/Fixed
    • Priority: Minor
    • Resolution: Fixed/Completed
    • Affects Version/s: 3.0.3
    • Fix Version/s: 3.0.4
    • Component/s: Core CiviCRM
    • Labels:
      None

      Description

      When civicrm calls setlocale() in Variables.php, it does not check for a return value of false. This means that either the php install does not have gettext installed or none of the locales are available and should cause an error to warn the user.

      I have attached a patch against 3.0.3 to add a check for setlocale() returning false and throw a fatal error. (This is actually my first time submitting a patch, so hopefully I got the format right

      Jordan

        Attachments

          Activity

          [CRM-5554] Civicrm should check for a false return value on calling setlocale()
          Piotr Szotkowski added a comment -

          The patch is perfectly good (and most appreciated!), but I'm not sure we actually want to do this. With this patch, people without gettext support or without the locales will see this error; with the current situation, they simply silently fall back to the default locale.

          Do you have a particular use case where throwing a fatal error in this case is actually preferable?

          Jordan Thoms added a comment -

          See http://forum.civicrm.org/index.php/topic,11324.0.html.
          Not having gettext support creates some subtle bugs throughout civicrm, such as empty values for $config->monetaryDecimalPoint, which then causes there to be no decimal point shown at all on the javascript calculation for the event screen.

          So It's not really an option to run civicrm without gettext currently. Maybe a better option would be to give the user an warning and set some sane defaults (ie . dot for decimal point and comma for thousands separator.)

          Then we are not adding any new requirements but will also remove the possibility of having empty values for decimal point and thousands separator, and be letting users know that their locale setting could not be used because of the missing gettext support.

          I can write up a patch to do this if you think it's a good option.

          Piotr Szotkowski added a comment -

          Ok, so we should sanely handle the lack of certain packages - I'm still not really convinced that throwing a fatal error at a user (who might not be able to fix it anyway) is the right approach.

          Can you confirm what's the problem of your setup? Ideally, send me (shot@civicrm.org) the output of phpinfo() call (or php -i call)?

          Donald A. Lobo added a comment -


          so two more issues reported something similar to this on IRC (cjdavis, parvez). so we should definitely check and put in SOME defaults and/or even throw a fatal

          Piotr Szotkowski added a comment -

          I think I fixed this in r25874 - Jordan, can you please take a look and verify? http://fisheye2.atlassian.com/changelog/CiviCRM/?cs=25874

          Piotr Szotkowski added a comment -

          Closing to clear the queue for 3.0.4 release; please reopen if not fixed on your end.

            People

            • Assignee:
              Piotr Szotkowski
              Reporter:
              Jordan Thoms

              Dates

              • Created:
                Updated:
                Resolved: