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

          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