Moodle
  1. Moodle
  2. MDL-35472

HTML tags appear in page titles when formatstringstriptags is disabled

    Details

    • Testing Instructions:
      Hide

      With formatstringstriptags disabled

      1. Go to Site Admin > Appearance > HTML Settings and make sure "Remove HTML tags from all activity names" is checked
      2. Create a course
      3. Add an activity (any will do but lets say a quiz)
      4. Give activity the name "<span style="font-size:2em;">My Quiz</span>" and save
      5. View the activity

      Result:

      The activity will be created. When viewed the activity on the page the title will exclude the tags e.g. it will be "My Quiz". The font size will be normal and the browser title won't include any tags.

      With formatstringstriptags enabled

      1. Go to Site Admin > Appearance > HTML Settings and make sure "Remove HTML tags from all activity names" is not checked
      2. Create a course
      3. Add an activity (any will do but lets say a quiz)
      4. Give activity the name "<span style="font-size:2em;">My Quiz</span>" and save
      5. View the activity

      Result:

      The activity will be created. The browser title bar will still exclude the tags e.g. it will be "My Quiz". The activity name will now include the tags, so the activity name on the page will be in a large font.

      No HTML tags in the activity name

      If the activity name doesn't include HTML tags then the result is the same with formatstringstriptags on or off - the activity name should be displayed unchanged in the title.

      Show
      With formatstringstriptags disabled Go to Site Admin > Appearance > HTML Settings and make sure "Remove HTML tags from all activity names" is checked Create a course Add an activity (any will do but lets say a quiz) Give activity the name "<span style="font-size:2em;">My Quiz</span>" and save View the activity Result: The activity will be created. When viewed the activity on the page the title will exclude the tags e.g. it will be "My Quiz". The font size will be normal and the browser title won't include any tags. With formatstringstriptags enabled Go to Site Admin > Appearance > HTML Settings and make sure "Remove HTML tags from all activity names" is not checked Create a course Add an activity (any will do but lets say a quiz) Give activity the name "<span style="font-size:2em;">My Quiz</span>" and save View the activity Result: The activity will be created. The browser title bar will still exclude the tags e.g. it will be "My Quiz". The activity name will now include the tags, so the activity name on the page will be in a large font. No HTML tags in the activity name If the activity name doesn't include HTML tags then the result is the same with formatstringstriptags on or off - the activity name should be displayed unchanged in the title.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Rank:
      44171

      Description

      If formatstringstriptags is disabled (so HTML is allowed in activity names), then viewing most activities will lead to the tags appearing in the page title.

      Replication steps:

      1. Go to Site Admin > Appearance > HTML Settings and uncheck "Remove HTML tags from all activity names"
      2. Create a course
      3. Add an activity (any will do but lets say a quiz)
      4. Give activity the name "<span style="font-size:2em;">My Quiz</span>" and save
      5. Click to enter the quiz

      Expected result: The HTML tags should be stripped from the title.

      Actual result: The browser title bar will include the tags from the activity name, e.g. the title will be:

      test course: <span style="font-size:2em;">My Quiz</span>

        Activity

        Hide
        Michael de Raadt added a comment -

        Thanks for reporting that, Simon. And thanks for providing a patch.

        Feel free to push this to peer review if you wish.

        Show
        Michael de Raadt added a comment - Thanks for reporting that, Simon. And thanks for providing a patch. Feel free to push this to peer review if you wish.
        Hide
        Andrew Davis added a comment -

        [Y] Syntax
        [NA] Output
        [Y] Whitespace
        [NA] Language
        [NA] Databases
        [N] Testing
        [NA] Security
        [NA] Documentation
        [] Git
        [] Sanity check

        This needs some testing instructions. Make sure to include the various permutations. "Remove HTML tags from all activity names" on and off, html tags in the title, no tags in the title.

        You're missing the component from the commit message. It should be like "MDL-35472 component: Prevent html tags appearing in page titles" Im not actually sure what the component should be. Intuitively it seems like it should be core_page but I'll double check.

        Show
        Andrew Davis added a comment - [Y] Syntax [NA] Output [Y] Whitespace [NA] Language [NA] Databases [N] Testing [NA] Security [NA] Documentation [] Git [] Sanity check This needs some testing instructions. Make sure to include the various permutations. "Remove HTML tags from all activity names" on and off, html tags in the title, no tags in the title. You're missing the component from the commit message. It should be like " MDL-35472 component: Prevent html tags appearing in page titles" Im not actually sure what the component should be. Intuitively it seems like it should be core_page but I'll double check.
        Hide
        Simon Coggins added a comment -

        I've added some testing instructions. Looking at the other commits that modify pagelib.php there are quite a few options for component to choose from. "pagelib" seems popular. Let me know what is preferred and I'll update the commit message.

        It would be useful to have a list of which component to use on the dev docs, especially for core changes.

        Show
        Simon Coggins added a comment - I've added some testing instructions. Looking at the other commits that modify pagelib.php there are quite a few options for component to choose from. "pagelib" seems popular. Let me know what is preferred and I'll update the commit message. It would be useful to have a list of which component to use on the dev docs, especially for core changes.
        Hide
        Andrew Davis added a comment -

        There was a little discussion in the Moodle developer chat room about the appropriate component. The outcome seems to be that components in commit messages are no longer really necessary so maybe its not even worth bothering going back and fixing.

        Theoretically the list of core components is here http://docs.moodle.org/dev/Frankenstyle#Core_subsystems I just can't see where this fits in. Pick whatever you feel is most appropriate.... I hope that's not too vague.

        Show
        Andrew Davis added a comment - There was a little discussion in the Moodle developer chat room about the appropriate component. The outcome seems to be that components in commit messages are no longer really necessary so maybe its not even worth bothering going back and fixing. Theoretically the list of core components is here http://docs.moodle.org/dev/Frankenstyle#Core_subsystems I just can't see where this fits in. Pick whatever you feel is most appropriate.... I hope that's not too vague.
        Hide
        Simon Coggins added a comment -

        To be honest none of those seem particularly appropriate for this one - I think I'll leave it without a component.

        Show
        Simon Coggins added a comment - To be honest none of those seem particularly appropriate for this one - I think I'll leave it without a component.
        Hide
        Andrew Davis added a comment -

        Fair enough.

        With the testing instructions, try to avoid asking people to do testing before applying the fix. At the point of integration testing there's no need to replicate the original bug. Plus most of the testers update a test Moodle installation then test a bunch of issues one after the other. Upgrade issues aside you should be able to avoid the need to do anything prior to updating.

        Once thats done feel free to submit for integration. Let me know if you don't have the capability to do that and I'll do it for you.

        Show
        Andrew Davis added a comment - Fair enough. With the testing instructions, try to avoid asking people to do testing before applying the fix. At the point of integration testing there's no need to replicate the original bug. Plus most of the testers update a test Moodle installation then test a bunch of issues one after the other. Upgrade issues aside you should be able to avoid the need to do anything prior to updating. Once thats done feel free to submit for integration. Let me know if you don't have the capability to do that and I'll do it for you.
        Hide
        Simon Coggins added a comment -

        Instructions updated. I don't seem to have permission to submit for integration so could you do it please.

        Show
        Simon Coggins added a comment - Instructions updated. I don't seem to have permission to submit for integration so could you do it please.
        Hide
        Andrew Davis added a comment -

        Submitting for integration.

        Show
        Andrew Davis added a comment - Submitting for integration.
        Hide
        Sam Hemelryk added a comment -

        Thanks Simon, this has been integrated now.

        Show
        Sam Hemelryk added a comment - Thanks Simon, this has been integrated now.
        Hide
        David Monllaó added a comment -

        It passes, tested in 22 and master

        Show
        David Monllaó added a comment - It passes, tested in 22 and master
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Closing as fixed, many thanks for your awesome collaboration.

        Show
        Eloy Lafuente (stronk7) added a comment - Closing as fixed, many thanks for your awesome collaboration.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: