fix(padToggle): require defaultLabel for screen-reader fallback

The empty <label data-l10n-id="..."></label> meant screen readers had
nothing to announce while html10n was still loading or if a translation
failed to fetch. Native Etherpad's templates always render English text
inside the label tag for exactly this reason; the helper was missing
that safety net.

Make defaultLabel a required config field. Helper now renders
<label data-l10n-id="..." for="...">Default text</label> with both the
defaultLabel and l10nId HTML-escaped to prevent injection. html10n still
overwrites the text the moment it loads in the user's locale.

Validate on both server and client factories so a misconfigured plugin
fails loudly on import rather than silently shipping a nameless
checkbox.

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

Author: JohnMcLear

README.md

@@ -192,7 +192,9 @@ const {padToggle} = require('ep_plugin_helpers');
 const t = padToggle({
   pluginName: 'ep_myplugin',          // must match /^ep_[a-z0-9_]+$/
   settingId: 'my-feature',            // → ids: options-my-feature, padsettings-options-my-feature
-  l10nId: 'ep_myplugin.myFeature',    // i18n key (no hardcoded English label)
+  l10nId: 'ep_myplugin.myFeature',    // i18n key, html10n overwrites the fallback
+  defaultLabel: 'My feature',         // a11y fallback — rendered inside <label> so screen readers
+                                      // announce something before html10n loads
   defaultEnabled: false,              // overridable via settings.json[pluginName].defaultEnabled
 });
 

pad-toggle-server.js

@@ -26,11 +26,15 @@ try {
   padOptionsPluginPassthrough = caps && caps.padOptionsPluginPassthrough === true;
 } catch (_e) { /* older core — leave as false */ }
 
+const HTML_ESCAPE_RE = /[&<>"']/g;
+const HTML_ESCAPES = {'&': '&amp;', '<': '&lt;', '>': '&gt;', '"': '&quot;', "'": '&#39;'};
+const escapeHtml = (s) => String(s).replace(HTML_ESCAPE_RE, (c) => HTML_ESCAPES[c]);
+
 const validateConfig = (config) => {
   if (!config || typeof config !== 'object') {
     throw new Error('padToggle requires a config object');
   }
-  const {pluginName, settingId, l10nId, defaultEnabled = true} = config;
+  const {pluginName, settingId, l10nId, defaultLabel, defaultEnabled = true} = config;
   if (!PLUGIN_NAME_RE.test(pluginName || '')) {
     throw new Error(
         `padToggle pluginName must match /^ep_[a-z0-9_]+$/, got: ${pluginName}`);
@@ -39,19 +43,26 @@ const validateConfig = (config) => {
     throw new Error('padToggle requires settingId (string)');
   }
   if (!l10nId || typeof l10nId !== 'string') {
-    throw new Error('padToggle requires l10nId (string) — never hardcode a label');
+    throw new Error('padToggle requires l10nId (string) — i18n is mandatory');
+  }
+  if (!defaultLabel || typeof defaultLabel !== 'string') {
+    throw new Error(
+        'padToggle requires defaultLabel (string) — accessibility fallback ' +
+        'rendered inside <label> so screen readers announce something before ' +
+        'html10n loads. html10n overwrites it at runtime via data-l10n-id.');
   }
-  return {pluginName, settingId, l10nId, defaultEnabled: !!defaultEnabled};
+  return {pluginName, settingId, l10nId, defaultLabel, defaultEnabled: !!defaultEnabled};
 };
 
-const renderCheckbox = (settingId, l10nId, idPrefix) =>
+const renderCheckbox = (settingId, l10nId, defaultLabel, idPrefix) =>
   `<p>` +
     `<input type="checkbox" id="${idPrefix}options-${settingId}">` +
-    `<label for="${idPrefix}options-${settingId}" data-l10n-id="${l10nId}"></label>` +
+    `<label for="${idPrefix}options-${settingId}" ` +
+        `data-l10n-id="${escapeHtml(l10nId)}">${escapeHtml(defaultLabel)}</label>` +
   `</p>`;
 
 const padToggleServer = (rawConfig) => {
-  const {pluginName, settingId, l10nId, defaultEnabled} = validateConfig(rawConfig);
+  const {pluginName, settingId, l10nId, defaultLabel, defaultEnabled} = validateConfig(rawConfig);
   let cachedDefaultEnabled = defaultEnabled;
 
   const loadSettings = async (hookName, args) => {
@@ -84,13 +95,13 @@ const padToggleServer = (rawConfig) => {
   };
 
   const eejsBlock_mySettings = (hookName, args, cb) => {
-    args.content += renderCheckbox(settingId, l10nId, '');
+    args.content += renderCheckbox(settingId, l10nId, defaultLabel, '');
     return cb();
   };
 
   const eejsBlock_padSettings = (hookName, args, cb) => {
     if (!padOptionsPluginPassthrough) return cb();
-    args.content += renderCheckbox(settingId, l10nId, 'padsettings-');
+    args.content += renderCheckbox(settingId, l10nId, defaultLabel, 'padsettings-');
     return cb();
   };
 

pad-toggle.js

@@ -26,7 +26,13 @@ const validateConfig = (config) => {
     throw new Error('padToggle requires settingId (string)');
   }
   if (!l10nId || typeof l10nId !== 'string') {
-    throw new Error('padToggle requires l10nId (string) — never hardcode a label');
+    throw new Error('padToggle requires l10nId (string) — i18n is mandatory');
+  }
+  // Client side does no rendering, but the same validation runs here so an
+  // author who forgets defaultLabel on the client gets the same loud error
+  // as on the server.
+  if (!config.defaultLabel || typeof config.defaultLabel !== 'string') {
+    throw new Error('padToggle requires defaultLabel (string) — a11y fallback for screen readers');
   }
   return {pluginName, settingId, l10nId, defaultEnabled: !!defaultEnabled};
 };

test/pad-toggle.js

@@ -7,6 +7,7 @@ const baseConfig = () => ({
   pluginName: 'ep_test',
   settingId: 'thing',
   l10nId: 'ep_test.thing',
+  defaultLabel: 'Show Thing',
 });
 
 describe('padToggle', () => {
@@ -20,11 +21,15 @@ describe('padToggle', () => {
           /pluginName must match/);
     });
 
-    it('throws when settingId or l10nId is missing', () => {
+    it('throws when settingId, l10nId, or defaultLabel is missing', () => {
       assert.throws(() => padToggle({...baseConfig(), settingId: ''}),
           /settingId/);
       assert.throws(() => padToggle({...baseConfig(), l10nId: ''}),
           /l10nId/);
+      assert.throws(() => padToggle({...baseConfig(), defaultLabel: ''}),
+          /defaultLabel/);
+      assert.throws(() => padToggle({...baseConfig(), defaultLabel: undefined}),
+          /defaultLabel/);
     });
 
     it('returns the full server hook surface for valid config', () => {
@@ -46,14 +51,31 @@ describe('padToggle', () => {
   });
 
   describe('eejsBlock_mySettings', () => {
-    it('emits a checkbox with namespaced id and data-l10n-id label', (done) => {
+    it('emits a checkbox with namespaced id, data-l10n-id, and label fallback', (done) => {
       const t = padToggle(baseConfig());
       const args = {content: ''};
       t.eejsBlock_mySettings('hook', args, () => {
         assert.match(args.content, /id="options-thing"/);
         assert.match(args.content, /data-l10n-id="ep_test\.thing"/);
-        // No hardcoded English fallback inside the label tag.
-        assert.match(args.content, /data-l10n-id="ep_test\.thing"><\/label>/);
+        // a11y fallback: <label> must contain the default text so screen
+        // readers announce something even if html10n hasn't loaded yet.
+        assert.match(args.content, /data-l10n-id="ep_test\.thing">Show Thing<\/label>/);
+        done();
+      });
+    });
+
+    it('HTML-escapes the defaultLabel and l10nId to prevent injection', (done) => {
+      const t = padToggle({
+        ...baseConfig(),
+        defaultLabel: 'A & B <script>',
+        l10nId: 'ep_test.thing"onerror',
+      });
+      const args = {content: ''};
+      t.eejsBlock_mySettings('hook', args, () => {
+        assert.ok(!args.content.includes('<script>'),
+            'raw <script> must not survive into rendered HTML');
+        assert.match(args.content, /A &amp; B &lt;script&gt;/);
+        assert.match(args.content, /data-l10n-id="ep_test\.thing&quot;onerror"/);
         done();
       });
     });