Move fix for getgrav#1114 to Truncator::truncateLetters

The original fix provided by getgrav#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.
dliessi · May 4, 2018 · a256f5e · a256f5e
1 parent 40b475e
commit a256f5e
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 6 deletions.
diff --git a/system/src/Grav/Common/Helpers/Truncator.php b/system/src/Grav/Common/Helpers/Truncator.php
@@ -98,6 +98,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 +112,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));
     }