Hello Tim
If you can have a look at this. I think it is ready for peer-review.
Of course, this is really minor and I know you are taking some well deserved holidays. So better to wait your return !
The unit tests are OK and I tested to import some files with images with no apparent problem.

I'll leave this to Tim to review, but it would be good if you could add some testing instructions.

Tim is away until the end of the week, so he might not respond for a while.

Hello Michael,
Thanks for looking at this issue.
Yes I know Tim is away until the end of the week, but as this is really a minor issue, this can wait.
I wanted to first have Tim's comments before doing branches for Moodle 2.3 and moodle 2.2 and adding testing instructions.
Testing will be to verify that I didn't created regressions as there is no new functionnalities only code cleaning.
But we need to ensure I didn't break anything in question bank and lesson import (the fact that the unit tests I did for the blackboard_six import format are still OK is good, but it is not enough to ensure this change is working)

+1 from me.

Please cherry-pick and add testing instructions, then this can be submitted for integration.

Cherry picked to MOODLE_22_STABLE (without the unit tests) and MOODLE_23_STABLE.
I will add some testing instructions. I wonder how much of the testing process of MDL-25492 and MDL-35147 I should ask to redo here ? We sure need to test that image import in question bank is not broken, but this change also has the potential to create regressions in all question bank and lesson import as mod/lesson format.php and question/format.php are changed ? Maybe it's enough if we test that some fixtures files import is still OK in question bank import and in lesson import ?

Thanks Jean-Michel.

Rebased all branchs. I am a little worried this didn't get into 2.2.6 and 2.3.3.

Hi Jean-Michel/Tim,

Is this improvement safe for back-porting to the stable branches? It looks to me that some non-core format could potentially be relying on this behaviour?

I wonder if it'd be safer to go master only on this to be more cautious. What do you think?

Hello Dan,
Thanks for having a look at this.
Yes I think it is quite safe because it only correct code I introduced myself a few months ago, so this is really a bugfix not a new feature.
In fact as you can see from the corrections in lesson I am in fact removing (not adding) some changes done when I allowed question formats to import images directly back in July-August 2011.
When I did this changes I allowed the question structure questiontext and generalfeedback fields to be either strings or arrays. As Eloy pointed out this was not very good. This change revert them to be strings in all cases as they have been for years.
The only way I see it could break something is the very unlikely case where somebody has modified a non core format after last summer to use the mechanism I introduced, but :

• I don't know of any question import format (no one in the plugin database, and no discussion about one in the quiz forum) that import images apart the ones I am currently working on myself : my work to fix the webct import format in MDL-30001 and a "gift with media files" new format see https://github.com/jmvedrine/moodle-qformat_giftmedia. Both are based on the fact that MDL-36243 is integrated, so if it is only integrated in master this will really be a pain for me and Moodle users because no Moodle user would be able to use these formats before June 2013 !!
• question import formats is (sadly) a very quiet area of Moodle : a lot of formats had been broken for years when I began working on them and very few Moodle developpers work on that area
• This change in the code was left unpublished and no mention of it was added to the readme file (sorry !) so a developer would have to look in the code to use it. As I said very unlikely
• If a third party format is using this behaviour, it's only a small job (look at the changes in question/format/blackboard_six/formatqti.php or question/format/blackboard_six/formatpool.php) and a few lines of code to make them use the new behaviour

So please consider integrating this bugfix in all branchs. Thanks a lot.

+1 from me also. Please backport if you are happy to do so.

Thanks for clarifying.

I've integrated this to master, 24, 23 and 22.

Thanks Tim, Thanks Dan

Works as expected. Passing. Thanks for another great contribution Jean-Michel.

Thanks Mark

Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

Closing, ciao