Just confirm that in forum/post.php the double format_string were actually not useful, except that everything is good

Hi Jerome,

I think it is not necessary to have format_string for each string. I tried both with double format_string and removing them all, it is outputting the same result.

Thank you for reviewing.

Submitting for integration review.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

Hi Rosie,

Thanks for making these changes, things look 95% there, but I did note a coding error in mod/feedback/mapcourse.php.
Could you please make the following two changes:

1. Fix the coding errors in mod/feedback/mapcourse.php
2. Improve the test instructions so that the tester has an indication of each page that needs to be viewed, presently the tester would likely just visit the module frontpage and call it passed leaving coding errors like the above going unnoticed.

Cheers
Sam

Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

Hi Sam,

1. Fixed the patch that caused by an extra bracket.
2. Improved the testing instructions.

Thanks.

Sending for integration review.

Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

Thanks Rossi, this is a very interesting fact that you've spotted about double formatting when using those functions.

But I'm not 100% sure that I agree with your solution.
For the start, not every heading needs to be formatted, examples:
$PAGE->set_title(get_string(...)) - does not need any formatting

Also when title consists of course/module/whatever name PLUS some string, for example:
$PAGE->set_heading(format_string($course->fullname, true, array('context' => $coursecontext)) . ': ' . $hdr);

Another example is when we need non-page context. For example, course name on the module page should be formatted to course context but the page context is module, so calling format_text() without arguments is not correct (and this is what set_title() is doing).

Also to note that there are many places that you did not change, because grep would not show them, for example:
https://github.com/moodle/moodle/blob/master/badges/newbadge.php#L52..L54

BTW I have a plugin for testing format_string() / format_text()
https://github.com/marinaglancy/moodle-filter_testcontext

What I would suggest to do is (for master only): add argument to the function set_title() and set_heading() specifying whether the string is already formatted (default to false). And use it wisely.
Maybe we should not grep all usages of set_title/set_heading but instead go by component and correct all wrong formatting in headings/navigation.

Please tell me what you think.

Hi Marina,

I'm just following Eloy's suggestion from MDL-41797.

I agree on fixing the set_title() and set_heading() for master only and I think it should be address as separate issue.

so for this issue, it should be for 2.5 and 2.6 only.

Sorry, Rossi, we can not integrate code in stables only. This way it can be very easily forgotten to amend master. And especially since you will be leaving us soon.
Most of the changes in this branch can go to master as well. There are just string concatenations that need other approach.

I have a suggestion to you: how about you leave in this branch only cases without string concatenations, such as "set_title(format_string(XXX))"
The rest we will convert in master properly.

Updated all patches to leave format_string() for concatenation strings.

Thanks Marina

Thanks Rossi, integrated in 2.5, 2.6 and master

Thanks for fixing this Rossie,

No errors/warning appeared on listed modules. Works fine.

Passing...

This weeks weekly release is now available and includes your code.
A big pat on the back to you again for once more being a cog in the Moodle machine.

Best wishes, the Moodle integration team.