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:

      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

        Gliffy Diagrams

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

            Seems ok to me, thanks.

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