fix(l10n): silence spurious 'could not translate element content' warning on form controls (#7797)

* fix(l10n): short-circuit form controls in translateNode (no spurious warning)

`<select data-l10n-id="...">` with `<option>` element children — the
pattern used by ep_headings2, ep_align, ep_font_size, ep_font_family, …
— used to drop into the textContent branch of html10n.translateNode and
hunt for a text-node child to overwrite. There is none, so the loop
exited with `found = false` and emitted:

  Unexpected error: could not translate element content for key ep_headings.style

The SELECT/INPUT/TEXTAREA aria-label fallback already lived inside the
same else-branch, *after* the warning, so the accessible name landed
correctly but the noisy console line still fired on every pad load.

Move the form-control case into its own `else if`, before the text-node
hunt: aria-label is the only sensible localization target for these
elements (a <select>'s text is its <option> labels, not its own name).

Closes the console warning reported on Etherpad 3.1.0.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(test): actually exercise the form-control short-circuit branch

Qodo's review on #7797 caught two real test bugs:

1. `pad.toolbar.bold.title` ends in `.title`, which is in html10n's
   attribute allowlist. translateNode picks `prop = 'title'`, takes
   the first branch (node[prop] = str.str), and never reaches the
   textContent path where the warning lives. The test would pass
   without my fix.

   Switched the key to `pad.loading` — same stable pad-bundle
   translation, but the suffix isn't in the allowlist, so `prop`
   defaults to `textContent` and the test actually exercises the
   regression path.

2. `page.waitForTimeout(50)` is a fixed sleep, but `html10n.localize`
   runs its work inside a `build()` callback, so completion timing
   depends on the loader. Replaced with a deterministic `html10n.mt
   .bind('localized', …)` await.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
John McLear 2026-05-17 19:59:00 +01:00 committed by GitHub
parent 10558ed115
commit acc7a5dd4f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 53 additions and 11 deletions

View file

@ -680,6 +680,16 @@ export class Html10n {
// @ts-ignore
node[prop] = str.str!
populateAriaLabel()
} else if (node.tagName === 'SELECT' || node.tagName === 'INPUT' ||
node.tagName === 'TEXTAREA') {
// Form-controllable elements carry their accessible name on aria-label
// rather than as text content — a <select>'s text is its <option>
// labels, not its own name. Plugins that put `data-l10n-id` on a
// <select> (ep_headings2, ep_align, ep_font_size, …) used to trigger a
// spurious "could not translate element content" warning here because
// the text-node hunt below finds nothing to write. Short-circuit and
// just localize aria-label. See ether/ep_align#182 review.
populateAriaLabel()
} else {
let children = node.childNodes,
found = false
@ -697,17 +707,6 @@ export class Html10n {
if (!found) {
console.warn('Unexpected error: could not translate element content for key '+str.id, node)
}
// Form-controllable elements (<select>, <input>, <textarea>) carry their
// accessible name on aria-label rather than as text content (a <select>'s
// text is its <option> labels, not its own name). The textContent branch
// above doesn't fall through to populateAriaLabel(), so plugins that put
// data-l10n-id on a <select> and rely on the auto-population (introduced
// in #7584) end up with no accessible name. Populate aria-label here so
// those controls stay localized too. See ether/ep_align#182 review.
const tag = node.tagName;
if (tag === 'SELECT' || tag === 'INPUT' || tag === 'TEXTAREA') {
populateAriaLabel()
}
}
}

View file

@ -59,6 +59,49 @@ test('html10n auto-populates aria-label on <textarea> with data-l10n-id', async
await expect(ta).toHaveAttribute('data-l10n-aria-label', 'true');
});
test('no "could not translate element content" warning for <select data-l10n-id>', async ({page}) => {
// Regression: thm reported the warning on Etherpad 3.1.0 because the
// <select data-l10n-id="ep_headings.style"> in ep_headings2 has only
// <option> element children — the text-node hunt in translateNode finds
// nothing and used to warn before the SELECT/INPUT/TEXTAREA short-circuit
// landed. The accessible name still ends up on aria-label.
//
// To actually exercise the bug path the key must default `prop` to
// textContent (i.e. the suffix after the last `.` must NOT be one of
// title|innerHTML|alt|textContent|value|placeholder, see
// html10n.translateNode). `pad.loading` is a stable pad-bundle key that
// satisfies that.
const warnings: string[] = [];
page.on('console', (msg) => {
if (msg.type() === 'warning') warnings.push(msg.text());
});
await page.evaluate(() => new Promise<void>((res) => {
const sel = document.createElement('select');
sel.id = 'html10n-test-no-warn';
sel.setAttribute('data-l10n-id', 'pad.loading');
for (const v of ['a', 'b', 'c']) {
const opt = document.createElement('option');
opt.value = v;
opt.textContent = v.toUpperCase();
sel.appendChild(opt);
}
document.body.appendChild(sel);
// localize completes asynchronously inside a build() callback; wait
// on the `localized` event instead of a fixed timeout so the test
// is deterministic on slow CI runners.
// @ts-ignore window.html10n is exposed by pad.ts
window.html10n.mt.bind('localized', () => res());
// @ts-ignore
window.html10n.localize(['en']);
}));
const offending = warnings.filter((m) =>
m.includes('could not translate element content') &&
m.includes('pad.loading'));
expect(offending).toEqual([]);
});
test('an author-supplied aria-label on a form control is preserved', async ({page}) => {
// Mirror the existing semantics for non-form-control elements: if the
// template author wrote their own aria-label, html10n must not