Moodle
  1. Moodle
  2. MDL-31458

Forum email subjects containing ampersand are sent as &

    Details

    • Testing Instructions:
      Hide
      1. Set $CFG->divertallemailsto = 'youremailaddress'; in config.php
      2. As admin..
      3. Create a course and name its shortname "A&B"
      4. Enrol a student
      5. Add a news forum
      6. Create a new post titled "Test & Test" with a message
      7. Tick the mail now button and post to forum
      8. hit admin/cron.php (hopefully there will be a message about forum mailed out)
      9. Check your inbox and look at the title of the email
      Show
      Set $CFG->divertallemailsto = 'youremailaddress'; in config.php As admin.. Create a course and name its shortname "A&B" Enrol a student Add a news forum Create a new post titled "Test & Test" with a message Tick the mail now button and post to forum hit admin/cron.php (hopefully there will be a message about forum mailed out) Check your inbox and look at the title of the email
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
    • Rank:
      37986

      Description

      Forum email subject lines for topics with ampersand (&) in them are being replaced with &

      Other special characters do not seem to be affected. It appears format_string is responsible for this mangling:

      $string = replace_ampersands_not_followed_by_entity($string);

      Patch incoming that adds a simple html_entity_decode after this step, so the email subject line is returned to normal.

        Activity

        Show
        Tony Levi added a comment - https://github.com/tlevi/moodle/tree/mdl31458
        Hide
        Dan Poltawski added a comment -

        Thanks for the patch Tony. Unfortunately I don't think it solved the case of a course short-name with an ampersand and so I have made a modified patch using html_to_text on the whole subject.

        To be honest its a bit annoying that we need to format_text it at all since we are encoding text only to then decode it which is a waste of cpu cycles..

        However I believe we need to do it for multilang titles so can't get rid of the formatig completely.

        Submitting for integration.

        Show
        Dan Poltawski added a comment - Thanks for the patch Tony. Unfortunately I don't think it solved the case of a course short-name with an ampersand and so I have made a modified patch using html_to_text on the whole subject. To be honest its a bit annoying that we need to format_text it at all since we are encoding text only to then decode it which is a waste of cpu cycles.. However I believe we need to do it for multilang titles so can't get rid of the formatig completely. Submitting for integration.
        Hide
        Tony Levi added a comment -

        Fair enough - IIRC there were a few reported cases this patch didn't catch.

        Show
        Tony Levi added a comment - Fair enough - IIRC there were a few reported cases this patch didn't catch.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Sam Hemelryk added a comment -

        Thanks Dan, this has been integrated now

        Show
        Sam Hemelryk added a comment - Thanks Dan, this has been integrated now
        Hide
        Ankit Agarwal added a comment -

        Working as expected.
        Title is properly displayed now.
        Tested on 21, 22 and master
        Passing
        Thanks

        Show
        Ankit Agarwal added a comment - Working as expected. Title is properly displayed now. Tested on 21, 22 and master Passing Thanks
        Hide
        Eloy Lafuente (stronk7) added a comment -

        U P S T R E A M I Z E D !

        Many thanks for the hard work, closing this as fixed.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks for the hard work, closing this as fixed. Ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: