MDL-20537: shorten-text fix

diff --git a/lib/moodlelib.php b/lib/moodlelib.php
index 8879042dfa87923a62e53d16a4d558e747b8bd83..427e15fb8dd3329a084518a5a04526f21dc62639 100644 (file)
--- a/lib/moodlelib.php
+++ b/lib/moodlelib.php
@@ -7735,13 +7735,19 @@ function shorten_text($text, $ideal=30, $exact = false, $ending='...') {
         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])) {
@@ -7750,15 +7756,14 @@ function shorten_text($text, $ideal=30, $exact = false, $ending='...') {
                     // 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];
@@ -7821,6 +7826,24 @@ function shorten_text($text, $ideal=30, $exact = false, $ending='...') {
     // 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 . '>';

diff --git a/lib/simpletest/testmoodlelib.php b/lib/simpletest/testmoodlelib.php
index af3384d87790a729c022bff08daf41072ca30968..e81c8cd9fadf6dd1f41cc2f61d80cb93adbf5a27 100644 (file)
--- a/lib/simpletest/testmoodlelib.php
+++ b/lib/simpletest/testmoodlelib.php
@@ -354,6 +354,34 @@ class moodlelib_test extends UnitTestCase {
         $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));
+    }
+
 }
 
 ?>