Moodle
  1. Moodle
  2. MDL-38824

Activity completion when Activities have apostrophes breaks alt text

    Details

    • Testing Instructions:
      Hide

      0. Ensure completion tracking is enabled in advanced features.
      1. Create a new empty course with completion tracking enabled (leave other settings default).
      2. Add a new forum to the first week of the course. Set completion to automatic, requiring 1 post. Set name to: argh'"s forum
      3. Turn editing off.
      4. View source. Search for all instances of the string "argh" so as to find the 'argh'"s forum' strings regardless of how they are quoted.

      BEFORE FIX: Some of those instances are invalid HTML code because they contain a single quote inside an attribute that is single-quoted. Firefox displays these in red in its source view.

      EXPECTED: Any instances that are in attributes should be escaped suitably for the surrounding quotes.

      5. Go into the forum and post a message.
      6. Go back to the homepage (the box should be ticked) and view source again to check that the strings are still quoted.

      Show
      0. Ensure completion tracking is enabled in advanced features. 1. Create a new empty course with completion tracking enabled (leave other settings default). 2. Add a new forum to the first week of the course. Set completion to automatic, requiring 1 post. Set name to: argh'"s forum 3. Turn editing off. 4. View source. Search for all instances of the string "argh" so as to find the 'argh'"s forum' strings regardless of how they are quoted. BEFORE FIX: Some of those instances are invalid HTML code because they contain a single quote inside an attribute that is single-quoted. Firefox displays these in red in its source view. EXPECTED: Any instances that are in attributes should be escaped suitably for the surrounding quotes. 5. Go into the forum and post a message. 6. Go back to the homepage (the box should be ticked) and view source again to check that the strings are still quoted.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE

      Description

      This has been fixed for the manual Activity completion case (see MDL-36808), however it hasn't been fixed when the completion is set to automatic.

      This is also fixed for 2.5 due to a refactoring (see MDL-37085), but has yet to be fixed in the 2.4 branch.

      The following code (taken from http://git.moodle.org/gw?p=moodle.git;a=blob;f=course/lib.php;hb=refs/heads/MOODLE_24_STABLE )

      /course/lib.php 1681-1685

                           } else {
                               // In auto mode, or when editing, the icon is just an image
                               echo "<span class='autocompletion'>";
                               echo "<img src='$imgsrc' alt='$imgalt' title='$imgalt' /></span>";
                           }
      

      should be replaced with this (ported from the 2.5 branch)

      Fixed /course/lib.php

                          } else {
                              // In auto mode, or when editing, the icon is just an image
                              $completionpixicon = new pix_icon('i/completion-'.$completionicon, $imgalt, '',
                                      array('title' => $imgalt));
                              echo html_writer::tag('span', $OUTPUT->render($completionpixicon),
                                      array('class' => 'autocompletion'));
                          }
      

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Sam Marshall added a comment -

            Added test instructions based on those from MDL-36808. (I think the correct fix is also to copy the fix from that issue rather than try to backport the 2.5 code.) I think this also applies to Moodle 2.3 but will confirm when submitting patch.

            Show
            Sam Marshall added a comment - Added test instructions based on those from MDL-36808 . (I think the correct fix is also to copy the fix from that issue rather than try to backport the 2.5 code.) I think this also applies to Moodle 2.3 but will confirm when submitting patch.
            Hide
            Sam Marshall added a comment -

            Here's a patch. By the way the reason why I think this code (same as other issue in that it just changes existing code to use html_writer) is better as a minimal bugfix for the old releases, is that it's a minor change that doesn't require other variables to be defined etc.

            Show
            Sam Marshall added a comment - Here's a patch. By the way the reason why I think this code (same as other issue in that it just changes existing code to use html_writer) is better as a minimal bugfix for the old releases, is that it's a minor change that doesn't require other variables to be defined etc.
            Hide
            Sam Marshall added a comment -

            Please could somebody code-review this minor fix for 2.3 and 2.4 (not master). I went through the test script, it seems to work.

            Show
            Sam Marshall added a comment - Please could somebody code-review this minor fix for 2.3 and 2.4 (not master). I went through the test script, it seems to work.
            Hide
            Mark Nelson added a comment -

            Hi Sam, code looks perfect. Thanks again for your contribution, sending to integration.

            Show
            Mark Nelson added a comment - Hi Sam, code looks perfect. Thanks again for your contribution, sending to integration.
            Hide
            Mark Nelson added a comment -

            Also, thanks for your description letting me know why this was not needed in master.

            Show
            Mark Nelson added a comment - Also, thanks for your description letting me know why this was not needed in master.
            Hide
            Dan Poltawski added a comment -

            Thanks Sam, i've integrated this to 24 and 23.

            Show
            Dan Poltawski added a comment - Thanks Sam, i've integrated this to 24 and 23.
            Hide
            Dan Poltawski added a comment -

            Tested during integration and passed.

            Show
            Dan Poltawski added a comment - Tested during integration and passed.
            Hide
            Dan Poltawski added a comment -

            Blooming Marvelous! It's time for a knees up - your changes are upstream!

            Thanks for making Moodle better!

            Toodle pip

            Show
            Dan Poltawski added a comment - Blooming Marvelous! It's time for a knees up - your changes are upstream! Thanks for making Moodle better! Toodle pip

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: