Moodle

Multilang tags appear in activity completion tips

Details

Description

When using the multilang filter (http://docs.moodle.org/en/Multi-language_content_filter) with activity completion, the span tags used in activity names are present in the tip that appears when hovering over an activity completion checkbox.

Replication steps:

  1. Log in as admin
  2. Navigate to Site admin > Plugins > Filters > Manage filters
  3. Set the Multi-language content filter to On and allow it to apply to "Content and headings"
  4. Navigate to Site admin > Advanced features
  5. Check "Enable completion tracking" (enablecompletion)
  6. Navigate to a course
  7. Edit the course's settings and in the Student progress section, set Completion tracking to "Enabled,..."
  8. Save settings
  9. Turn editing on
  10. Create an activity such as a quiz.
  11. Add the following as the activity name: <span lang="en" class="multilang">This is a test</span><span lang="de" class="multilang">Dies ist ein Test</span>
  12. In the Activity completion settings section, set the Completion tracking option to "Students can manually mark..."
  13. Save the activity and return to the course
  14. Turn editing off
  15. Hover over the completion checkbox for the newly created quiz

Expected result: The tip shows "Mark as complete:" followed by the filtered name

Actual result: The name supplied with tags and both translations appears

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Michael de Raadt added a comment -

Probably a bit of a fringe case.

Show
Michael de Raadt added a comment - Probably a bit of a fringe case.
Hide
Permalink
Sam Marshall added a comment -

Here's a fix for review.

In addition to fixing the problem on the course page, I realised the same thing would happen in the activity completion report, so I fixed that too.

Show
Sam Marshall added a comment - Here's a fix for review. In addition to fixing the problem on the course page, I realised the same thing would happen in the activity completion report, so I fixed that too.
Hide
Permalink
Andrew Davis added a comment -

Looks good. I'm guessing this bug almost certainly exists in both 2.2 and 2.1. I'm fairly certain that context_module::instance($mod->id) doesnt exist in 2.2 or 2.1 so unfortunately you wont be able to do a straight cherry-pick.

Show
Andrew Davis added a comment - Looks good. I'm guessing this bug almost certainly exists in both 2.2 and 2.1. I'm fairly certain that context_module::instance($mod->id) doesnt exist in 2.2 or 2.1 so unfortunately you wont be able to do a straight cherry-pick.
Hide
Permalink
Sam Marshall added a comment -

context_*::instance is in 2.2, just not 2.1. As this is only a minor bug I suggest leaving it broken in 2.1. Will submit for integration on 2.2 and master shortly.

Show
Sam Marshall added a comment - context_*::instance is in 2.2, just not 2.1. As this is only a minor bug I suggest leaving it broken in 2.1. Will submit for integration on 2.2 and master shortly.
Hide
Permalink
Sam Marshall added a comment -

also - thanks for review

Show
Sam Marshall added a comment - also - thanks for review
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Uhm.. I know it's an small inconsistency... but.. who decides if one activity title is processed with the multilang filter, the activity filters or the course filters?

I think it's the former, but in the report you're using the later...

Holding this a bit...

Show
Eloy Lafuente (stronk7) added a comment - Uhm.. I know it's an small inconsistency... but.. who decides if one activity title is processed with the multilang filter, the activity filters or the course filters? I think it's the former, but in the report you're using the later... Holding this a bit...
Hide
Permalink
Sam Marshall added a comment -

OK, it didn't already have it and looks like I was too lazy to make it get the right context in report/progress I'll do an updated version shortly.

Show
Sam Marshall added a comment - OK, it didn't already have it and looks like I was too lazy to make it get the right context in report/progress I'll do an updated version shortly.
Hide
Permalink
Sam Marshall added a comment -

I added a line to get the modcontext and rebased both branches on current master (should be == 23) and 22. Also retested (the master version), still seems to work.

How about that?

Show
Sam Marshall added a comment - I added a line to get the modcontext and rebased both branches on current master (should be == 23) and 22. Also retested (the master version), still seems to work. How about that?
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Perfect, thanks. Proceeding...

Show
Eloy Lafuente (stronk7) added a comment - Perfect, thanks. Proceeding...
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Integrated, thanks! (22, 23 and master).

Show
Eloy Lafuente (stronk7) added a comment - Integrated, thanks! (22, 23 and master).
Hide
Permalink
Frédéric Massart added a comment -

Test passed on 2.2, 2.3, master! Cheers!

Show
Frédéric Massart added a comment - Test passed on 2.2, 2.3, master! Cheers!
Hide
Permalink
Sam Hemelryk added a comment -

Congratulations your code is upstream - gold star for you!

This issue + 79 others made it in in time for the minor releases.
Thank you everyone involved for your exuberant efforts.

Show
Sam Hemelryk added a comment - Congratulations your code is upstream - gold star for you! This issue + 79 others made it in in time for the minor releases. Thank you everyone involved for your exuberant efforts.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: