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
    • Pull 2.4 Branch:
      MDL-38824-m24
    • Rank:
      48892

      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'));
                          }
      

        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: