Moodle
  1. Moodle
  2. MDL-36942

XML import of questions can introduce an extra space at the start of each line

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.2.7, 2.3.4, 2.4.1
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      1. Create a new question category.

      2. In that category, create a question containing some pre-formatted text that spans several lines.

      3. Export that question category as Moodle XML.

      4. Import that file into a new course.

      5. Make sure the question text, and other fields, have not had extra spaces added at the start of each line.

      Show
      1. Create a new question category. 2. In that category, create a question containing some pre-formatted text that spans several lines. 3. Export that question category as Moodle XML. 4. Import that file into a new course. 5. Make sure the question text, and other fields, have not had extra spaces added at the start of each line.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      46483

      Description

      When importing a question in Moodle XML format, where the question contains multiline (non-html) text fields, an extra space is inserted at the start of every line. For example, if the xml description of a question contains a tag like

      <text>line1
      line2
      line3
      </text>

      the imported value of that text field will be "line1\n line2\n line3\n " (note the extra spaces). The cause is the first line in the function readquestions in file question/format/xml/format.php: "$text = implode($lines, ' ');" which is intended to combine all the file lines into one big string, but does so with an extra space introduced at the start of each line.

        Activity

        Hide
        Tim Hunt added a comment -

        Looking back, it has been that way for a very long time: https://github.com/moodle/moodle/blame/84769fd8/question/format/xml/format.php#L232. That is from 2006, and the code was probably like that even earlier.

        We should probably change it, but how to we make sure that changing it does not break anything?

        Show
        Tim Hunt added a comment - Looking back, it has been that way for a very long time: https://github.com/moodle/moodle/blame/84769fd8/question/format/xml/format.php#L232 . That is from 2006, and the code was probably like that even earlier. We should probably change it, but how to we make sure that changing it does not break anything?
        Hide
        Tim Hunt added a comment -

        Thanks Richard. Having thought about it some more, there will always be a \n at the end of each line, so there is no need to add the space. Therefore, I have submitted this change for integration.

        Show
        Tim Hunt added a comment - Thanks Richard. Having thought about it some more, there will always be a \n at the end of each line, so there is no need to add the space. Therefore, I have submitted this change for integration.
        Hide
        Richard Lobb added a comment -

        Thanks Tim, sounds good. The only potential problem area I can see is if someone has implemented a workaround in their import code (which I confess I nearly did) to delete the space after every newline. But best to force any such hacks to get fixed, anyway.

        Show
        Richard Lobb added a comment - Thanks Tim, sounds good. The only potential problem area I can see is if someone has implemented a workaround in their import code (which I confess I nearly did) to delete the space after every newline. But best to force any such hacks to get fixed, anyway.
        Hide
        Sam Hemelryk added a comment -

        Thanks Tim, this has been integrated now

        Show
        Sam Hemelryk added a comment - Thanks Tim, this has been integrated now
        Hide
        Adrian Greeve added a comment -

        Tested on the 2.2, 2.3 and master integration branches.
        The extra space has now been removed.
        Test passed.

        Show
        Adrian Greeve added a comment - Tested on the 2.2, 2.3 and master integration branches. The extra space has now been removed. Test passed.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now! Closing, ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: