Course name in frontpage not applying filters

[izendegi]

We've detected that course names aren't correctly shown when using this theme, because there is a filter notation used in there and that filter isn't applied there.

That only happens on the frontpage, so this issue may be related to the "trim course title" setting, but not sure of that as long as it cannot be disabled.

We're using Moodle 3.5.0 and Fordson theme v3.5 release 1 (2018051700)

[dbnschools]

I think this is because I send the text to be sanitized first before it is displayed so that it removes any html and returns clean text.

[dbnschools]

Is this the course title or is it the course summary?

[izendegi]

Actually it happens in both the course fullname and the course summary.

I've tried with shorter course titles to avoid the "trim course title" setting and it still happens, so that's not related.

Maybe the filters must be applied before sanitizing the text?

[izendegi]

Hi again,

Our intention is to use your great theme in a multilingual Moodle site (basque, spanish and english), and to do so we use the multilingual filters all the site around.

I've tested the theme more deeply and found that (altough most of the fields apply the filters correctly) this filter issue also happens in some other theme fields, at least in these ones:

createbuttontext slideiconbuttontext modchoosercustomlabel

So the issue may be renamed to "Some fields not applying the filters correctly"

[dbnschools]

So I looked over the core_renderer file and it appears when we grab the value we apply format_text which seems to strip out any html or other code. Most of this is in the wonderbox function https://github.com/dbnschools/moodle-theme_fordson/blob/master/classes/output/core_renderer.php#L584

If you remove the format_text and the "( )" in each instance then it should allow your text filter to work. I'm evaluating doing this for the next release but I seem to recall a reason to not do this.

[dbnschools]

So... I think I need to change all the format_text lines to look like this: format_text($PAGE->theme->settings->fptextbox, FORMAT_HTML, array('noclean' => true));

I'm just following this stuff here: https://docs.moodle.org/dev/Output_functions

[dbnschools]

Try changing the core_renderer file and see if it allows your language filter to work.

[izendegi]

Hi,

I've been testing that I it seems like the fix is just the other way around, instead of removing the format_text() the issue was that the format_text function wasn't called on those two strings:

https://github.com/izendegi/moodle-theme_fordson/compare/master...Filter_issue?quick_pull=1#diff-0e8f7db13244d56fdb07ade2c35167f1

I've taken a look to the modchooser.php file and tried to fix the modchoosercustomlabel string too but it seems like there is something more needed there.

[dbnschools]

So I should add format_text to all the strings? I don't use the filter so I am not sure what is going to make it work for you.

[izendegi]

So I should add format_text to all the strings? I don't use the filter so I am not sure what is going to make it work for you.

I'm not sure, but I think so. You can use filters 1 in any customizable text in Moodle, and it seems like those filters are only applied when calling format_text.

In our case we're using the Multilang2 filter 2, and we use it to show the text depending on the language the user is navigating. For example, in the slideiconbuttontext field we set this

{mlang eu}Laguntza{mlang}{mlang es}Ayuda{mlang}{mlang en}Help{mlang}

and when the filter is applied it displays Laguntza, Ayuda or Help depending on the user navigation language. When the filter is not applied the string is displayed as shown above.

By calling format_text the problem gets solved in createbuttontext and slideiconbuttontext, but it doesn't in modchoosercustomlabel, I donn't know what else needs to be done there.

[dbnschools]

OK. This better clarifies things. Let me take a look later today and I will see what can be done.

[dbnschools]

Look in file classes/output/modchooser.php line 87. $PAGE->theme->settings->modchoosercustomlabel to format_text($PAGE->theme->settings->modchoosercustomlabel)) Add the format_text to this and see if it solves the activity chooser issue. If that is the case I will include all these fixes for the next release of Fordson.

[dbnschools]

