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

[JohnMcLear]

Summary

• <select data-l10n-id="…"> with <option> element children (ep_headings2, ep_align, ep_font_size, ep_font_family, …) used to print Unexpected error: could not translate element content for key … on every pad load. The textContent branch in html10n.translateNode hunted for a text-node child, found none, and warned — even though the existing SELECT/INPUT/TEXTAREA aria-label fallback further down still set the accessible name correctly.
• Move the form-control case into its own else if, before the text-node hunt. aria-label is the only sensible localization target for <select>/<input>/<textarea> (their text is on <option> / value, not directly on the control).
• Add a regression test that injects a <select data-l10n-id> and asserts no could not translate element content warning is emitted.

Reported by thm on Etherpad 3.1.0:

console also says: padbootstrap-…js:1 Unexpected error: could not translate element content for key ep_headings.style <select id="heading-selection" aria-label="Stil" data-l10n-id="ep_headings.style" data-l10n-aria-label="true" style="display: none;">…</select>

Existing aria-label population (PR #7584) is unchanged in observable behaviour; the new test alongside it locks in that the warning stays silent.

Test plan

• tsc --noEmit clean
• CI: full backend + frontend playwright
• New spec: html10n_form_controls_aria.spec.ts → 'no "could not translate element content" warning for <select data-l10n-id>'
• Pre-existing specs in html10n_form_controls_aria.spec.ts still pass

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Suppress spurious warning for form controls with data-l10n-id

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Eliminate spurious "could not translate element content" warning for form controls
  - Move form-control case (<select>, <input>, <textarea>) into separate branch before text-node
  hunt
  - Prevents warning when elements have only <option> children instead of text nodes
• Add regression test for <select data-l10n-id> warning suppression
  - Verifies no console warning emitted during localization
  - Ensures aria-label still populated correctly on form controls

Diagram

flowchart LR
  A["translateNode called<br/>on form control"] --> B{"Check if SELECT/<br/>INPUT/TEXTAREA"}
  B -->|Yes| C["Populate aria-label<br/>only"]
  B -->|No| D["Hunt for text-node<br/>child"]
  D -->|Found| E["Update textContent"]
  D -->|Not found| F["Warn about missing<br/>element content"]
  C --> G["No spurious warning"]
  E --> G

Loading

File Changes

1. src/static/js/vendors/html10n.ts 🐞 Bug fix +10/-11

Short-circuit form controls before text-node hunt

• Reorganized translateNode method to check for form-control elements (SELECT, INPUT,
 TEXTAREA) before attempting text-node localization
• Moved form-control aria-label population into dedicated else if branch to short-circuit
 text-node hunt
• Removed post-hunt form-control check that was executing after spurious warning
• Added detailed comment explaining why form controls only support aria-label localization

src/static/js/vendors/html10n.ts

---

2. src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts 🧪 Tests +35/-0

Add regression test for form control warning

• Added regression test for <select data-l10n-id> warning suppression
• Test creates select element with option children and verifies no "could not translate element
 content" warning is emitted
• Captures console warnings and filters for the specific error message
• Validates that aria-label is still populated correctly despite form-control short-circuit

src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Regression test misses bug ✓ Resolved 🐞 Bug ≡ Correctness

Description

The new Playwright test uses data-l10n-id = 'pad.toolbar.bold.title', which makes
translateNode() translate the title property (prop != 'textContent') and bypasses the
textContent/text-node hunt path where the "could not translate element content" warning occurs. This
test therefore won’t fail on the pre-fix behavior and won’t prevent regressions of the SELECT
short-circuit logic.

Code

src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts[R73-77]

+  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']) {

Evidence

translateNode() derives prop from the suffix after the last dot if it is in the allowed
attribute list (which includes title). The warning is only emitted in the prop === 'textContent'
+ node.children.length > 0 path after failing to find a writable text node, so using a .title
key bypasses the warning path entirely.

src/static/js/vendors/html10n.ts[643-659]
src/static/js/vendors/html10n.ts[678-710]
src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts[62-95]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new regression test intends to cover the warning emitted when translating a `<select data-l10n-id="…">` with only `<option>` children, but it uses a l10n key that ends with `.title`. In `Html10n.translateNode()`, keys ending in `.title` translate the element’s `title` property and *do not* enter the `prop === 'textContent'` branch where the warning happens.
### Issue Context
To reproduce the warning reliably, the test must use a translation key whose last segment is **not** one of the special attribute names (`title`, `innerHTML`, `alt`, `textContent`, `value`, `placeholder`), so `prop` defaults to `textContent`.
### Fix Focus Areas
- src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts[73-95]
- src/static/js/vendors/html10n.ts[643-710]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Flaky timeout-based waiting ✓ Resolved 🐞 Bug ☼ Reliability

Description

The test uses a fixed 50ms sleep to “give html10n a tick”, but html10n.localize() performs work in
an asynchronous callback after build(), so the warning could be emitted after the sleep on slow
CI. This can cause nondeterministic failures or false passes.

Code

src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts[R88-90]

+  // Give html10n a tick to run.
+  await page.waitForTimeout(50);
+

Evidence

localize() triggers translation inside a callback passed to build(), so completion is not
guaranteed within a fixed 50ms delay. The test currently waits an arbitrary amount of time before
asserting about warnings.

src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts[88-95]
src/static/js/vendors/html10n.ts[474-509]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test relies on `waitForTimeout(50)` instead of synchronizing on a deterministic signal that localization has finished.
### Issue Context
`html10n.localize()` calls `build(..., cb)` and performs translation inside the callback, so the timing depends on loader/cache and event loop scheduling.
### Fix Focus Areas
- src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts[88-95]
- src/static/js/vendors/html10n.ts[474-509]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---