From 96c3f75a4dd73dda18cc1c8deedf7604870e955e Mon Sep 17 00:00:00 2001 From: sam marshall Date: Wed, 23 Oct 2013 14:46:59 +0100 Subject: [PATCH] MDL-34654 Glossary: ampersand breaks auto-linking --- filter/glossary/filter.php | 13 ++++- filter/glossary/tests/filter_test.php | 80 +++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 filter/glossary/tests/filter_test.php diff --git a/filter/glossary/filter.php b/filter/glossary/filter.php index 5c57e2cc5b5..2f4283d6ac5 100644 --- a/filter/glossary/filter.php +++ b/filter/glossary/filter.php @@ -134,6 +134,11 @@ class filter_glossary extends moodle_text_filter { 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 @@ class filter_glossary extends moodle_text_filter { '&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. diff --git a/filter/glossary/tests/filter_test.php b/filter/glossary/tests/filter_test.php new file mode 100644 index 00000000000..c8f396689b9 --- /dev/null +++ b/filter/glossary/tests/filter_test.php @@ -0,0 +1,80 @@ +. + +/** + * 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&D')); + + // Format text with all three entries in HTML. + $html = '

A&B C&D normal

'; + $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 . '&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&B', $matches[2][0]); + $this->assertEquals($glossary->name . ': C&D', $matches[2][1]); + $this->assertEquals($glossary->name . ': normal', $matches[2][2]); + } +}