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:

      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.

        Gliffy Diagrams

          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: