Moodle
  1. Moodle
  2. MDL-31622

Names of days and months in calendar popup are not translatable

    Details

    • Testing Instructions:
      Hide

      Test pre-requisites

      • A few languages packs installed, including with non-enlish characters
      • Installing corresponding locales on your system `sudo locale-gen --no-archive ja_JP.UTF-8`
      • You might have to manually install language packs for master

      Test steps

      1. Go to your home page and switch to a language
      2. Go to the calendar, and add a new event
      3. Check that the popup calendar translates months and days of the week
      4. Repeat with each other languages
      Show
      Test pre-requisites A few languages packs installed, including with non-enlish characters Installing corresponding locales on your system `sudo locale-gen --no-archive ja_JP.UTF-8` You might have to manually install language packs for master Test steps Go to your home page and switch to a language Go to the calendar, and add a new event Check that the popup calendar translates months and days of the week Repeat with each other languages
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-31622-master
    • Rank:
      38192

      Description

      If the popup calendar is used names of months and days of week are always english. Please replace the description in the code at

      moodle/lib/yui/3.4.1/build/calendar-base/lang

      by Moodle lang strings. This is not the Moodle way to code language elements

      1. arabic add event.png
        169 kB
      2. arabic course page.png
        136 kB
      3. cal01.png
        24 kB
      4. german.png
        161 kB

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for spotting that, Ralf.

          Show
          Michael de Raadt added a comment - Thanks for spotting that, Ralf.
          Hide
          Frédéric Massart added a comment -

          Estimated time: 7 hours

          Show
          Frédéric Massart added a comment - Estimated time: 7 hours
          Hide
          Frédéric Massart added a comment - - edited

          Hi Apu, as we discussed this issue I assigned you as Peer Reviewer. I have two solutions. MDL-31622-23 for a basic handling of the strings, and:

          https://github.com/FMCorz/moodle/compare/MOODLE_23_STABLE...MDL-31622-23-2nd

          for i18n using the system locale, as the calendar system does pretty much everywhere. This patch is actually messy and I don't really like it, even though it creates consistency with the system.

          The first patch create lang strings which won't be used very often, probably only for the date selector at that stage, but that's cleaner (IMO).

          Both patches are not meant to be pushed for integration yet, but to demonstrate possible ways of doing it. (the substr() is definitely a bad approach)

          Show
          Frédéric Massart added a comment - - edited Hi Apu, as we discussed this issue I assigned you as Peer Reviewer. I have two solutions. MDL-31622 -23 for a basic handling of the strings, and: https://github.com/FMCorz/moodle/compare/MOODLE_23_STABLE...MDL-31622-23-2nd for i18n using the system locale, as the calendar system does pretty much everywhere. This patch is actually messy and I don't really like it, even though it creates consistency with the system. The first patch create lang strings which won't be used very often, probably only for the date selector at that stage, but that's cleaner (IMO). Both patches are not meant to be pushed for integration yet, but to demonstrate possible ways of doing it. (the substr() is definitely a bad approach)
          Hide
          Aparup Banerjee added a comment -

          Hi Fred,

          • have you noted moodle_setlocale() ? just noting that there are cross OS issues etc being handled there.
          • according to some quick googling, yui calendar has some built in language sets - how do we use those?
          • presently i like the '2nd' solution but i don't like that we're calculating time and substr()'s everytime. (not sure we have to or not though)

          note:have you seen the bug trail from MDL-17672 -> MDL-26391 & MDL-22625 re timing issues.

          Mainly, i think there must be a better, more supported way to do this though : maybe compatible easily with http://localize.drupal.org/translate/downloads?project=yui_calendar :-D (not sure if we already do)

          Anyway, for good measure i've added our renown language master, David Mudrak, to lend his wise opinion.

          Show
          Aparup Banerjee added a comment - Hi Fred, have you noted moodle_setlocale() ? just noting that there are cross OS issues etc being handled there. according to some quick googling, yui calendar has some built in language sets - how do we use those? presently i like the '2nd' solution but i don't like that we're calculating time and substr()'s everytime. (not sure we have to or not though) note:have you seen the bug trail from MDL-17672 -> MDL-26391 & MDL-22625 re timing issues. http://developer.yahoo.com/yui/examples/calendar/taiwan.html shows there can be more complications in this calculation with various locales having different possible settings for time offsets wrt julian calendar. (i shudder to think of gregorian or other cultures) Mainly, i think there must be a better, more supported way to do this though : maybe compatible easily with http://localize.drupal.org/translate/downloads?project=yui_calendar :-D (not sure if we already do) Anyway, for good measure i've added our renown language master, David Mudrak, to lend his wise opinion.
          Hide
          Frédéric Massart added a comment -

          Hi Apu, thanks for reviewing this.

          You are right, the second solution shouldn't be calculating those values each time. I also noticed that using substr() is probably very bad! Thinking about languages like Japanese or others using a different alphabet, one character shorter/longer could lead to a completely different word.

          Thinking about the YUI translations, I first noticed that they were missing a lot of translations. The second problem is those extra parameters. I am not sure we want people to change their calendar to another type than Gregorian for now (seeing related issues about TZ). I think the first thing we should focus on is to translate those weekdays/months names.

          Now, the last thing is that in order to have those complete translations for weekdays and months, we need 1-char, 2-chars, 3-chars and full length weekday names, which LC_TIME does not provide. As I mentioned, substr() is not a good option to shorten those words. We could also change the layout of YUI to have those 3-chars length weekdays, but that requires a bit of theming.

          All in all, having $string['monday'] = strftime('%A', 12345); in lang/langconfig.php is maybe not the worst option. Assuming that AMOS will understand it. This file already contains some 'advanced translators strings'.

          Show
          Frédéric Massart added a comment - Hi Apu, thanks for reviewing this. You are right, the second solution shouldn't be calculating those values each time. I also noticed that using substr() is probably very bad! Thinking about languages like Japanese or others using a different alphabet, one character shorter/longer could lead to a completely different word. Thinking about the YUI translations, I first noticed that they were missing a lot of translations. The second problem is those extra parameters. I am not sure we want people to change their calendar to another type than Gregorian for now (seeing related issues about TZ). I think the first thing we should focus on is to translate those weekdays/months names. Now, the last thing is that in order to have those complete translations for weekdays and months, we need 1-char, 2-chars, 3-chars and full length weekday names, which LC_TIME does not provide. As I mentioned, substr() is not a good option to shorten those words. We could also change the layout of YUI to have those 3-chars length weekdays, but that requires a bit of theming. All in all, having $string ['monday'] = strftime('%A', 12345); in lang/langconfig.php is maybe not the worst option. Assuming that AMOS will understand it. This file already contains some 'advanced translators strings'.
          Hide
          David Mudrak added a comment -

          Sorry, things like

           $string['monday'] = strftime('%A', 12345);
          

          in Moodle string files are unacceptable. Only static strings are allowed there, no PHP eval, no concatenation (and not only because of AMOS).

          WRT one-char abbreviations of week day names - note these are not an option in some languages, including English. Even two-chars might be an issue in some langs. And of course, substr() is generally bad for these things, as is noted above.

          Without looking into this issue deeply, I would suggest 1) let us try the reuse the current approach (ie using names provided by locale) as much as possible. 2) Only when at the end we find that approach too limiting, we might introduce these names as Moodle native strings and inject them into YUI. Ideally in a way that is easily maintainable during YUI upgrades (compare to what we already do for TinyMCE, for example). However, I believe we are not the first project facing this issue...

          Show
          David Mudrak added a comment - Sorry, things like $string['monday'] = strftime('%A', 12345); in Moodle string files are unacceptable. Only static strings are allowed there, no PHP eval, no concatenation (and not only because of AMOS). WRT one-char abbreviations of week day names - note these are not an option in some languages, including English. Even two-chars might be an issue in some langs. And of course, substr() is generally bad for these things, as is noted above. Without looking into this issue deeply, I would suggest 1) let us try the reuse the current approach (ie using names provided by locale) as much as possible. 2) Only when at the end we find that approach too limiting, we might introduce these names as Moodle native strings and inject them into YUI. Ideally in a way that is easily maintainable during YUI upgrades (compare to what we already do for TinyMCE, for example). However, I believe we are not the first project facing this issue...
          Hide
          Frédéric Massart added a comment -

          Thanks for your feedback David. There is a method which relates on locales, I guess that would be enough as long as we don't want to have longer/shorter weekdays/months in the calendar.

          Apu, it's up for your review .

          Show
          Frédéric Massart added a comment - Thanks for your feedback David. There is a method which relates on locales, I guess that would be enough as long as we don't want to have longer/shorter weekdays/months in the calendar. Apu, it's up for your review .
          Hide
          David Mudrak added a comment -

          I like the patch, thanks Fred.

          Show
          David Mudrak added a comment - I like the patch, thanks Fred.
          Hide
          Aparup Banerjee added a comment -

          Hi Fred,
          this look alright to me.

          i did wonder about how this isn't using any of our caching on disk. looked at langconfig.php but i'm not so familiar there. anyway spent enough time and this does look alright to push up for further integration review.

          Show
          Aparup Banerjee added a comment - Hi Fred, this look alright to me. i did wonder about how this isn't using any of our caching on disk. looked at langconfig.php but i'm not so familiar there. anyway spent enough time and this does look alright to push up for further integration review.
          Hide
          Frédéric Massart added a comment -

          Thanks for your review Apu. I guess we are not using any caching system because we are usually outputting the months/days of week by using strftime(). We would need to first use strftime() to extract the month from a timestamp and then read the cache (in most cases), which does mot make sense.

          Show
          Frédéric Massart added a comment - Thanks for your review Apu. I guess we are not using any caching system because we are usually outputting the months/days of week by using strftime(). We would need to first use strftime() to extract the month from a timestamp and then read the cache (in most cases), which does mot make sense.
          Hide
          Dan Poltawski 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
          Dan Poltawski 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
          Dan Poltawski added a comment -

          Thanks Fred, i've integrated this now.

          I'm not so keen on those bare epoch time values, I might've been tempted to use some string to time
          function, but I suppose we're saving some CPU cycles this way.

          strftime('%a', strtotime('Monday'));

          Show
          Dan Poltawski added a comment - Thanks Fred, i've integrated this now. I'm not so keen on those bare epoch time values, I might've been tempted to use some string to time function, but I suppose we're saving some CPU cycles this way. strftime('%a', strtotime('Monday'));
          Hide
          Frédéric Massart added a comment - - edited

          After discussing it with Tim, assigning the test to Andrew as I was both assignee and tester on this issue.

          Show
          Frédéric Massart added a comment - - edited After discussing it with Tim, assigning the test to Andrew as I was both assignee and tester on this issue.
          Hide
          Andrew Davis added a comment - - edited

          I'm having some trouble testing this in master.

          "Installing corresponding locales on your system `sudo locale-gen --no-archive ja_JP.UTF-8`"

          Is there any reason to do this rather than going to /admin/tool/langimport/index.php to install language packs? I've not had to modify my system outside of Moodle before just to support other languages.

          The answer to that may possibly invalidate the following results.

          I tried Bahasa Melayu, German and Arabic. No translation in Malay. The translation in German and Arabic was inconsistent. Days of the week are translated in calendar blocks but not in the calendar popup while adding an event. Either Im misunderstanding the testing instructions, I need to test this in a specific language or something is wrong. screenshots attached.

          I'll be offline for a few hours. If necessary this can be re-assigned to someone else to test.

          Show
          Andrew Davis added a comment - - edited I'm having some trouble testing this in master. "Installing corresponding locales on your system `sudo locale-gen --no-archive ja_JP.UTF-8`" Is there any reason to do this rather than going to /admin/tool/langimport/index.php to install language packs? I've not had to modify my system outside of Moodle before just to support other languages. The answer to that may possibly invalidate the following results. I tried Bahasa Melayu, German and Arabic. No translation in Malay. The translation in German and Arabic was inconsistent. Days of the week are translated in calendar blocks but not in the calendar popup while adding an event. Either Im misunderstanding the testing instructions, I need to test this in a specific language or something is wrong. screenshots attached. I'll be offline for a few hours. If necessary this can be re-assigned to someone else to test.
          Hide
          Frédéric Massart added a comment -

          Actually, the one and only place where Moodle requires some locales to be installed on the machine is when handling date translation. The calendar uses LC_TIME to print out month and week of days. To keep consistency with that behaviour, I used the same principle for the popup calendar.

          The command line that I wrote in the test instructions is to install the Japanese language pack. I don't know what is the lang code for Arabic, German is probably de_DE.UTF-8.

          Show
          Frédéric Massart added a comment - Actually, the one and only place where Moodle requires some locales to be installed on the machine is when handling date translation. The calendar uses LC_TIME to print out month and week of days. To keep consistency with that behaviour, I used the same principle for the popup calendar. The command line that I wrote in the test instructions is to install the Japanese language pack. I don't know what is the lang code for Arabic, German is probably de_DE.UTF-8.
          Hide
          Andrew Davis added a comment -

          I've installed de_DE.UTF-8 (German) and ar_QA.UTF-8 (Qatar Arabic I believe) and will see how I go now.

          Show
          Andrew Davis added a comment - I've installed de_DE.UTF-8 (German) and ar_QA.UTF-8 (Qatar Arabic I believe) and will see how I go now.
          Hide
          Andrew Davis added a comment -

          Works as described with German. I'm still having trouble with Arabic but I suspect that I haven't got it set up correctly.

          Show
          Andrew Davis added a comment - Works as described with German. I'm still having trouble with Arabic but I suspect that I haven't got it set up correctly.
          Hide
          Dan Poltawski added a comment -

          *Notice*: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26

          Congratulations

          {tracker.user.name}

          !

          You've made into Moodle

          {tracker.fixversion-1}

          +

          I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world.

          cheers!

          {tracker.friendlyintegrator}
          Show
          Dan Poltawski added a comment - * Notice *: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26 Congratulations {tracker.user.name} ! You've made into Moodle {tracker.fixversion-1} + I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world. cheers! {tracker.friendlyintegrator}
          Hide
          Matthew Davidson added a comment -

          The code changes to formlib.php are causing a regression bug. MDL-34708

          To fix it, we changed the code from:

          $config = array(array(
          'firstdayofweek' => get_string('firstdayofweek', 'langconfig'),
          'mon' => strftime('%a', 360000), // 5th Jan 1970 at 12pm
          'tue' => strftime('%a', 446400),
          'wed' => strftime('%a', 532800),
          'thu' => strftime('%a', 619200),
          'fri' => strftime('%a', 705600),
          'sat' => strftime('%a', 792000),
          'sun' => strftime('%a', 878400),
          'january' => strftime('%B', 14400), // 1st Jan 1970 at 12pm
          'february' => strftime('%B', 2692800),
          'march' => strftime('%B', 5112000),
          'april' => strftime('%B', 7790400),
          'may' => strftime('%B', 10382400),
          'june' => strftime('%B', 13060800),
          'july' => strftime('%B', 15652800),
          'august' => strftime('%B', 18331200),
          'september' => strftime('%B', 21009600),
          'october' => strftime('%B', 23601600),
          'november' => strftime('%B', 26280000),
          'december' => strftime('%B', 28872000)
          ));

          To:

          $config = array(array(
          'firstdayofweek' => get_string('firstdayofweek', 'langconfig'),
          'mon' => strftime('%a', strtotime("Monday")), // 5th Jan 1970 at 12pm
          'tue' => strftime('%a', strtotime("Tuesday")),
          'wed' => strftime('%a', strtotime("Wednesday")),
          'thu' => strftime('%a', strtotime("Thursday")),
          'fri' => strftime('%a', strtotime("Friday")),
          'sat' => strftime('%a', strtotime("Saturday")),
          'sun' => strftime('%a', strtotime("Sunday")),
          'january' => strftime('%B', strtotime("January")), // 1st Jan 1970 at 12pm
          'february' => strftime('%B', strtotime("February")),
          'march' => strftime('%B', strtotime("March")),
          'april' => strftime('%B', strtotime("April")),
          'may' => strftime('%B', strtotime("May")),
          'june' => strftime('%B', strtotime("June")),
          'july' => strftime('%B', strtotime("July")),
          'august' => strftime('%B', strtotime("August")),
          'september' => strftime('%B', strtotime("September")),
          'october' => strftime('%B', strtotime("October")),
          'november' => strftime('%B', strtotime("November")),
          'december' => strftime('%B', strtotime("December"))
          ));

          Show
          Matthew Davidson added a comment - The code changes to formlib.php are causing a regression bug. MDL-34708 To fix it, we changed the code from: $config = array(array( 'firstdayofweek' => get_string('firstdayofweek', 'langconfig'), 'mon' => strftime('%a', 360000), // 5th Jan 1970 at 12pm 'tue' => strftime('%a', 446400), 'wed' => strftime('%a', 532800), 'thu' => strftime('%a', 619200), 'fri' => strftime('%a', 705600), 'sat' => strftime('%a', 792000), 'sun' => strftime('%a', 878400), 'january' => strftime('%B', 14400), // 1st Jan 1970 at 12pm 'february' => strftime('%B', 2692800), 'march' => strftime('%B', 5112000), 'april' => strftime('%B', 7790400), 'may' => strftime('%B', 10382400), 'june' => strftime('%B', 13060800), 'july' => strftime('%B', 15652800), 'august' => strftime('%B', 18331200), 'september' => strftime('%B', 21009600), 'october' => strftime('%B', 23601600), 'november' => strftime('%B', 26280000), 'december' => strftime('%B', 28872000) )); To: $config = array(array( 'firstdayofweek' => get_string('firstdayofweek', 'langconfig'), 'mon' => strftime('%a', strtotime("Monday")), // 5th Jan 1970 at 12pm 'tue' => strftime('%a', strtotime("Tuesday")), 'wed' => strftime('%a', strtotime("Wednesday")), 'thu' => strftime('%a', strtotime("Thursday")), 'fri' => strftime('%a', strtotime("Friday")), 'sat' => strftime('%a', strtotime("Saturday")), 'sun' => strftime('%a', strtotime("Sunday")), 'january' => strftime('%B', strtotime("January")), // 1st Jan 1970 at 12pm 'february' => strftime('%B', strtotime("February")), 'march' => strftime('%B', strtotime("March")), 'april' => strftime('%B', strtotime("April")), 'may' => strftime('%B', strtotime("May")), 'june' => strftime('%B', strtotime("June")), 'july' => strftime('%B', strtotime("July")), 'august' => strftime('%B', strtotime("August")), 'september' => strftime('%B', strtotime("September")), 'october' => strftime('%B', strtotime("October")), 'november' => strftime('%B', strtotime("November")), 'december' => strftime('%B', strtotime("December")) ));
          Hide
          Dan Poltawski added a comment -

          Fred, in case you didn't see - MDL-34708

          Show
          Dan Poltawski added a comment - Fred, in case you didn't see - MDL-34708
          Hide
          Frédéric Massart added a comment -

          Thanks Dan, I guess trying to minimize unnecessary processing was a bad idea here. Thanks for the report and the patch Matthew.

          Show
          Frédéric Massart added a comment - Thanks Dan, I guess trying to minimize unnecessary processing was a bad idea here. Thanks for the report and the patch Matthew.

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: