Move fix for #1114 to Truncator::truncateLetters (#2004)

* Move fix for #1114 to Truncator::truncateLetters The original fix provided by #1125 in Utils::truncateHtml compared the summary size with the full HTML string length. Thus, the string was still being truncated (and the HTML rewritten) even when only the HTML string length, and not the text length, exceeded the summary size. * Add fix for #1114 also to Truncator::truncateWords
getgrav · May 15, 2018 · a1abcfd · a1abcfd
1 parent 7f90ad8
commit a1abcfd
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 8 deletions.
diff --git a/system/src/Grav/Common/Helpers/Truncator.php b/system/src/Grav/Common/Helpers/Truncator.php
@@ -47,6 +47,7 @@ public static function truncateWords($html, $limit = 0, $ellipsis = '')
 
         // Iterate over words.
         $words = new DOMWordsIterator($body);
+        $truncated = false;
         foreach ($words as $word) {
 
             // If we have exceeded the limit, we delete the remainder of the content.
@@ -70,12 +71,19 @@ public static function truncateWords($html, $limit = 0, $ellipsis = '')
                     self::insertEllipsis($curNode, $ellipsis);
                 }
 
+                $truncated = true;
+
                 break;
             }
 
         }
 
-        return self::innerHTML($body);
+        // Return original HTML if not truncated.
+        if ($truncated) {
+            return self::innerHTML($body);
+        } else {
+            return $html;
+        }
     }
 
     /**
@@ -98,6 +106,7 @@ public static function truncateLetters($html, $limit = 0, $ellipsis = "")
 
         // Iterate over letters.
         $letters = new DOMLettersIterator($body);
+        $truncated = false;
         foreach ($letters as $letter) {
 
             // If we have exceeded the limit, we want to delete the remainder of this document.
@@ -111,11 +120,18 @@ public static function truncateLetters($html, $limit = 0, $ellipsis = "")
                     self::insertEllipsis($currentText[0], $ellipsis);
                 }
 
+                $truncated = true;
+
                 break;
             }
         }
 
-        return self::innerHTML($body);
+        // Return original HTML if not truncated.
+        if ($truncated) {
+            return self::innerHTML($body);
+        } else {
+            return $html;
+        }
     }
 
     /**

diff --git a/system/src/Grav/Common/Utils.php b/system/src/Grav/Common/Utils.php
@@ -296,10 +296,6 @@ public static function safeTruncate($string, $limit = 150)
      */
     public static function truncateHtml($text, $length = 100, $ellipsis = '...')
     {
-        if (mb_strlen($text) <= $length) {
-            return $text;
-        }
-
         return Truncator::truncateLetters($text, $length, $ellipsis);
     }
 

diff --git a/tests/unit/Grav/Common/UtilsTest.php b/tests/unit/Grav/Common/UtilsTest.php
@@ -127,7 +127,7 @@ public function testTruncateHtml()
         $this->assertEquals('<p>This...</p>', Utils::truncateHtml('<p>This is a string to truncate</p>', 4));
         $this->assertEquals('<p>This is a...</p>', Utils::truncateHtml('<p>This is a string to truncate</p>', 10));
         $this->assertEquals('<p>This is a string to truncate</p>', Utils::truncateHtml('<p>This is a string to truncate</p>', 100));
-        $this->assertEquals('<input type="file" id="file" multiple>', Utils::truncateHtml('<input type="file" id="file" multiple />', 6));
+        $this->assertEquals('<input type="file" id="file" multiple />', Utils::truncateHtml('<input type="file" id="file" multiple />', 6));
         $this->assertEquals('<ol><li>item 1 <i>so...</i></li></ol>', Utils::truncateHtml('<ol><li>item 1 <i>something</i></li><li>item 2 <strong>bold</strong></li></ol>', 10));
 		$this->assertEquals("<p>This is a string.</p>\n<p>It splits two lines.</p>", Utils::truncateHtml("<p>This is a string.</p>\n<p>It splits two lines.</p>", 100));
     }
@@ -138,7 +138,7 @@ public function testSafeTruncateHtml()
         $this->assertEquals('<p>This is...</p>', Utils::safeTruncateHtml('<p>This is a string to truncate</p>', 2));
         $this->assertEquals('<p>This is a string to...</p>', Utils::safeTruncateHtml('<p>This is a string to truncate</p>', 5));
         $this->assertEquals('<p>This is a string to truncate</p>', Utils::safeTruncateHtml('<p>This is a string to truncate</p>', 20));
-        $this->assertEquals('<input type="file" id="file" multiple>', Utils::safeTruncateHtml('<input type="file" id="file" multiple />', 6));
+        $this->assertEquals('<input type="file" id="file" multiple />', Utils::safeTruncateHtml('<input type="file" id="file" multiple />', 6));
         $this->assertEquals('<ol><li>item 1 <i>something</i></li><li>item 2...</li></ol>', Utils::safeTruncateHtml('<ol><li>item 1 <i>something</i></li><li>item 2 <strong>bold</strong></li></ol>', 5));
     }