Moodle

question format learnwise is not utf-8 compatible

Details

  • Type: Bug Bug
  • Status: Closed 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:

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 - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

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

Vote (0)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: