Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

MDL-20537: fix for infrequent shorten_text bug

  • Loading branch information...
commit f59ae746932d2f07b48abd3cea08a29ed832fd44 1 parent 0f40532
sam marshall sammarshallou authored
Showing with 60 additions and 9 deletions.
  1. +32 −9 lib/moodlelib.php
  2. +28 −0 lib/simpletest/testmoodlelib.php
41 lib/moodlelib.php
View
@@ -6840,13 +6840,19 @@ function shorten_text($text, $ideal=30, $exact = false) {
return $text;
}
- // splits all html-tags to scanable lines
+ // Splits on HTML tags. Each open/close/empty tag will be the first thing
+ // and only tag in its 'line'
preg_match_all('/(<.+?>)?([^<>]*)/s', $text, $lines, PREG_SET_ORDER);
$total_length = strlen($ending);
- $open_tags = array();
$truncate = '';
+ // This array stores information about open and close tags and their position
+ // in the truncated string. Each item in the array is an object with fields
+ // ->open (true if open), ->tag (tag name in lower case), and ->pos
+ // (byte position in truncated text)
+ $tagdetails = array();
+
foreach ($lines as $line_matchings) {
// if there is any html-tag in this line, handle it and add it (uncounted) to the output
if (!empty($line_matchings[1])) {
@@ -6855,15 +6861,14 @@ function shorten_text($text, $ideal=30, $exact = false) {
// do nothing
// if tag is a closing tag (f.e. </b>)
} else if (preg_match('/^<\s*\/([^\s]+?)\s*>$/s', $line_matchings[1], $tag_matchings)) {
- // delete tag from $open_tags list
- $pos = array_search($tag_matchings[1], array_reverse($open_tags, true)); // can have multiple exact same open tags, close the last one
- if ($pos !== false) {
- unset($open_tags[$pos]);
- }
+ // record closing tag
+ $tagdetails[] = (object)array('open'=>false,
+ 'tag'=>strtolower($tag_matchings[1]), 'pos'=>strlen($truncate));
// if tag is an opening tag (f.e. <b>)
} else if (preg_match('/^<\s*([^\s>!]+).*?>$/s', $line_matchings[1], $tag_matchings)) {
- // add tag to the beginning of $open_tags list
- array_unshift($open_tags, strtolower($tag_matchings[1]));
+ // record opening tag
+ $tagdetails[] = (object)array('open'=>true,
+ 'tag'=>strtolower($tag_matchings[1]), 'pos'=>strlen($truncate));
}
// add html-tag to $truncate'd text
$truncate .= $line_matchings[1];
@@ -6926,6 +6931,24 @@ function shorten_text($text, $ideal=30, $exact = false) {
// add the defined ending to the text
$truncate .= $ending;
+ // Now calculate the list of open html tags based on the truncate position
+ $open_tags = array();
+ foreach ($tagdetails as $taginfo) {
+ if(isset($breakpos) && $taginfo->pos >= $breakpos) {
+ // Don't include tags after we made the break!
+ break;
+ }
+ if($taginfo->open) {
+ // add tag to the beginning of $open_tags list
+ array_unshift($open_tags, $taginfo->tag);
+ } else {
+ $pos = array_search($taginfo->tag, array_reverse($open_tags, true)); // can have multiple exact same open tags, close the last one
+ if ($pos !== false) {
+ unset($open_tags[$pos]);
+ }
+ }
+ }
+
// close all unclosed html-tags
foreach ($open_tags as $tag) {
$truncate .= '</' . $tag . '>';
28 lib/simpletest/testmoodlelib.php
View
@@ -219,6 +219,34 @@ function test_make_user_directory() {
$this->assertFalse(make_user_directory(true, true));
}
+
+ function test_shorten_text() {
+ $text = "short text already no tags";
+ $this->assertEqual($text, shorten_text($text));
+
+ $text = "<p>short <b>text</b> already</p><p>with tags</p>";
+ $this->assertEqual($text, shorten_text($text));
+
+ $text = "long text without any tags blah de blah blah blah what";
+ $this->assertEqual('long text without any tags ...', shorten_text($text));
+
+ $text = "<div class='frog'><p><blockquote>Long text with tags that will ".
+ "be chopped off but <b>should be added back again</b></blockquote></p></div>";
+ $this->assertEqual("<div class='frog'><p><blockquote>Long text with " .
+ "tags that ...</blockquote></p></div>", shorten_text($text));
+
+ $text = "some text which shouldn't &nbsp; break there";
+ $this->assertEqual("some text which shouldn't &nbsp; ...",
+ shorten_text($text, 31));
+ $this->assertEqual("some text which shouldn't ...",
+ shorten_text($text, 30));
+
+ // This case caused a bug up to 1.9.5
+ $text = "<h3>standard 'break-out' sub groups in TGs?</h3>&nbsp;&lt;&lt;There are several";
+ $this->assertEqual("<h3>standard 'break-out' sub groups in ...</h3>",
+ shorten_text($text, 43));
+ }
+
}
?>
Please sign in to comment.
Something went wrong with that request. Please try again.