Skip to content

Commit

Permalink
MDL-47962 filter_glossary: only create replacement HTML if needed
Browse files Browse the repository at this point in the history
  • Loading branch information
@timhunt
timhunt committed Sep 20, 2018
1 parent 1d8d00d commit 5a8c44d
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 62 deletions.
114 changes: 63 additions & 51 deletions filter/glossary/filter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,10 @@ public function setup($page, $context) {

/**
* Get all the concepts for this context.
* @return filterobject[] the concepts, and filterobjects, with an added
* ->conceptid field.
* @return filterobject[] the concepts, and filterobjects.
*/
protected function get_all_concepts() {
global $CFG, $USER;
global $USER;

if ($this->cache === null) {
$this->cache = cache::make_from_params(cache_store::MODE_REQUEST, 'filter', 'glossary');
Expand Down Expand Up @@ -92,48 +91,16 @@ protected function get_all_concepts() {

foreach ($allconcepts as $concepts) {
foreach ($concepts as $concept) {
if ($concept->category) { // Link to a category.
$title = get_string('glossarycategory', 'filter_glossary',
['glossary' => $glossaries[$concept->glossaryid], 'category' => $concept->concept]);
$link = new moodle_url('/mod/glossary/view.php', array('g' => $concept->glossaryid, 'mode' => 'cat', 'hook' => $concept->id));
$attributes = array(
'href' => $link,
'title' => $title,
'class' => 'glossary autolink category glossaryid' . $concept->glossaryid);
$conceptid = 0;

} else { // Link to entry or alias
$title = get_string('glossaryconcept', 'filter_glossary',
['glossary' => $glossaries[$concept->glossaryid], 'concept' => $concept->concept]);
// Hardcoding dictionary format in the URL rather than defaulting
// to the current glossary format which may not work in a popup.
// for example "entry list" means the popup would only contain
// a link that opens another popup.
$link = new moodle_url('/mod/glossary/showentry.php', array('eid' => $concept->id, 'displayformat' => 'dictionary'));
$attributes = array(
'href' => $link,
'title' => str_replace('&', '&', $title), // Undo the s() mangling.
'class' => 'glossary autolink concept glossaryid' . $concept->glossaryid);
$conceptid = $concept->id;
}
// This flag is optionally set by resource_pluginfile()
// if processing an embedded file use target to prevent getting nested Moodles.
if (!empty($CFG->embeddedsoforcelinktarget)) {
$attributes['target'] = '_top';
}
$href_tag_begin = html_writer::start_tag('a', $attributes);

$filterobj = new filterobject($concept->concept, $href_tag_begin, '</a>',
$concept->casesensitive, $concept->fullmatch);
$filterobj->conceptid = $conceptid;
$conceptlist[] = $filterobj;
$conceptlist[] = new filterobject($concept->concept, null, null,
$concept->casesensitive, $concept->fullmatch, null,
[$this, 'filterobject_prepare_replacement_callback'], [$concept, $glossaries]);
}
}

// We sort longest first, so that when we replace the terms,
// the longest ones are replaced first. This does the right thing
// when you have two terms like 'Moodle' and 'Moodle 3.5'. You want the longest match.
usort($conceptlist, 'filter_glossary::sort_entries_by_length');
usort($conceptlist, [$this, 'sort_entries_by_length']);

$conceptlist = filter_prepare_phrases_for_filtering($conceptlist);

Expand All @@ -146,6 +113,50 @@ protected function get_all_concepts() {
return $conceptlist;
}

/**
* Callback used by filterobject / filter_phrases.
*
* @param object $concept the concept that is being replaced (from get_all_concepts).
* @param array $glossaries the list of glossary titles (from get_all_concepts).
* @return array [$hreftagbegin, $hreftagend, $replacementphrase] for filterobject.
*/
public function filterobject_prepare_replacement_callback($concept, $glossaries) {
global $CFG;

if ($concept->category) { // Link to a category.
$title = get_string('glossarycategory', 'filter_glossary',
['glossary' => $glossaries[$concept->glossaryid], 'category' => $concept->concept]);
$link = new moodle_url('/mod/glossary/view.php',
['g' => $concept->glossaryid, 'mode' => 'cat', 'hook' => $concept->id]);
$attributes = array(
'href' => $link,
'title' => $title,
'class' => 'glossary autolink category glossaryid' . $concept->glossaryid);

} else { // Link to entry or alias.
$title = get_string('glossaryconcept', 'filter_glossary',
['glossary' => $glossaries[$concept->glossaryid], 'concept' => $concept->concept]);
// Hardcoding dictionary format in the URL rather than defaulting
// to the current glossary format which may not work in a popup.
// for example "entry list" means the popup would only contain
// a link that opens another popup.
$link = new moodle_url('/mod/glossary/showentry.php',
['eid' => $concept->id, 'displayformat' => 'dictionary']);
$attributes = array(
'href' => $link,
'title' => str_replace('&amp;', '&', $title), // Undo the s() mangling.
'class' => 'glossary autolink concept glossaryid' . $concept->glossaryid);
}

// This flag is optionally set by resource_pluginfile()
// if processing an embedded file use target to prevent getting nested Moodles.
if (!empty($CFG->embeddedsoforcelinktarget)) {
$attributes['target'] = '_top';
}

return [html_writer::start_tag('a', $attributes), '</a>', null];
}

public function filter($text, array $options = array()) {
global $GLOSSARY_EXCLUDEENTRY;

Expand All @@ -157,7 +168,11 @@ public function filter($text, array $options = array()) {

if (!empty($GLOSSARY_EXCLUDEENTRY)) {
foreach ($conceptlist as $key => $filterobj) {
if (is_object($filterobj) && $filterobj->conceptid == $GLOSSARY_EXCLUDEENTRY) {
if (!is_object($filterobj)) {
continue;
}
$concept = $filterobj->replacementcallbackdata[0];
if (!$concept->category && $concept->id == $GLOSSARY_EXCLUDEENTRY) {
unset($conceptlist[$key]);
}
}
Expand All @@ -170,16 +185,13 @@ public function filter($text, array $options = array()) {
return filter_phrases($text, $conceptlist); // Actually search for concepts!
}

private static function sort_entries_by_length($entry0, $entry1) {
$len0 = strlen($entry0->phrase);
$len1 = strlen($entry1->phrase);

if ($len0 < $len1) {
return 1;
} else if ($len0 > $len1) {
return -1;
} else {
return 0;
}
/**
* usort helper used in get_all_concepts above.
* @param filterobject $filterobject0 first item to compare.
* @param filterobject $filterobject1 second item to compare.
* @return int -1, 0 or 1.
*/
private function sort_entries_by_length($filterobject0, $filterobject1) {
return strlen($filterobject1->phrase) <=> strlen($filterobject0->phrase);
}
}
37 changes: 37 additions & 0 deletions filter/glossary/tests/filter_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,43 @@ public function test_link_to_entry_with_alias() {
$this->assertEquals($glossary->name . ': second alias', $matches[2][2]);
}

public function test_longest_link_used() {
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');
$shorter = $generator->create_content($glossary, array('concept' => 'Tim'));
$longer = $generator->create_content($glossary, array('concept' => 'Time'));

// Format text with all three entries in HTML.
$html = '<p>Time will tell</p>';
$filtered = format_text($html, FORMAT_HTML, array('context' => $context));

// Find all the glossary links in the result.
$matches = array();
preg_match_all('~eid=([0-9]+).*?title="(.*?)"~', $filtered, $matches);

// There should be 1 glossary link to Time, not Tim.
$this->assertEquals(1, count($matches[1]));
$this->assertEquals($longer->id, $matches[1][0]);

// Check text of title attribute.
$this->assertEquals($glossary->name . ': Time', $matches[2][0]);
}

public function test_link_to_category() {
global $CFG;
$this->resetAfterTest(true);
Expand Down
38 changes: 27 additions & 11 deletions lib/filterlib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -506,19 +506,30 @@ class filterobject {
* @param bool $fullmatch whether to match complete words. If true, 'T' won't be matched in 'Tim'.
* @param mixed $replacementphrase replacement text to go inside begin and end. If not set,
* the body of the replacement will be the original phrase.
* @param callback $replacementcallback if set, then this will be called just before
* $hreftagbegin, $hreftagend and $replacementphrase are needed, so they can be computed only if required.
* The call made is
* list($linkobject->hreftagbegin, $linkobject->hreftagend, $linkobject->replacementphrase) =
* call_user_func_array($linkobject->replacementcallback, $linkobject->replacementcallbackdata);
* so the return should be an array [$hreftagbegin, $hreftagend, $replacementphrase], the last of which may be null.
* @param array $replacementcallbackdata data to be passed to $replacementcallback (optional).
*/
public function __construct($phrase, $hreftagbegin = '<span class="highlight">',
$hreftagend = '</span>',
$casesensitive = false,
$fullmatch = false,
$replacementphrase = null) {

$this->phrase = $phrase;
$this->hreftagbegin = $hreftagbegin;
$this->hreftagend = $hreftagend;
$this->casesensitive = !empty($casesensitive);
$this->fullmatch = !empty($fullmatch);
$this->replacementphrase = $replacementphrase;
$hreftagend = '</span>',
$casesensitive = false,
$fullmatch = false,
$replacementphrase = null,
$replacementcallback = null,
array $replacementcallbackdata = null) {

$this->phrase = $phrase;
$this->hreftagbegin = $hreftagbegin;
$this->hreftagend = $hreftagend;
$this->casesensitive = !empty($casesensitive);
$this->fullmatch = !empty($fullmatch);
$this->replacementphrase = $replacementphrase;
$this->replacementcallback = $replacementcallback;
$this->replacementcallbackdata = $replacementcallbackdata;
}
}

Expand Down Expand Up @@ -1429,6 +1440,11 @@ function filter_prepare_phrases_for_filtering(array $linkarray) {
* @param filterobject $linkobject the link object on which to set additional fields.
*/
function filter_prepare_phrase_for_replacement(filterobject $linkobject) {
if ($linkobject->replacementcallback !== null) {
list($linkobject->hreftagbegin, $linkobject->hreftagend, $linkobject->replacementphrase) =
call_user_func_array($linkobject->replacementcallback, $linkobject->replacementcallbackdata);
}

if (!isset($linkobject->hreftagbegin) or !isset($linkobject->hreftagend)) {
$linkobject->hreftagbegin = '<span class="highlight"';
$linkobject->hreftagend = '</span>';
Expand Down

0 comments on commit 5a8c44d

Please sign in to comment.