From c2a21f0f9fc23b6989ba186d8475e37286ff38f0 Mon Sep 17 00:00:00 2001
From: John McLear <john@mclear.co.uk>
Date: Sun, 17 May 2026 19:04:12 +0100
Subject: [PATCH 1/2] fix(l10n): short-circuit form controls in translateNode
 (no spurious warning)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`<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>
---
 src/static/js/vendors/html10n.ts              | 21 ++++++-----
 .../specs/html10n_form_controls_aria.spec.ts  | 35 +++++++++++++++++++
 2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/src/static/js/vendors/html10n.ts b/src/static/js/vendors/html10n.ts
index 821a4d0771c..bb6d758ccba 100644
--- a/src/static/js/vendors/html10n.ts
+++ b/src/static/js/vendors/html10n.ts
@@ -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()
-      }
     }
   }
 
diff --git a/src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts b/src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts
index 8508af5b690..ebd899a2960 100644
--- a/src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts
+++ b/src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts
@@ -59,6 +59,41 @@ 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.
+  const warnings: string[] = [];
+  page.on('console', (msg) => {
+    if (msg.type() === 'warning') warnings.push(msg.text());
+  });
+
+  await page.evaluate(() => {
+    const sel = document.createElement('select');
+    sel.id = 'html10n-test-no-warn';
+    sel.setAttribute('data-l10n-id', 'pad.toolbar.bold.title');
+    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);
+    // @ts-ignore
+    window.html10n.localize(['en']);
+  });
+
+  // Give html10n a tick to run.
+  await page.waitForTimeout(50);
+
+  const offending = warnings.filter((m) =>
+      m.includes('could not translate element content') &&
+      m.includes('pad.toolbar.bold.title'));
+  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

From cd54278159bac84a33ee6b5b875ab5b4ef818267 Mon Sep 17 00:00:00 2001
From: John McLear <john@mclear.co.uk>
Date: Sun, 17 May 2026 19:19:43 +0100
Subject: [PATCH 2/2] fix(test): actually exercise the form-control
 short-circuit branch
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 .../specs/html10n_form_controls_aria.spec.ts  | 22 +++++++++++++------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts b/src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts
index ebd899a2960..da03cbf1b05 100644
--- a/src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts
+++ b/src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts
@@ -65,15 +65,21 @@ test('no "could not translate element content" warning for <select data-l10n-id>
   // <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(() => {
+  await page.evaluate(() => new Promise<void>((res) => {
     const sel = document.createElement('select');
     sel.id = 'html10n-test-no-warn';
-    sel.setAttribute('data-l10n-id', 'pad.toolbar.bold.title');
+    sel.setAttribute('data-l10n-id', 'pad.loading');
     for (const v of ['a', 'b', 'c']) {
       const opt = document.createElement('option');
       opt.value = v;
@@ -81,16 +87,18 @@ test('no "could not translate element content" warning for <select data-l10n-id>
       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']);
-  });
-
-  // Give html10n a tick to run.
-  await page.waitForTimeout(50);
+  }));
 
   const offending = warnings.filter((m) =>
       m.includes('could not translate element content') &&
-      m.includes('pad.toolbar.bold.title'));
+      m.includes('pad.loading'));
   expect(offending).toEqual([]);
 });