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:
        • Pull Master Diff URL:
        • 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

            has been marked as being related by

            Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-36212 Some forum mails not sent because of wrong html entity decoding

            • Major - Major loss of function, incorrect output
            • Closed

              Activity

              Ascending order - Click to sort in descending order
              Hide
              Permalink
              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
              Permalink
              Petr Škoda added a comment -

              Seems ok to me, thanks.

              Show
              Petr Škoda added a comment - Seems ok to me, thanks.
              Hide
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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: