MDL-34654 Glossary: ampersand breaks auto-linking

filter/glossary/filter.php

@@ -134,6 +134,11 @@ public function filter($text, array $options = array()) {
                 foreach ($concepts as $key => $concept) {
                     // Trim empty or unlinkable concepts
                     $currentconcept = trim(strip_tags($concept->concept));
+
+                    // Concept must be HTML-escaped, so do the same as format_string
+                    // to turn ampersands into &.
+                    $currentconcept = replace_ampersands_not_followed_by_entity($currentconcept);
+
                     if (empty($currentconcept)) {
                         unset($concepts[$key]);
                         continue;
@@ -171,10 +176,14 @@ public function filter($text, array $options = array()) {
                                       '&mode=cat&hook='.$concept->id.'">';
                 } else { // Link to entry or alias
                     if (!empty($concept->originalconcept)) {  // We are dealing with an alias (so show and point to original)
-                        $title = str_replace('"', "'", strip_tags($glossaryname.': '.$concept->originalconcept));
+                        $title = str_replace('"', "'", html_entity_decode(
+                                strip_tags($glossaryname.': '.$concept->originalconcept)));
                         $concept->id = $concept->entryid;
                     } else { // This is an entry
-                        $title = str_replace('"', "'", strip_tags($glossaryname.': '.$concept->concept));
+                        // We need to remove entities from the content here because it
+                        // will be escaped by html_writer below.
+                        $title = str_replace('"', "'", html_entity_decode(
+                                strip_tags($glossaryname.': '.$concept->concept)));
                     }
                     // hardcoding dictionary format in the URL rather than defaulting
                     // to the current glossary format which may not work in a popup.

filter/glossary/tests/filter_test.php

@@ -0,0 +1,80 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Unit tests.
+ *
+ * @package filter_glossary
+ * @category test
+ * @copyright 2013 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once($CFG->dirroot . '/filter/glossary/filter.php'); // Include the code to test.
+
+/**
+ * Test case for glossary.
+ */
+class filter_glossary_filter_testcase extends advanced_testcase {
+
+    /**
+     * Test ampersands.
+     */
+    public function test_ampersands() {
+        global $CFG;
+        $this->resetAfterTest(true);
+
+        // Enable glossary filter at top level.
+        filter_set_global_state('glossary', TEXTFILTER_ON);
+        $CFG->glossary_linkentries = 1;
+
+        // Create a test course.
+        $course = $this->getDataGenerator()->create_course();
+        $context = context_course::instance($course->id);
+
+        // Create a glossary.
+        $glossary = $this->getDataGenerator()->create_module('glossary',
+                array('course' => $course->id, 'mainglossary' => 1));
+
+        // Create two entries with ampersands and one normal entry.
+        $generator = $this->getDataGenerator()->get_plugin_generator('mod_glossary');
+        $normal = $generator->create_content($glossary, array('concept' => 'normal'));
+        $amp1 = $generator->create_content($glossary, array('concept' => 'A&B'));
+        $amp2 = $generator->create_content($glossary, array('concept' => 'C&amp;D'));
+
+        // Format text with all three entries in HTML.
+        $html = '<p>A&amp;B C&amp;D normal</p>';
+        $filtered = format_text($html, FORMAT_HTML, array('context' => $context));
+
+        // Find all the glossary links in the result.
+        $matches = array();
+        preg_match_all('~courseid=' . $course->id . '&amp;eid=([0-9]+).*?title="(.*?)"~', $filtered, $matches);
+
+        // There should be 3 glossary links.
+        $this->assertEquals(3, count($matches[1]));
+        $this->assertEquals($amp1->id, $matches[1][0]);
+        $this->assertEquals($amp2->id, $matches[1][1]);
+        $this->assertEquals($normal->id, $matches[1][2]);
+
+        // Check text and escaping of title attribute.
+        $this->assertEquals($glossary->name . ': A&amp;B', $matches[2][0]);
+        $this->assertEquals($glossary->name . ': C&amp;D', $matches[2][1]);
+        $this->assertEquals($glossary->name . ': normal', $matches[2][2]);
+    }
+}