Skip to content

Commit 17373b2

Browse files
flodoloTheoChevalier
authored andcommitted
Create QA view for Empty strings (fixes #813) (#814)
1 parent 1075320 commit 17373b2

11 files changed

Lines changed: 254 additions & 21 deletions

File tree

app/classes/Transvision/Bugzilla.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ public static function reportErrorLink($locale, $entity, $source_string, $target
4949
{
5050
$bug_summary = rawurlencode("[{$locale}] Translation update proposed for {$entity}");
5151
$transvision_url = "https://transvision.mozfr.org/{$entity_link}";
52+
$source_string = $source_string != ''
53+
? $source_string
54+
: '(empty string)';
55+
$target_string = $target_string != ''
56+
? $target_string
57+
: '(empty string)';
5258
$bug_message = rawurlencode(
5359
html_entity_decode(
5460
"The string:\n{$source_string}\n\n"

app/classes/Transvision/Strings.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,11 @@ public static function multipleStringReplace($replacements, $string)
115115
public static function highlightSpecial($string, $exclude_whitespaces = true)
116116
{
117117
$replacements = [
118-
' ' => '<span class="highlight-space" title="White space"> </span>',
119-
' ' => '<span class="highlight-gray" title="Non breakable space"> </span>',
120-
'' => '<span class="highlight-red" title="Narrow no-break space"> </span>',
121-
'' => '<span class="highlight-gray" title="Real ellipsis">…</span>',
122-
'&hellip;' => '<span class="highlight-red" title="HTML ellipsis">…</span>',
118+
' ' => '<span class="highlight-special highlight-space" title="White space"> </span>',
119+
' ' => '<span class="highlight-special highlight-gray" title="Non breakable space"> </span>',
120+
'' => '<span class="highlight-special highlight-red" title="Narrow no-break space"> </span>',
121+
'' => '<span class="highlight-special highlight-gray" title="Real ellipsis">…</span>',
122+
'&hellip;' => '<span class="highlight-special highlight-red" title="HTML ellipsis">…</span>',
123123
];
124124

125125
if ($exclude_whitespaces) {

app/controllers/empty_strings.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
namespace Transvision;
3+
4+
// Get requested repo and locale
5+
require_once INC . 'l10n-init.php';
6+
7+
include MODELS . 'empty_strings.php';
8+
include VIEWS . 'empty_strings.php';

app/inc/dispatcher.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,14 @@
7171
$css_files[] = 'tmx.css';
7272
$js_files[] = '/js/select_all.js';
7373
break;
74+
case 'empty-strings':
75+
$experimental = true;
76+
$controller = 'empty_strings';
77+
$page_title = 'Empty Strings';
78+
$page_descr = '';
79+
$js_files[] = '/js/component_filter.js';
80+
$js_files[] = '/js/sorttable.js';
81+
break;
7482
case 'news':
7583
$controller = 'changelog';
7684
$page_title = 'Transvision News and Release Notes';

app/inc/urls.php

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,23 @@
22

33
$urls = [
44
'/' => 'root',
5-
'news' => 'changelog',
6-
'rss' => 'rss',
7-
'stats' => 'stats',
5+
'3locales' => '3locales',
6+
'accesskeys' => 'keys',
7+
'api' => 'api',
88
'channelcomparison' => 'channelcomp',
99
'consistency' => 'consistency',
10-
'accesskeys' => 'keys',
1110
'credits' => 'credits',
1211
'downloads' => 'downloads',
12+
'empty-strings' => 'empty_strings',
13+
'news' => 'changelog',
14+
'productization' => 'productization',
15+
'rss' => 'rss',
1316
'showrepos' => 'showrepos',
17+
'stats' => 'stats',
18+
'string' => 'onestring',
1419
'unchanged' => 'unchangedstrings',
15-
'unlocalized' => 'unlocalized',
1620
'unlocalized-all' => 'unlocalized_all',
1721
'unlocalized-json' => 'unlocalized_json',
22+
'unlocalized' => 'unlocalized',
1823
'variables' => 'checkvariables',
19-
'3locales' => '3locales',
20-
'string' => 'onestring',
21-
'productization' => 'productization',
22-
'api' => 'api',
2324
];

app/models/empty_strings.php

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
namespace Transvision;
3+
4+
// Build arrays for the search form, ignore mozilla.org and iOS
5+
$channel_selector = Utils::getHtmlSelectOptions(
6+
array_intersect_key(
7+
$repos_nice_names,
8+
array_flip($desktop_repos)
9+
),
10+
$repo,
11+
true
12+
);
13+
14+
$target_locales_list = Utils::getHtmlSelectOptions(
15+
Project::getRepositoryLocales($repo),
16+
$locale
17+
);
18+
19+
$reference_locale = Project::getReferenceLocale($repo);
20+
21+
$reference_strings = Utils::getRepoStrings($reference_locale, $repo);
22+
$locale_strings = Utils::getRepoStrings($locale, $repo);
23+
24+
/*
25+
Identify empty strings in reference locale, store the translation
26+
only if locale has a non empty string for that ID. Then repeat the
27+
process for the locale.
28+
*/
29+
$empty_strings = [];
30+
31+
$empty_reference = array_filter($reference_strings, function ($string) {
32+
return strlen($string) == 0;
33+
});
34+
foreach ($empty_reference as $string_id => $value) {
35+
if (isset($locale_strings[$string_id]) && $locale_strings[$string_id] != '') {
36+
$empty_strings[$string_id] = [
37+
'reference' => $value,
38+
'translation' => $locale_strings[$string_id],
39+
];
40+
}
41+
}
42+
43+
$empty_locale = array_filter($locale_strings, function ($string) {
44+
return strlen($string) == 0;
45+
});
46+
foreach ($empty_locale as $string_id => $value) {
47+
if (isset($reference_strings[$string_id]) && $reference_strings[$string_id] != '') {
48+
$empty_strings[$string_id] = [
49+
'reference' => $reference_strings[$string_id],
50+
'translation' => $value,
51+
];
52+
}
53+
}
54+
55+
unset($reference_strings);
56+
unset($locale_strings);
57+
58+
if (count($empty_strings) > 0) {
59+
ksort($empty_strings);
60+
$components = Project::getComponents($empty_strings);
61+
$filter_block = '';
62+
foreach ($components as $value) {
63+
$filter_block .= " <a href='#{$value}' id='{$value}' class='filter'>{$value}</a>";
64+
}
65+
}

app/views/empty_strings.php

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
<?php
2+
namespace Transvision;
3+
4+
?>
5+
<p class="intro">This view displays strings that are empty in the reference language
6+
but have a translation in the requested language, and viceversa.
7+
It can be used to identify strings that should have been localized and
8+
are empty by mistake, and strings that should actually remain empty.
9+
</p>
10+
<form name="searchform" id="simplesearchform" method="get" action="">
11+
<fieldset id="main_search">
12+
13+
<?php if (isset($target_locales_list)) : ?>
14+
<fieldset>
15+
<label>Locale</label>
16+
<div class="select-style">
17+
<select name="locale" title="Locale" id="simplesearch_locale">
18+
<?=$target_locales_list?>
19+
</select>
20+
</div>
21+
</fieldset>
22+
<?php endif; ?>
23+
24+
<?php if (isset($channel_selector)) : ?>
25+
<fieldset>
26+
<label>Repository</label>
27+
<div class="select-style">
28+
<select name="repo" title="Repository" id="simplesearch_repository">
29+
<?=$channel_selector?>
30+
</select>
31+
</div>
32+
</fieldset>
33+
<?php endif; ?>
34+
35+
<input type="submit" value="Go" alt="Go" />
36+
</fieldset>
37+
</form>
38+
<?php if (isset($filter_block)) {
39+
?>
40+
<div id="filters">
41+
<h4>Filter by folder:</h4>
42+
<a href="#showall" id="showall" class="filter">Show all results</a>
43+
<?=$filter_block; ?>
44+
</div>
45+
<?php
46+
47+
}
48+
49+
if (count($empty_strings) == 0) {
50+
echo "<div class=\"message\"><p>No strings found.</p></div>";
51+
} else {
52+
$text_direction = RTLSupport::getDirection($locale);
53+
$table = "<table class='collapsable results_table sortable'>
54+
<thead>
55+
<tr class='column_headers'>
56+
<th>Entity</th>
57+
<th>{$reference_locale}</th>
58+
<th>{$locale}</th>
59+
</tr>
60+
</thead>
61+
<tbody>\n";
62+
63+
foreach ($empty_strings as $key => $strings) {
64+
$entity = ShowResults::formatEntity($key);
65+
$component = explode('/', $key)[0];
66+
$reference_string = htmlspecialchars($strings['reference']);
67+
$locale_string = Strings::highlightSpecial(htmlspecialchars($strings['translation']), false);
68+
69+
$entity_link = "?sourcelocale={$reference_locale}"
70+
. "&locale={$locale}"
71+
. "&repo={$repo}"
72+
. "&search_type=entities&recherche={$key}"
73+
. "&perfect_match=perfect_match";
74+
75+
$bugzilla_link = [Bugzilla::reportErrorLink(
76+
$locale, $key, $reference_string, $locale_string, $repo, $entity_link
77+
)];
78+
79+
$reference_path = VersionControl::hgPath($reference_locale, $repo, $key);
80+
$locale_path = VersionControl::hgPath($locale, $repo, $key);
81+
82+
if (! $reference_string) {
83+
$reference_string = '<em class="error">(empty)</em>';
84+
}
85+
if ($locale_string == '@@missing@@') {
86+
$locale_string = '<em class="error">Missing string</em>';
87+
} elseif ($locale_string == '') {
88+
$locale_string = '<em class="error">(empty)</em>';
89+
}
90+
91+
// Replace / and : in the key name and use it as an anchor name
92+
$anchor_name = str_replace(['/', ':'], '_', $key);
93+
94+
$table .= "
95+
<tr class='{$component}'>
96+
<td>
97+
<span class='celltitle'>Entity</span>
98+
<a class='resultpermalink tag' id='{$anchor_name}' href='#{$anchor_name}' title='Permalink to this string'>link</a>
99+
<a class='l10n tag' href='/string/?entity={$key}&amp;repo={$repo}' title='List all translations for this entity'>l10n</a>
100+
<a class='link_to_entity' href=\"/{$entity_link}\">{$entity}</a>
101+
</td>
102+
<td dir='ltr' lang='{$reference_locale}'>
103+
<span class='celltitle'>{$reference_locale}</span>
104+
<div class='string'>
105+
{$reference_string}
106+
</div>
107+
<div dir='ltr' class='result_meta_link'>
108+
<a class='source_link' href='{$reference_path}'>
109+
&lt;source&gt;
110+
</a>
111+
</div>
112+
</td>
113+
<td dir='{$text_direction}' lang='{$locale}'>
114+
<span class='celltitle'>{$locale}</span>
115+
<div class='string'>{$locale_string}</div>
116+
<div dir='ltr' class='result_meta_link'>
117+
<a class='source_link' href='{$locale_path}'>
118+
&lt;source&gt;
119+
</a>
120+
&nbsp;
121+
<a class='bug_link' target='_blank' href='{$bugzilla_link[0]}'>
122+
&lt;report a bug&gt;
123+
</a>
124+
</div>
125+
</td>
126+
</tr>";
127+
}
128+
$table .= " </tbody>\n</table>\n";
129+
130+
echo $table;
131+
}

app/views/templates/base.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
<ul>
4646
{$li_link('keys', 'Check your access keys', 'Access Keys')}
4747
{$li_link('checkvariables', 'Check what variable differences there are from English', 'Check Variables')}
48+
{$li_link('empty_strings', 'Display empty strings in English or locale', 'Empty Strings')}
4849
{$li_link('unchangedstrings', 'Display all strings identical to English', 'Unchanged Strings')}
4950
{$li_link('unlocalized_all', 'Display common words remaining in English', 'Unlocalized Words')}
5051
</ul>

tests/functional/pages.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
['consistency/?locale=fr&repo=central', 200, 'English String', 'Available Translations'],
88
['consistency/?locale=en-US&repo=central', 200, 'No inconsistent translations found.', 'Analyze translation consistency across repositories.'],
99
['credits/', 200, 'Transvision 1.0 was created', 'Transvision is a community project under the MozFR umbrella'],
10+
['empty-strings/', 200, 'Empty Strings', 'Repository'],
1011
['downloads/', 200, 'Select which strings', 'Generate the TMX'],
1112
['news/', 200, 'Version 4.0', 'End user visible changes'],
1213
['productization/', 200, 'Show productization', 'firefox'],

tests/units/Transvision/Strings.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,13 @@ public function multipleStringReplaceDP()
9696
return [
9797
[
9898
[
99-
' ' => '<span class="highlight-gray" title="Non breakable space"> </span>', // Nbsp highlight
100-
'' => '<span class="highlight-red" title="Thin space"> </span>', // Thin space highlight
101-
'' => '<span class="highlight-gray">…</span>', // Right ellipsis highlight
102-
'&hellip;' => '<span class="highlight-gray">…</span>', // Right ellipsis highlight
99+
' ' => '<span class="highlight-special highlight-gray" title="Non breakable space"> </span>', // Nbsp highlight
100+
'' => '<span class="highlight-special highlight-red" title="Thin space"> </span>', // Thin space highlight
101+
'' => '<span class="highlight-special highlight-gray">…</span>', // Right ellipsis highlight
102+
'&hellip;' => '<span class="highlight-special highlight-gray">…</span>', // Right ellipsis highlight
103103
],
104104
'&hellip;  …',
105-
'<span class="highlight-gray">…</span><span class="highlight-gray" title="Non breakable space"> </span><span class="highlight-red" title="Thin space"> </span><span class="highlight-gray">…</span>',
105+
'<span class="highlight-special highlight-gray">…</span><span class="highlight-special highlight-gray" title="Non breakable space"> </span><span class="highlight-special highlight-red" title="Thin space"> </span><span class="highlight-special highlight-gray">…</span>',
106106
],
107107
];
108108
}
@@ -123,10 +123,10 @@ public function testHighlightSpecial()
123123
$obj = new _Strings();
124124
$this
125125
->string($obj->highlightSpecial('Foo is bar ; Bar is Foo…'))
126-
->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>');
126+
->isEqualTo('Foo is bar<span class="highlight-special highlight-gray" title="Non breakable space"> </span>;<span class="highlight-special highlight-gray" title="Non breakable space"> </span>Bar is Foo<span class="highlight-special highlight-gray" title="Real ellipsis">…</span>');
127127
$this
128128
->string($obj->highlightSpecial('Foo is bar ; Bar is Foo…', false))
129-
->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>');
129+
->isEqualTo('Foo<span class="highlight-special highlight-space" title="White space"> </span>is<span class="highlight-special highlight-space" title="White space"> </span>bar<span class="highlight-special highlight-gray" title="Non breakable space"> </span>;<span class="highlight-special highlight-gray" title="Non breakable space"> </span>Bar<span class="highlight-special highlight-space" title="White space"> </span>is<span class="highlight-special highlight-space" title="White space"> </span>Foo<span class="highlight-special highlight-gray" title="Real ellipsis">…</span>');
130130
}
131131

132132
public function markStringDP()

0 commit comments

Comments
 (0)