Highlight special characters, support comparison and onestring view

Fixes #761 Make special char highlighting consistent accross views + highlight source strings
mozfr · Aug 2, 2016 · 719d5e9 · 719d5e9
1 parent 0732efa
commit 719d5e9
Show file tree

Hide file tree

Showing 10 changed files with 69 additions and 56 deletions.
diff --git a/app/classes/Transvision/ShowResults.php b/app/classes/Transvision/ShowResults.php
@@ -215,25 +215,6 @@ public static function formatEntity($entity, $highlight = false)
         return $repo . '<span class="superset">&nbsp;&bull;&nbsp;</span>' . $path . '<br>' . $entity;
     }
 
-    /**
-     * Highlight specific elements in the string.
-     *
-     * @param  string $string Source text
-     * @return string Same string with specific sub-strings in <span>
-     *                       elements for styling with CSS
-     */
-    public static function highlight($string)
-    {
-        $replacements = [
-            ' '        => '<span class="highlight-space" title="White space"> </span>',
-            ' '        => '<span class="highlight-red" title="Unicode non-breaking space"> </span>',
-            '…'        => '<span class="highlight-gray" title="Real ellipsis">…</span>',
-            '&hellip;' => '<span class="highlight-red" title="HTML ellipsis">…</span>',
-        ];
-
-        return Strings::multipleStringReplace($replacements, $string);
-    }
-
     /**
      * Html table of search results used by the main view (needs a lot of refactoring)
      *
@@ -328,20 +309,15 @@ public static function resultsTable($search_object, $search_results, $page)
             $target_string = htmlspecialchars($target_string);
             $source_string = Utils::highlightString($source_string);
             $target_string = Utils::highlightString($target_string);
+            $source_string = Strings::highlightSpecial($source_string);
+            $target_string = Strings::highlightSpecial($target_string);
 
             if ($extra_locale) {
                 $target_string2 = htmlspecialchars($target_string2);
                 $target_string2 = Utils::highlightString($target_string2);
+                $target_string2 = Strings::highlightSpecial($target_string2);
             }
 
-            $replacements = [
-                ' '            => '<span class="highlight-gray" title="Non breakable space"> </span>', // Nbsp highlight
-                ' '            => '<span class="highlight-red" title="Thin space"> </span>', // Thin space highlight
-                '…'            => '<span class="highlight-gray">…</span>', // Right ellipsis highlight
-                '&hellip;'     => '<span class="highlight-gray">…</span>', // Right ellipsis highlight
-            ];
-
-            $target_string = Strings::multipleStringReplace($replacements, $target_string);
             $clipboard_target_string  = 'clip_' . md5($target_string);
             $clipboard_target_string2 = 'clip_' . md5($target_string2);
 

diff --git a/app/classes/Transvision/Strings.php b/app/classes/Transvision/Strings.php
@@ -102,6 +102,33 @@ public static function multipleStringReplace($replacements, $string)
         return str_replace(array_keys($replacements), $replacements, $string);
     }
 
+    /**
+     * Highlight special characters in the string
+     *
+     * @param  string  $string              Source text
+     * @param  boolean $exclude_whitespaces Optional param to specify if we need
+     *                                      to highlight white spaces. White
+     *                                      spaces are not highlighted by default.
+     * @return string  Same string with specific sub-strings in <span>
+     *                                     elements for styling with CSS
+     */
+    public static function highlightSpecial($string, $exclude_whitespaces = true)
+    {
+        $replacements = [
+            ' '        => '<span class="highlight-space" title="White space"> </span>',
+            ' '        => '<span class="highlight-gray" title="Non breakable space"> </span>',
+            ' '        => '<span class="highlight-red" title="Narrow no-break space"> </span>',
+            '…'        => '<span class="highlight-gray" title="Real ellipsis">…</span>',
+            '&hellip;' => '<span class="highlight-red" title="HTML ellipsis">…</span>',
+        ];
+
+        if ($exclude_whitespaces) {
+            unset($replacements[' ']);
+        }
+
+        return self::multipleStringReplace($replacements, $string);
+    }
+
     /**
      * Get multibyte UTF-8 string length, html tags stripped
      *

diff --git a/app/models/channelcomparison.php b/app/models/channelcomparison.php
@@ -45,17 +45,25 @@
 
 /*
     Find new locale strings added between repositories, store them with the
-    reference string.
+    reference string and highlight special characters.
 */
 $added_strings = array_diff_key($strings[$chan1], $strings[$chan2]);
 $new_strings = [];
 $en_US_strings_chan1 = Utils::getRepoStrings('en-US', $chan1);
+
 foreach ($added_strings as $string_id => $translation) {
     $reference_string = isset($en_US_strings_chan1[$string_id])
         ? $en_US_strings_chan1[$string_id]
-        : '@N/A@';
+        : '@@missing@@';
+
     $new_strings[$string_id] = [
-        'reference'   => $reference_string,
-        'translation' => $translation,
+        'reference'   => Utils::secureText($reference_string),
+        'translation' => Strings::highlightSpecial(Utils::secureText($translation)),
     ];
 }
+
+// Highlight special characters in common strings
+foreach ($common_strings as $key => &$value) {
+    $common_strings[$key]  = Strings::highlightSpecial(Utils::secureText($value));
+    $strings[$chan2][$key] = Strings::highlightSpecial(Utils::secureText($strings[$chan2][$key]));
+}
diff --git a/app/views/channelcomparison.php b/app/views/channelcomparison.php
@@ -66,8 +66,8 @@
     <?php foreach ($common_strings as $key => $value) : ?>
     <tr>
         <?=$td('Key', ShowResults::formatEntity($key))?>
-        <?=$td($chan1, Utils::secureText($value))?>
-        <?=$td($chan2, Utils::secureText($strings[$chan2][$key]))?>
+        <?=$td($chan1, $value)?>
+        <?=$td($chan2, $strings[$chan2][$key])?>
     </tr>
     <?php endforeach; ?>
     </tbody>
@@ -91,12 +91,12 @@
     <?php foreach ($new_strings as $string_id => $string_values): ?>
     <tr>
         <?=$td('Entity', ShowResults::formatEntity($string_id))?>
-        <?php if ($new_strings[$string_id]['reference'] != '@N/A@'): ?>
-        <?=$td('en-US', Utils::secureText($string_values['reference']))?>
+        <?php if ($new_strings[$string_id]['reference'] != '@@missing@@'): ?>
+        <?=$td('en-US', $string_values['reference'])?>
         <?php else: ?>
-        <?=$td('en-US', '<em class="error">(not available)</em>')?>
+        <?=$td('en-US', '<em class="error">Warning: Missing string</em>')?>
         <?php endif; ?>
-        <?=$td($locale, Utils::secureText($string_values['translation']))?>
+        <?=$td($locale, $string_values['translation'])?>
     </tr>
     <?php endforeach; ?>
     <tbody>

diff --git a/app/views/consistency.php b/app/views/consistency.php
@@ -65,7 +65,7 @@
         echo '<a href="' . $search_link . '" title="Search for this string">' . Utils::secureText($data['source']) . "</a></td>\n";
         echo '<td>';
         foreach ($data['target'] as $target) {
-            echo '<div class="inconsistent_translation highlight-specialchars">' . ShowResults::highlight(Utils::secureText($target)) . "</div>\n";
+            echo '<div class="inconsistent_translation highlight-specialchars">' . Strings::highlightSpecial(Utils::secureText($target), false) . "</div>\n";
         }
         echo "</td>\n</tr>\n";
     }

diff --git a/app/views/onestring.php b/app/views/onestring.php
@@ -36,7 +36,7 @@
         if (! $translation) {
             echo "  <td><em class='error'>Warning: Empty string</em></td><td></td>\n";
         } else {
-            echo "  <td lang='#{$locale}' {$rtl_support} >" . Utils::secureText($translation) . "</td>\n" .
+            echo "  <td lang='#{$locale}' {$rtl_support} >" . Strings::highlightSpecial(Utils::secureText($translation)) . "</td>\n" .
                  "  <td><a class='onestring_search' href='{$search_link}' title='Search for the entity in this locale'>🔍</a></td>\n";
         }
 

diff --git a/app/views/results_entities.php b/app/views/results_entities.php
@@ -31,10 +31,9 @@
     $bz_target_string = $target_string = isset($tmx_target[$entity])
                                             ? Utils::secureText($tmx_target[$entity])
                                             : '@@missing@@';
-    // Highlight non-breaking spaces only after strings have been escaped
-    $target_string = str_replace(' ', '<span class="highlight-gray"> </span>', $target_string);
-
-    $source_string = Utils::secureText($tmx_source[$entity]);
+    // Highlight special characters only after strings have been escaped
+    $target_string = Strings::highlightSpecial($target_string);
+    $source_string = Strings::highlightSpecial(Utils::secureText($tmx_source[$entity]));
 
     $clipboard_target_string  = 'clip_' . md5($target_string);
 
@@ -46,8 +45,8 @@
         $bz_target_string2 = $target_string2 = isset($tmx_target2[$entity])
                                                     ? Utils::secureText($tmx_target2[$entity])
                                                     : '';
-        // Highlight non-breaking spaces only after strings have been escaped
-        $target_string2 = str_replace(' ', '<span class="highlight-gray"> </span>', $target_string2);
+        // Highlight special characters only after strings have been escaped
+        $target_string2 = Strings::highlightSpecial($target_string2);
 
         $clipboard_target_string2  = 'clip_' . md5($target_string2);
 

diff --git a/tests/units/Transvision/ShowResults.php b/tests/units/Transvision/ShowResults.php
@@ -38,14 +38,6 @@ public function testGetTMXResults($a, $b, $c)
                 ->isEqualTo($c);
     }
 
-    public function testHighlight()
-    {
-        $obj = new _ShowResults();
-        $this
-            ->string($obj->highlight('Foo is bar ; Bar is Foo…'))
-                ->isEqualTo('Foo<span class="highlight-space" title="White space"> </span>is<span class="highlight-space" title="White space"> </span>bar<span class="highlight-red" title="Unicode non-breaking space"> </span>;<span class="highlight-red" title="Unicode non-breaking space"> </span>Bar<span class="highlight-space" title="White space"> </span>is<span class="highlight-space" title="White space"> </span>Foo<span class="highlight-gray" title="Real ellipsis">…</span>');
-    }
-
     public function getTranslationMemoryResultsDP()
     {
         include TMX . 'en-US/cache_en-US_central.php';

diff --git a/tests/units/Transvision/Strings.php b/tests/units/Transvision/Strings.php
@@ -118,6 +118,17 @@ public function testmultipleStringReplace($a, $b, $c)
                 ->isEqualTo($c);
     }
 
+    public function testHighlightSpecial()
+    {
+        $obj = new _Strings();
+        $this
+            ->string($obj->highlightSpecial('Foo is bar ; Bar is Foo…'))
+                ->isEqualTo('Foo is bar<span class="highlight-gray" title="Non breakable space"> </span>;<span class="highlight-gray" title="Non breakable space"> </span>Bar is Foo<span class="highlight-gray" title="Real ellipsis">…</span>');
+        $this
+            ->string($obj->highlightSpecial('Foo is bar ; Bar is Foo…', false))
+                ->isEqualTo('Foo<span class="highlight-space" title="White space"> </span>is<span class="highlight-space" title="White space"> </span>bar<span class="highlight-gray" title="Non breakable space"> </span>;<span class="highlight-gray" title="Non breakable space"> </span>Bar<span class="highlight-space" title="White space"> </span>is<span class="highlight-space" title="White space"> </span>Foo<span class="highlight-gray" title="Real ellipsis">…</span>');
+    }
+
     public function getLengthDP()
     {
         return [

diff --git a/web/style/transvision.css b/web/style/transvision.css
@@ -474,18 +474,18 @@ table tr td.no_highligth {
     white-space: pre-wrap; /* That allows exposing contiguous white spaces */
 }
 
-.highlight-specialchars .highlight-gray {
+.highlight-gray {
     background-color: rgba(0, 0, 0, 0.2);
 }
 
-.highlight-specialchars .highlight-space {
+.highlight-space {
     background-color: rgba(0, 0, 0, 0.1);
     display: inline-block;
     height: 0.5em;
     vertical-align: middle;
 }
 
-.highlight-specialchars .highlight-red {
+.highlight-red {
     background-color: rgba(255, 0, 0, 0.4);
 }