Moodle
  1. Moodle
  2. MDL-36093

Non ASCII chars in string 'labelsep' are printed litteraly in export files

    Details

    • Testing Instructions:
      Hide
      1. Go to "Site administration > Language > Language customisation", open language pack and update string 'labelsep' in 'langconfig.php' to value ': '
      2. Create a course and add some students and activities (assignment, forums, quiz etc).
      3. Go to the course administration "Grade administration > Export > Excel spreadsheet" and assert that there's no ' ' printed literally for each 'labelsep' in the column name of each grade item, both in the preview and in the exported file (ex: Assignment: Assignment 1).
      4. Do the same for "OpenDocument spreadsheet" and "Plain text file" exports (column names are not outputted in "XML file").
      Show
      Go to "Site administration > Language > Language customisation", open language pack and update string 'labelsep' in 'langconfig.php' to value ': ' Create a course and add some students and activities (assignment, forums, quiz etc). Go to the course administration "Grade administration > Export > Excel spreadsheet" and assert that there's no ' ' printed literally for each 'labelsep' in the column name of each grade item, both in the preview and in the exported file (ex: Assignment: Assignment 1). Do the same for "OpenDocument spreadsheet" and "Plain text file" exports (column names are not outputted in "XML file").
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
      git@github.com:StudiUM/moodle.git
    • Pull 2.4 Branch:
      MDL-36093-moodle24
    • Pull 2.5 Branch:
      MDL-36093-moodle25
    • Pull Master Branch:
      MDL-36093-master
    • Rank:
      44854

      Description

      When exporting grades, the string 'labelsep' (defined in 'langconfig') is used for columns labelling, which leads to weird columns labels, depending on the value of the string in the lang pack.

      E.g. in french, 'labelsep' is defined with a non-breaking space followed by a column. This results in following column label in CSV grades export file: "Devoir Nom du devoir" (entity is printed litteraly, not interpreted).

      Possible solutions:

      1. Remove 'labelsep' from functions exporting files and educate developers not to re-introduce it (workaround), or
      2. Define a new string 'labelsepfile' (or whatever) to be used in exporting functions (another workaround), or
      3. Find a way so that 'labelsep' non ASCII chars are correctly output in the export files (better).

      AFAIK, the only occurence of this problem as of today is in file grade/export/lib.php, in the function 'format_column_name', line 183.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting this.

          Feel free to help implement a solution.

          Show
          Michael de Raadt added a comment - Thanks for reporting this. Feel free to help implement a solution.
          Hide
          Jean-Philippe Gaudreau added a comment -

          Hi Nicolas,

          I dug this issue and I think the problem is not in the code but in the french (fr) language package that contains html special characters.

          Here's what the string 'labelsep' value is in the langconfig.php file :

          $string['labelsep'] = ' ';
          

          If you compare to the other packages, it should clearly be written this way :

          $string['labelsep'] = ' ';
          

          Or even this way if you want to be more similar to most of the other languages :

          $string['labelsep'] = ' : ';
          

          I tested all of that by manually modifying the string in the french language package and everything works as expected.

          IMO this bug should be closed 'Won't fix' and french string 'labelsep' modified with value ' ' or ' : ' in AMOS.

          I'll wait until tomorrow to see if anyone has an objection about that.

          Thx!

          Show
          Jean-Philippe Gaudreau added a comment - Hi Nicolas, I dug this issue and I think the problem is not in the code but in the french (fr) language package that contains html special characters. Here's what the string 'labelsep' value is in the langconfig.php file : $string['labelsep'] = ' '; If you compare to the other packages, it should clearly be written this way : $string['labelsep'] = ' '; Or even this way if you want to be more similar to most of the other languages : $string['labelsep'] = ' : '; I tested all of that by manually modifying the string in the french language package and everything works as expected. IMO this bug should be closed 'Won't fix' and french string 'labelsep' modified with value ' ' or ' : ' in AMOS. I'll wait until tomorrow to see if anyone has an objection about that. Thx!
          Hide
          Jean-Philippe Gaudreau added a comment -

          If the community insists on replacing the '&...' html special characters, I could fix it by using the html_to_text function in lib/weblib.php when writing column names to file.

          Show
          Jean-Philippe Gaudreau added a comment - If the community insists on replacing the '&...' html special characters, I could fix it by using the html_to_text function in lib/weblib.php when writing column names to file.
          Hide
          Nicolas Martignoni added a comment -

          Hi Jean-Philippe,

          This is clearly the problem. The string labelsep is used for 2 different uses:

          1. To separate a label from the value assigned to the label, e.g. "Height of box: 12px" where "Height of box" is the label, ": " is labelsep and "12px" is the value. In this case, the typography is important. The french typographically correct way to enter the colon is with a non-breaking space to separate the text and the punctuation after it (this is the case for :;!? chars too, see https://fr.wikipedia.org/wiki/Ponctuation#Les_espaces_autour_des_signes_de_ponctuation_.28r.C3.A8gles_actuelles.29). Hence the correct definition in langconfig.php. We really want a non breaking space before the colon, because without it, there's great chance that the colon goes to the next line when the space on screen is tight.
          2. To separate columns in export files. In this case, typography is not relevant and we should not have any space before or after the colon (IMHO).

          Just close this as "wont fix" will bring us to the situation that lead to open this bug report, that is french people will get bad typography on every Moodle page that uses labelsep, which would indeed be bad.

          Your suggested solution with html_to_text would be indeed ideal.

          Thanks for your thoughts, and cheers!

          Show
          Nicolas Martignoni added a comment - Hi Jean-Philippe, This is clearly the problem. The string labelsep is used for 2 different uses: To separate a label from the value assigned to the label, e.g. "Height of box: 12px" where "Height of box" is the label, ": " is labelsep and "12px" is the value. In this case, the typography is important. The french typographically correct way to enter the colon is with a non-breaking space to separate the text and the punctuation after it (this is the case for :;!? chars too, see https://fr.wikipedia.org/wiki/Ponctuation#Les_espaces_autour_des_signes_de_ponctuation_.28r.C3.A8gles_actuelles.29 ). Hence the correct definition in langconfig.php. We really want a non breaking space before the colon, because without it, there's great chance that the colon goes to the next line when the space on screen is tight. To separate columns in export files. In this case, typography is not relevant and we should not have any space before or after the colon (IMHO). Just close this as "wont fix" will bring us to the situation that lead to open this bug report, that is french people will get bad typography on every Moodle page that uses labelsep, which would indeed be bad. Your suggested solution with html_to_text would be indeed ideal. Thanks for your thoughts, and cheers!
          Hide
          Jean-Philippe Gaudreau added a comment -

          Hi Nicolas,

          Ok I understand the situation and you are right. Going foward with the 'html_to_text' solution.

          Thx!

          Show
          Jean-Philippe Gaudreau added a comment - Hi Nicolas, Ok I understand the situation and you are right. Going foward with the 'html_to_text' solution. Thx!
          Hide
          Nicolas Martignoni added a comment -

          Thank YOU for your work!

          Show
          Nicolas Martignoni added a comment - Thank YOU for your work!
          Hide
          Helen Foster added a comment -

          Nicolas, thanks for your comments, and Jean-Philippe, thanks for working on the issue.

          Just wondering whether our expert in all things language-related David Mudrak has any thoughts about it?

          Show
          Helen Foster added a comment - Nicolas, thanks for your comments, and Jean-Philippe, thanks for working on the issue. Just wondering whether our expert in all things language-related David Mudrak has any thoughts about it?
          Hide
          Frédéric Massart added a comment -

          Thank you guys, I also think that this is the adequate solution. We should then educate developers to remember labelsep contains HTML entities.

          Pushing for integration.

          Cheers!
          Fred

          Show
          Frédéric Massart added a comment - Thank you guys, I also think that this is the adequate solution. We should then educate developers to remember labelsep contains HTML entities. Pushing for integration. Cheers! Fred
          Hide
          Dan Poltawski added a comment -

          Thanks Jean-Philippe, integrated to master, 25 and 24.

          Show
          Dan Poltawski added a comment - Thanks Jean-Philippe, integrated to master, 25 and 24.
          Hide
          Petr Škoda added a comment -

          works, thanks

          Show
          Petr Škoda added a comment - works, thanks
          Hide
          Dan Poltawski added a comment -

          Congratulations! This change has been integrated upstream and is now available from our git and download mirrors. To celebrate, here is a joke:

          A SQL query goes into a bar, walks up to two tables and asks, "Can I join you?"

          Show
          Dan Poltawski added a comment - Congratulations! This change has been integrated upstream and is now available from our git and download mirrors. To celebrate, here is a joke: A SQL query goes into a bar, walks up to two tables and asks, "Can I join you?"

            People

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

              Dates

              • Created:
                Updated:
                Resolved: