Skip to content

Commit 9b2e370

Browse files
JohnMcLearclaude
andauthored
fix: use placeholder-free key for suggestion display label (#273) (#369)
The display-suggestion template's label used `ep_comments_page.comments_template.suggested_change_from`, whose English value is "Suggested change from \"{{changeFrom}}\" to \"{{changeTo}}\"". The span had no `data-l10n-args`, so the {{changeFrom}} / {{changeTo}} placeholders rendered as literal text when replying with a suggestion. Introduce a new key `ep_comments_page.comments_template.suggested_change_from_label` that is plain "Suggested change from" (no placeholders). The adjacent `.from-value` / `.to-value` spans already render the quoted values, so the label should only carry the static prefix. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 45b3995 commit 9b2e370

3 files changed

Lines changed: 62 additions & 1 deletion

File tree

locales/en.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
"ep_comments_page.comments_template.accept_change.value" : "Accept Change",
1313
"ep_comments_page.comments_template.revert_change.value" : "Revert Change",
1414
"ep_comments_page.comments_template.suggested_change_from" : "Suggested change from \"{{changeFrom}}\" to \"{{changeTo}}\"",
15+
"ep_comments_page.comments_template.suggested_change_from_label" : "Suggested change from",
1516
"ep_comments_page.comments_template.suggest_change_from" : "Suggest change from \"{{changeFrom}}\" to",
1617
"ep_comments_page.comments_template.to" : "To",
1718
"ep_comments_page.comments_template.include_suggestion" : "Include suggested change",

static/tests/backend/specs/l10n.js

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
'use strict';
2+
3+
const assert = require('assert').strict;
4+
const fs = require('fs');
5+
const path = require('path');
6+
7+
const pluginRoot = path.resolve(__dirname, '..', '..', '..', '..');
8+
const enPath = path.join(pluginRoot, 'locales', 'en.json');
9+
const commentsTemplatePath = path.join(pluginRoot, 'templates', 'comments.html');
10+
11+
const readJSON = (p) => JSON.parse(fs.readFileSync(p, 'utf8'));
12+
13+
describe(__filename, function () {
14+
let en;
15+
let template;
16+
17+
before(function () {
18+
en = readJSON(enPath);
19+
template = fs.readFileSync(commentsTemplatePath, 'utf8');
20+
});
21+
22+
it('every data-l10n-id referenced in comments.html exists in en.json', function () {
23+
const ids = new Set();
24+
const re = /data-l10n-id="([^"]+)"/g;
25+
let m;
26+
while ((m = re.exec(template)) !== null) ids.add(m[1]);
27+
assert(ids.size > 0, 'expected at least one data-l10n-id in comments.html');
28+
for (const id of ids) {
29+
// Some data-l10n-id values are computed via jquery template syntax like
30+
// `{{if reply}}...{{/if}}`. Enumerate the possible keys in that case.
31+
if (id.includes('{{if reply}}')) {
32+
const a = id.replace('{{if reply}}reply{{else}}comment{{/if}}', 'reply');
33+
const b = id.replace('{{if reply}}reply{{else}}comment{{/if}}', 'comment');
34+
assert(Object.prototype.hasOwnProperty.call(en, a), `missing ${a}`);
35+
assert(Object.prototype.hasOwnProperty.call(en, b), `missing ${b}`);
36+
continue;
37+
}
38+
assert(Object.prototype.hasOwnProperty.call(en, id),
39+
`missing translation for data-l10n-id "${id}"`);
40+
}
41+
});
42+
43+
it('suggestion label does not use a key with unresolved placeholders (regression for #273)',
44+
function () {
45+
// The display-suggestion template previously used
46+
// `suggested_change_from` whose English value contains {{changeFrom}} /
47+
// {{changeTo}}. No data-l10n-args is provided on the span, so the
48+
// placeholders were rendered as literal text. The replacement label
49+
// key must not require any arguments.
50+
const labelKey = 'ep_comments_page.comments_template.suggested_change_from_label';
51+
assert(Object.prototype.hasOwnProperty.call(en, labelKey),
52+
`expected ${labelKey} in en.json`);
53+
assert(!/\{\{/.test(en[labelKey]),
54+
`${labelKey} must not contain {{placeholders}}; got: ${en[labelKey]}`);
55+
56+
// The template references the new label key.
57+
assert(template.includes(`data-l10n-id="${labelKey}"`),
58+
`comments.html should reference ${labelKey} for the suggestion display label`);
59+
});
60+
});

templates/comments.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ <h1 data-l10n-id="ep_comments_page.comment">Comment</h1>
9797
{{if changeTo}}
9898
<form class="comment-changeTo-form suggestion-display">
9999
<div>
100-
<span class="from-label" data-l10n-id="ep_comments_page.comments_template.suggested_change_from">Suggested Change From</span>
100+
<span class="from-label" data-l10n-id="ep_comments_page.comments_template.suggested_change_from_label">Suggested change from</span>
101101
<span class="hidden from-value">${changeFrom}</span>
102102
<span class="hidden to-value">${changeTo}</span>
103103
</div>

0 commit comments

Comments
 (0)