Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-31514

Time format causing error messages on Windows servers

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Libraries
    • Labels:
    • Environment:
      Win 7, XAMPP, MySQL, any browser

      Description

      The following error is being reported numerous times around the site.

      Warning: Invalid CRT parameters detected in D:\xampp\htdocs\moodle_master\lib\moodlelib.php on line 1975

      This is caused by a change made in MDL-6102 in an attempt to remove leading zeros from time values. The format sequence %I was replaced with %l (lower-case L) in lang/en/langconfig.php, however this is not recognised by the libraries used to power Apache under Windows.

      I'm not sure why this change was made. The userdate() function already strips leading zeros, so this change would have had no effect. Also the added stripping of double-spaces in userdate(), added with the fix in MDL-6102 is probably not needed.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              salvetore Michael de Raadt added a comment -

              I've just added a patch, but essentially this just reverses MDL-6102.

              Show
              salvetore Michael de Raadt added a comment - I've just added a patch, but essentially this just reverses MDL-6102 .
              Hide
              quen Sam Marshall added a comment -

              Oops. I'll do a proper commit for this (although if you want to just revert it as an emergency measure, that would be OK).

              Leading zeros were not suppressed before the change. The code in userdate removes leading zeros from the 'day' field (%d) but not from the 'hour' field (%I).

              Show
              quen Sam Marshall added a comment - Oops. I'll do a proper commit for this (although if you want to just revert it as an emergency measure, that would be OK). Leading zeros were not suppressed before the change. The code in userdate removes leading zeros from the 'day' field (%d) but not from the 'hour' field (%I).
              Hide
              quen Sam Marshall added a comment -

              Glad it was only done for master...

              This fix combines the following:

              a) revert previous patch (done with git revert)
              b) add logic similar to the 'fixday' parameter so that it can also fix the hour parameter (called 'fixhour')

              Show
              quen Sam Marshall added a comment - Glad it was only done for master... This fix combines the following: a) revert previous patch (done with git revert) b) add logic similar to the 'fixday' parameter so that it can also fix the hour parameter (called 'fixhour')
              Hide
              salvetore Michael de Raadt added a comment -

              Thanks for working on that, Sam.

              That solution looks good. Pushing this to integration.

              Show
              salvetore Michael de Raadt added a comment - Thanks for working on that, Sam. That solution looks good. Pushing this to integration.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Hi Sam,
              I'm sending this back at the moment, changes appear to be 100% on target however the unit tests for userdate need to be updated to reflect these changes.

              Cheers
              Sam

              Show
              samhemelryk Sam Hemelryk added a comment - Hi Sam, I'm sending this back at the moment, changes appear to be 100% on target however the unit tests for userdate need to be updated to reflect these changes. Cheers Sam
              Hide
              salvetore Michael de Raadt added a comment -

              Sorry, Sam and Sam.

              I should have spotted that. Very tardy of me. I'm just keen to see this fix integrated because it's really annoying me.

              Show
              salvetore Michael de Raadt added a comment - Sorry, Sam and Sam. I should have spotted that. Very tardy of me. I'm just keen to see this fix integrated because it's really annoying me.
              Hide
              quen Sam Marshall added a comment -

              Yes sorry - will update unit tests today and resubmit.

              Show
              quen Sam Marshall added a comment - Yes sorry - will update unit tests today and resubmit.
              Hide
              quen Sam Marshall added a comment -

              Sorry, I have now corrected the unit tests (in the expected way i.e. I changed all the examples from 07:00 AM to 7:00 AM) and they are now working. (With the exception that, unrelated to this code, there's one failure about UTF-8 conversion or something.)

              Note: The bug is marked as affects stable etc but I think it was only ever broken on master (2.3 dev).

              Show
              quen Sam Marshall added a comment - Sorry, I have now corrected the unit tests (in the expected way i.e. I changed all the examples from 07:00 AM to 7:00 AM) and they are now working. (With the exception that, unrelated to this code, there's one failure about UTF-8 conversion or something.) Note: The bug is marked as affects stable etc but I think it was only ever broken on master (2.3 dev).
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

              TIA and ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
              Hide
              quen Sam Marshall added a comment -

              Rebased.

              Show
              quen Sam Marshall added a comment - Rebased.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks Sam, this has been integrated now

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks Sam, this has been integrated now
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Nice to see get this integrated
              Everything works as expected. I found an unrelated issue in recent activity page.
              Created MDL-31734 to deal with it.
              Thanks for fixing this
              passing

              Show
              ankit_frenz Ankit Agarwal added a comment - Nice to see get this integrated Everything works as expected. I found an unrelated issue in recent activity page. Created MDL-31734 to deal with it. Thanks for fixing this passing
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Some changes to Moodle should be milestones in the project by themselves.

              This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks!

              Closing as fixed, ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Some changes to Moodle should be milestones in the project by themselves. This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks! Closing as fixed, ciao

                People

                • Votes:
                  0 Vote for this issue
                  Watchers:
                  3 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    25/Jun/12