Actually, this will work:
if (count($commonlyused)) { $sections[] = new chooser_section('commonlyused', new lang_string('modchoosercommonlyusedtitle', 'theme_fordson', format_text($PAGE->theme->settings->modchoosercustomlabel, FORMAT_HTML, array('noclean' => true))) , array_map(function ($module) use ($context) { return new modchooser_item($module, $context); } , $commonlyused)); }

[dbnschools]

After further testing I came up with a better way to handle the activity chooser text. it involves a new language string and a few changes to the Modchooser class. If no custom label text was entered then the label defaulted to {$a}. That is not pretty.
So I made a default and then a check to see if the label wasn't empty which applies the custom label.

[izendegi]

Yes, that change did work!

And I agree with you, there should be a default value to avoid that {$a} showing up when that field is empty. And furthermore, that way when the field is empty there string would show automatically in the users' language (once that string is translated in AMOS), even without using the multilingual filter.

Great work, thanks a lot!

[dbnschools]

Great! I plan on pushing the changes up to github and then moodle.org next week.

[izendegi]

Hi!

I've already tested the new version and realized the filter issue has been fixed. Anyway, the changes you made on the activity chooser introduced another issue: when modchoosercustomlabel field is left empty everything works fine, but when it's filled in the same list shows up twice, one with the modchoosercustomlabel title and another one modchoosercommonlyusedtitle title:

P.S.: I answer here because I don't know if another issue should be created for this.

[dbnschools]

OK. I'll take a look.

[dbnschools]

Found the issue straight away. I forgot to set the condition if it was empty. Line 93 on classes/output/modchooser.php where if (count($commonlyused)) { change to: if (count($commonlyused)&& empty($PAGE->theme->settings->modchoosercustomlabel)) {

[dbnschools]

New update will be sent out today.

[izendegi]

That's it, I've tested that patch and worked like a charm.

[izendegi]

Anyway, the first reason of opening this issue (filters not being applied on the frontpage course list's titles and summaries) remained unsolved, but I've been able to patch that.

I've added to the 497 and 498 lines of /classes/output/core/course_renderer.php the format_text function:

$summary = theme_fordson_course_trim_char(format_text($summary), $trimsummaryvalue); $trimtitle = theme_fordson_course_trim_char(format_text($course->fullname), $trimtitlevalue);

[dbnschools]

Thanks. I updated these lines as well. There were 2 spots where this was used and I replaced them both.

[dbnschools]

izendegi, It seems like this can cause an issue in certain situations. I'm still looking into it and will try to have this included in the core theme.

[dbnschools]

Would this work with your filter?

$summary = format_text(theme_fordson_course_trim_char($summary, $trimsummaryvalue));
$trimtitle = format_text(theme_fordson_course_trim_char($course->fullname, $trimtitlevalue));

[izendegi]

I've tried your proposal and it only works if the string is not trimmed, because when the string is trimmed the filter code breaks.

I think it may be easier to understand with an example:

A string using the filter_multilang2 has this shape: {mlang eu}Adibidea{mlang} {mlang en}Example{mlang}

In this case, it should display "Adibidea" when the user is navigating in Basque (eu) and "Example" when navigating in English (en), but if we trim it to 30 characters before applying the filter we get {mlang eu}Adibidea{mlang} {mla and that way the filter gets broken.

The way I proposed, you first get the filter applied (the string becomes "Adibidea" or "Example") and after that you trim it to the characters you want to.

[izendegi]

I've been doing more testing and I've seen that in some places using format_string instead of format_text solves some issues, such as tooltip texts.

I've changed lines like this $tooltiptext = 'data-tooltip="tooltip" data-placement= "top" title="' . $course->fullname . '"';

with this one: $tooltiptext = 'data-tooltip="tooltip" data-placement= "top" title="' . format_string($course->fullname) . '"';

Was this the issue you detected or it was something else?

[dbnschools]

Great find. I am sorry that I didn't see this sooner. I must have missed this while out on vacation and I was just circling back around today on github. I changed out all the instances of this in course_renderer.php. It will be in Update 11 of Fordson.