Moodle
  1. Moodle
  2. MDL-37068

question format learnwise is not utf-8 compatible

    Details

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

      1. Find a test Learnwise format file. (The good news is that there is one in the code at question/format/learnwise/learnwise-example.xml)

      2. Import this into the question bank. Verify that there are no errors, and that the imported questions work.

      3. Now, the original bug report was something about HTML entities, and UTF-8, so if you are feeling brave, edit the example file to add some tricky examples of those, and test the import again.

      Show
      1. Find a test Learnwise format file. (The good news is that there is one in the code at question/format/learnwise/learnwise-example.xml) 2. Import this into the question bank. Verify that there are no errors, and that the imported questions work. 3. Now, the original bug report was something about HTML entities, and UTF-8, so if you are feeling brave, edit the example file to add some tricky examples of those, and test the import again.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      46621

      Description

      Decide what is the expected encoding of learnwise files and instead of qformat_learnwise->unhtmlentities() use textlib::entities_to_utf8() after converting the strings to utf-8.

      See MDL-36212 for more information

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          Jean-Michel, in your role as master of all things import/export, would you mind taking a look at this change? Thanks.

          I am about to add testing instructions.

          If all is OK, I will backport to stable branches.

          Show
          Tim Hunt added a comment - Jean-Michel, in your role as master of all things import/export, would you mind taking a look at this change? Thanks. I am about to add testing instructions. If all is OK, I will backport to stable branches.
          Hide
          Petr Škoda added a comment -

          Seems ok to me, thanks.

          Show
          Petr Škoda added a comment - Seems ok to me, thanks.
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Tim,
          Sure. Will look at this.

          Show
          Jean-Michel Vedrine added a comment - Hello Tim, Sure. Will look at this.
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Tim,
          Results of the review
          In fact Tim's branch fix a lot of separate issues in the learnwise format wich was badly broken (is it still used ? Nobody seems to report the problems)

          • this format wasn't accepting xml files because it missed the needed method FIXED
          • this format produced numerous notices: Undefined offset: 1 because the foreach ($optionlist as $option) loop was trying to process empty choices FIXED
          • As all other formats after Moodle 2.0 migration the import was dying with an Error writing to database because answers and feedbacks were strings and not arrays so the answerformat and feedbackformat values were missing when writing to database FIXED
          • finally as explained in MDL-36212 there was a problem with the use of get_html_translation_table(HTML_ENTITIES); in the unhtmlentities function with certain php versions. I have been unable to replicate the problem but as Petr says it is OK and he is a lot more qualified than me on textlib use, consider as FIXED

          The only remaining bit I saw is the use of a hardcoded string in line 134, but unfortunately search reveals other such hardcoded strings in some other formats (examview, webct, missing word, xhtml). So maybe it's better open a new tracker issue ?

          But I think we can consider learnwise format as fixed and running. I hope someone is using it and Tim's work will be usefull

          Show
          Jean-Michel Vedrine added a comment - Hello Tim, Results of the review In fact Tim's branch fix a lot of separate issues in the learnwise format wich was badly broken (is it still used ? Nobody seems to report the problems) this format wasn't accepting xml files because it missed the needed method FIXED this format produced numerous notices: Undefined offset: 1 because the foreach ($optionlist as $option) loop was trying to process empty choices FIXED As all other formats after Moodle 2.0 migration the import was dying with an Error writing to database because answers and feedbacks were strings and not arrays so the answerformat and feedbackformat values were missing when writing to database FIXED finally as explained in MDL-36212 there was a problem with the use of get_html_translation_table(HTML_ENTITIES); in the unhtmlentities function with certain php versions. I have been unable to replicate the problem but as Petr says it is OK and he is a lot more qualified than me on textlib use, consider as FIXED The only remaining bit I saw is the use of a hardcoded string in line 134, but unfortunately search reveals other such hardcoded strings in some other formats (examview, webct, missing word, xhtml). So maybe it's better open a new tracker issue ? But I think we can consider learnwise format as fixed and running. I hope someone is using it and Tim's work will be usefull
          Hide
          Jean-Michel Vedrine added a comment -

          Sorry, I forgot to start peer review before writting my comments !

          Show
          Jean-Michel Vedrine added a comment - Sorry, I forgot to start peer review before writting my comments !
          Hide
          Jean-Michel Vedrine added a comment -

          Next step of the workflow : finishing peer review !

          Show
          Jean-Michel Vedrine added a comment - Next step of the workflow : finishing peer review !
          Hide
          Jean-Michel Vedrine added a comment -

          Sorry I forgot to report a typo in the lang file plugidnname_help for pluginname_help this is why the help icon is not displayed.
          I think you will need to change version after correction ?

          Show
          Jean-Michel Vedrine added a comment - Sorry I forgot to report a typo in the lang file plugidnname_help for pluginname_help this is why the help icon is not displayed. I think you will need to change version after correction ?
          Hide
          Tim Hunt added a comment -

          Thanks Jean-Michel. Submitting for integration now.

          To INTEGRATORS: Please cherry-pick to 2.4.

          Show
          Tim Hunt added a comment - Thanks Jean-Michel. Submitting for integration now. To INTEGRATORS: Please cherry-pick to 2.4.
          Hide
          Tim Hunt added a comment -

          About the version number thing. You are right that if you change a string / JS / CSS in a contrib plug-in, then you have to change the version number. You don't have to do that with fixes that are to be integrated into Moodle core, because they increase the main version number for each weekly release.

          Show
          Tim Hunt added a comment - About the version number thing. You are right that if you change a string / JS / CSS in a contrib plug-in, then you have to change the version number. You don't have to do that with fixes that are to be integrated into Moodle core, because they increase the main version number for each weekly release.
          Hide
          Sam Hemelryk added a comment -

          Thanks Tim, all looked good and has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Tim, all looked good and has been integrated now.
          Hide
          Rajesh Taneja added a comment -

          Thanks for fixing this Tim,

          Was able to import learnwise format file in question bank (original and modified learnwise-example.xml)

          Show
          Rajesh Taneja added a comment - Thanks for fixing this Tim, Was able to import learnwise format file in question bank (original and modified learnwise-example.xml)
          Hide
          Eloy Lafuente (stronk7) added a comment -

          And your fantastic code has met core, hope they become good friends for a long period.

          Closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - And your fantastic code has met core, hope they become good friends for a long period. Closing, thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: