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 Master Branch:
      MDL-36093-master

      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.

        Gliffy Diagrams

          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 Skoda added a comment -

            works, thanks

            Show
            Petr Skoda 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: