Display the same info in each mainsearch search type (#762)

* Display the same info in each mainsearch search type
* Move error check to a function and test it, stop showing meta links when there is no string
* Fix capitalization + undefined variable
* Add warning on all translations view
* Only hide Translate with, the clipboard icon and string errors

app/classes/Transvision/ShowResults.php

@@ -340,6 +340,7 @@ public static function resultsTable($search_object, $search_results, $page)
             ];
 
             $target_string = Strings::multipleStringReplace($replacements, $target_string);
+            $clipboard_target_string  = 'clip_' . md5($target_string);
 
             $temp = explode('-', $locale1);
             $locale1_short_code = $temp[0];
@@ -355,49 +356,46 @@ public static function resultsTable($search_object, $search_results, $page)
                 $locale2_path = VersionControl::hgPath($locale2, $current_repo, $key);
             }
 
-            // Errors
-            $error_message = '';
-
-            // Check for final dot
-            if (substr(strip_tags($source_string), -1) == '.'
-                && substr(strip_tags($target_string), -1) != '.') {
-                $error_message = '<em class="error"> No final dot?</em>';
-            }
+            // Get the potential errors for $target_string (final dot, long/small string)
+            $error_message = ShowResults::buildErrorString($source_string, $target_string);
 
-            // Check abnormal string length
-            $length_diff = Utils::checkAbnormalStringLength($source_string, $target_string);
-            if ($length_diff) {
-                switch ($length_diff) {
-                    case 'small':
-                        $error_message = $error_message . '<em class="error"> Small string?</em>';
-                        break;
-                    case 'large':
-                        $error_message = $error_message . '<em class="error"> Large String?</em>';
-                        break;
-                }
-            }
+            // Don't show meta links by default
+            $meta_source = $meta_target = $meta_target2 = '';
 
-            // Missing string error
+            // If there is no source_string, display an error, otherwise display the string + meta links
             if (! $source_string) {
-                $source_string = '<em class="error">warning: missing string</em>';
-                $error_message = '';
+                $source_string = '<em class="error">Warning: Source string is empty</em>';
+            } else {
+                $meta_source = "
+                  <span>Translate with:</span>
+                  <a href='http://translate.google.com/#{$locale1_short_code}/{$locale2_short_code}/"
+                  // We use html_entity_decode twice because we can have strings as &amp;amp; stored
+                  . urlencode(strip_tags(html_entity_decode(html_entity_decode($source_string))))
+                  . "' target='_blank'>Google</a>
+                  <a href='http://www.bing.com/translator/?from={$locale1_short_code}&to={$locale2_short_code}&text="
+                  . urlencode(strip_tags(html_entity_decode(html_entity_decode($source_string))))
+                  . "' target='_blank'>BING</a>";
             }
 
+            // If there is no target_string, display an error, otherwise display the string + meta links
             if (! $target_string) {
-                $target_string = '<em class="error">warning: missing string</em>';
-                $error_message = '';
+                $target_string = '<em class="error">Warning: Missing string</em>';
+            } else {
+                $meta_target = "
+                      <span class='clipboard' data-clipboard-target='#{$clipboard_target_string}' alt='Copy to clipboard'><img src='/img/copy_icon_black_18x18.png'></span>
+                      {$error_message}";
             }
 
+            // If there is no target_string2, display an error, otherwise display the string + meta links
             if (! $target_string2) {
-                $target_string2 = '<em class="error">warning: missing string</em>';
-                $error_message = '';
+                $target_string2 = '<em class="error">Warning: Missing string</em>';
+            } else {
+                $meta_target2 = "<span class='clipboard' data-clipboard-target='#{$clipboard_target_string2}' alt='Copy to clipboard'><img src='/img/copy_icon_black_18x18.png'></span>";
             }
 
             // Replace / and : in the key name and use it as an anchor name
             $anchor_name = str_replace(['/', ':'], '_', $key);
 
-            $clipboard_target_string  = 'clip_' . md5($target_string);
-
             // 3locales view
             if ($extra_locale) {
                 if (in_array($current_repo, ['firefox_ios', 'mozilla_org'])) {
@@ -420,9 +418,9 @@ public static function resultsTable($search_object, $search_results, $page)
                       <a class='bug_link' target='_blank' href='{$bz_link[1]}'>
                         &lt;report a bug&gt;
                       </a>
-                      <span class='clipboard' data-clipboard-target='#{$clipboard_target_string2}' alt='Copy to clipboard'><img src='/img/copy_icon_black_18x18.png'></span>
+                      {$meta_target2}
                     </div>
-                  </td>";
+                </td>";
             } else {
                 $extra_column_rows = '';
             }
@@ -443,14 +441,7 @@ public static function resultsTable($search_object, $search_results, $page)
                       <a class='source_link' href='{$locale1_path}'>
                         &lt;source&gt;
                       </a>
-                      <span>Translate with:</span>
-                      <a href='http://translate.google.com/#{$locale1_short_code}/{$locale2_short_code}/"
-                      // We use html_entity_decode twice because we can have strings as &amp;amp; stored
-                      . urlencode(strip_tags(html_entity_decode(html_entity_decode($source_string))))
-                      . "' target='_blank'>Google</a>
-                      <a href='http://www.bing.com/translator/?from={$locale1_short_code}&to={$locale2_short_code}&text="
-                      . urlencode(strip_tags(html_entity_decode(html_entity_decode($source_string))))
-                      . "' target='_blank'>BING</a>
+                      {$meta_source}
                     </div>
                   </td>
 
@@ -465,8 +456,7 @@ public static function resultsTable($search_object, $search_results, $page)
                       <a class='bug_link' target='_blank' href='{$bz_link[0]}'>
                         &lt;report a bug&gt;
                       </a>
-                      <span class='clipboard' data-clipboard-target='#{$clipboard_target_string}' alt='Copy to clipboard'><img src='/img/copy_icon_black_18x18.png'></span>
-                      {$error_message}
+                      {$meta_target}
                     </div>
                   </td>
                 {$extra_column_rows}
@@ -508,4 +498,45 @@ public static function searchEntities($source_strings, $regex)
 
         return $entities;
     }
+
+    /**
+     * Build the error message for target string. Can return a combination of
+     * errors, including missing final dot, long string and short string.
+     *
+     * @param string $source_string String from the source locale
+     * @param string $target_string String from the target locale
+     *
+     * @return string A concatenated string of errors or an empty string if
+     *                there is no error.
+     */
+    public static function buildErrorString($source_string, $target_string)
+    {
+        $error_message = '';
+
+        // Check for final dot
+        if (substr(strip_tags($source_string), -1) == '.'
+            && substr(strip_tags($target_string), -1) != '.') {
+            $error_message = '<em class="error">No final dot?</em> ';
+        }
+
+        // Check abnormal string length
+        $length_diff = Utils::checkAbnormalStringLength($source_string, $target_string);
+        if ($length_diff) {
+            switch ($length_diff) {
+                case 'small':
+                    $error_message = $error_message . '<em class="error">Small string?</em> ';
+                    break;
+                case 'large':
+                    $error_message = $error_message . '<em class="error">Large string?</em> ';
+                    break;
+            }
+        }
+
+        // Missing string error
+        if (! $source_string || ! $target_string) {
+            $error_message = '';
+        }
+
+        return $error_message;
+    }
 }

app/views/check_variables.php

@@ -53,14 +53,14 @@
                           <span class='celltitle'>{$source_locale}</span>
                           <div class='string'>" . Utils::secureText($source[$string_id]) . "</div>
                           <div class='result_meta_link'>
-                            <a class='source_link' href='{$path_locale1}'><em>&lt;source&gt;</em></a>
+                            <a class='source_link' href='{$path_locale1}'>&lt;source&gt;</a>
                           </div>
                        </td>
                         <td dir='{$direction2}'>
                           <span class='celltitle'>$locale</span>
                           <div class='string'>" . Utils::secureText($target[$string_id]) . "</div>
                           <div class='result_meta_link'>
-                            <a class='source_link' href='{$path_locale2}'><em>&lt;source&gt;</em></a>
+                            <a class='source_link' href='{$path_locale2}'>&lt;source&gt;</a>
                             <a class='bug_link' target='_blank' href='{$bugzilla_link}'>&lt;report a bug&gt;</a>
                           </div>
                        </td>

app/views/onestring.php

@@ -31,10 +31,16 @@
         $rtl_support = RTLSupport::isRTL($locale) ? 'dir="rtl"' : '';
         $search_link = "/?sourcelocale={$reference_locale}&locale={$locale}&repo={$repo}&search_type=entities&recherche={$entity}&perfect_match=perfect_match";
         echo "<tr id='{$locale}'>\n" .
-             "  <th><a href='#{$locale}'>{$locale}</a></th>\n" .
-             "  <td lang='#{$locale}' {$rtl_support} >" . Utils::secureText($translation) . "</td>\n" .
-             "  <td><a class='onestring_search' href='{$search_link}' title='Search for the entity in this locale'>🔍</a></td>\n" .
-             "</tr>\n";
+             "  <th><a href='#{$locale}'>{$locale}</a></th>\n";
+
+        if (! $translation) {
+            echo "  <td><em class='error'>Warning: Missing string</em></td><td></td>\n";
+        } else {
+            echo "  <td lang='#{$locale}' {$rtl_support} >" . Utils::secureText($translation) . "</td>\n" .
+                 "  <td><a class='onestring_search' href='{$search_link}' title='Search for the entity in this locale'>🔍</a></td>\n";
+        }
+
+        "</tr>\n";
     }
     ?>
 </table>

app/views/results_entities.php

@@ -61,19 +61,31 @@
                                               $bz_target_string2, $current_repo, $entity_link)
                   . '">&lt;report a bug&gt;</a>';
 
+        // If there is no target_string2, display an error
+        if (! $target_string2) {
+            $target_string2 = '<em class="error">Warning: Missing string</em>';
+        }
+
         $extra_column_rows = "
     <td dir='{$direction3}'>
       <span class='celltitle'>{$locale2}</span>
       <div class='string'>{$target_string2}</div>
       <div dir='ltr' class='result_meta_link'>
-        <a class='source_link' href='{$path_locale3}'><em>&lt;source&gt;</em></a>
+        <a class='source_link' href='{$path_locale3}'>&lt;source&gt;</a>
         {$file_bug}
       </div>
     </td>";
     } else {
         $extra_column_rows = '';
     }
 
+    // Locale codes for machine translation services
+    $temp = explode('-', $source_locale);
+    $locale1_short_code = $temp[0];
+
+    $temp = explode('-', $locale);
+    $locale2_short_code = $temp[0];
+
     // Link to entity
     $entity_link = "?sourcelocale={$source_locale}"
                  . "&locale={$locale}"
@@ -85,6 +97,39 @@
                                           $bz_target_string, $current_repo, $entity_link)
               . '">&lt;report a bug&gt;</a>';
     $anchor_name = str_replace(['/', ':'], '_', $entity);
+
+    $clipboard_target_string  = 'clip_' . md5($target_string);
+
+    // Get the potential errors for $target_string (final dot, long/small string)
+    $error_message = ShowResults::buildErrorString($source_string, $target_string);
+
+    // Don't show meta links by default
+    $meta_source = $meta_target = '';
+
+    // If there is no source_string, display an error, otherwise display the string + meta links
+    if (! $source_string) {
+        $source_string = '<em class="error">Warning: Source string is empty</em>';
+    } else {
+        $meta_source =  "
+          <span>Translate with:</span>
+          <a href='http://translate.google.com/#{$locale1_short_code}/{$locale2_short_code}/"
+          // We use html_entity_decode twice because we can have strings as &amp;amp; stored
+          . urlencode(strip_tags(html_entity_decode(html_entity_decode($source_string))))
+          . "' target='_blank'>Google</a>
+          <a href='http://www.bing.com/translator/?from={$locale1_short_code}&to={$locale2_short_code}&text="
+          . urlencode(strip_tags(html_entity_decode(html_entity_decode($source_string))))
+          . "' target='_blank'>BING</a>";
+    }
+
+    // If there is no target_string, display an error, otherwise display the string + meta links
+    if (! $target_string) {
+        $target_string = '<em class="error">Warning: Missing string</em>';
+    } else {
+        $meta_target = "
+          {$error_message}
+          <span class='clipboard' data-clipboard-target='#{$clipboard_target_string}' alt='Copy to clipboard'><img src='/img/copy_icon_black_18x18.png'></span>";
+    }
+
     $table .= "
   <tr>
     <td>
@@ -97,15 +142,17 @@
       <span class='celltitle'>{$source_locale}</span>
       <div class='string'>{$source_string}</div>
       <div dir='ltr' class='result_meta_link'>
-        <a class='source_link' href='{$path_locale1}'><em>&lt;source&gt;</em></a>
+        <a class='source_link' href='{$path_locale1}'>&lt;source&gt;</a>
+        {$meta_source}
       </div>
     </td>
     <td dir='{$direction2}'>
       <span class='celltitle'>{$locale}</span>
-      <div class='string'>{$target_string}</div>
+      <div class='string' id='{$clipboard_target_string}'>{$target_string}</div>
       <div dir='ltr' class='result_meta_link'>
-        <a class='source_link' href='{$path_locale2}'><em>&lt;source&gt;</em></a>
+        <a class='source_link' href='{$path_locale2}'>&lt;source&gt;</a>
         {$file_bug}
+        {$meta_target}
       </div>
     </td>
     {$extra_column_rows}

tests/units/Transvision/ShowResults.php

@@ -299,4 +299,56 @@ public function testGetSuggestionsResults()
             ->array($obj->getSuggestionsResults($source, $target, 'Bookmark', 3))
                 ->isEqualTo(['Bookmark', 'Nouveaux marque-pages']);
     }
+
+    public function buildErrorStringDP()
+    {
+        return [
+            [
+                'Le système de marque-pages et.',
+                'Le système de marque-pages et',
+                '<em class="error">No final dot?</em> ',
+            ],
+            [
+                'Le système de marque-pages et',
+                'Le système de marque-pages et',
+                '',
+            ],
+            [
+                'Le système de marque-pages et.',
+                'Le système de marque-pages et.',
+                '',
+            ],
+            [
+                'The bookmarks and history system will not be functional because one of files is in use by another application. Some security software can cause this problem.',
+                'Le système de marque-pages et',
+                '<em class="error">No final dot?</em> <em class="error">Small string?</em> ',
+            ],
+            [
+                'The bookmarks and history system will not be functional because one of files is in use by another application. Some security software can cause this problem.',
+                'Le système de marque-pages et.',
+                '<em class="error">Small string?</em> ',
+            ],
+            [
+                'The bookmarks and history system will not be functional because one of files is in use by another application. Some security software can cause this problem.',
+                'Le système de marque-pages et dhistorique ne sera pas opérationnel car lun des fichiers de %S est en cours dutilisation par une autre application. Certains logiciels de sécurité peuvent causer ce problème. Le système de marque-pages et dhistorique ne sera pas opérationnel car lun des fichiers de %S est en cours dutilisation par une autre application. Certains logiciels de sécurité peuvent causer ce problème',
+                '<em class="error">No final dot?</em> <em class="error">Large string?</em> ',
+            ],
+            [
+                'Le système de marque-pages et',
+                'Le système de marque-pages et dhistorique ne sera pas opérationnel car lun des fichiers de %S est en cours dutilisation par une autre application. Certains logiciels de sécurité peuvent causer ce problème. Le système de marque-pages et dhistorique ne sera pas opérationnel car lun des fichiers de %S est en cours dutilisation par une autre application. Certains logiciels de sécurité peuvent causer ce problème.',
+                '<em class="error">Large string?</em> ',
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider buildErrorStringDP
+     */
+    public function testBuildErrorString($a, $b, $c)
+    {
+        $obj = new _ShowResults();
+        $this
+            ->string($obj->buildErrorString($a, $b))
+                ->isEqualTo($c);
+    }
 }