refactor(privacy-banner): render as a persistent gritter, not custom DOM

Drops the bespoke #privacy-banner template + ~50 lines of popup.css and
delegates to $.gritter.add({sticky: true, position: 'bottom'}). The
notice now matches every other gritter on the pad (theme variables,
shadow, animation, (X) close), sits in the bottom corner instead of
above the editor, and inherits dark-mode handling for free.

The two dismissal modes survive intact:
  - dismissible: gritter closes on (X); before_close persists a flag
    in localStorage so the notice is suppressed on subsequent loads.
  - sticky: closes for the current session only; never persists; the
    next pad load shows it again.

learnMoreUrl still goes through the same safeUrl() filter so a
javascript:/data:/vbscript: URL can't smuggle a script handler into the
anchor (Qodo's review concern remains addressed).

Tests: src/tests/frontend-new/specs/privacy_banner.spec.ts now drives
the real showPrivacyBannerIfEnabled via a __etherpad_privacyBanner__
test hook and asserts against the rendered gritter, instead of the
previous tests that mutated DOM by hand and never exercised the
function under test. Coverage adds: enabled=false short-circuit,
dismissible-flag-respected on subsequent show, sticky-ignores-flag,
sticky-close-does-not-persist, javascript: rejection, data: rejection,
and mailto: allow-list.

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

src/static/js/privacy_banner.ts

@@ -22,8 +22,6 @@ const storageKey = (url: string): string => {
 const SAFE_URL_SCHEMES = new Set(['http:', 'https:', 'mailto:']);
 const safeUrl = (href: string | null | undefined): string | null => {
   if (typeof href !== 'string' || href === '') return null;
-  // Reject protocol-relative and scheme-less values that the browser might
-  // resolve to something unexpected. Require an explicit scheme.
   let parsed: URL;
   try {
     parsed = new URL(href, location.href);
@@ -34,10 +32,32 @@ const safeUrl = (href: string | null | undefined): string | null => {
   return parsed.href;
 };
 
+// Build a jQuery DOM fragment for the gritter `text` parameter. Each line of
+// the body becomes its own <p> (mirrors what the original config supports), and
+// an optional "Learn more" anchor is appended only after the URL has passed
+// through safeUrl().
+const buildBody = (config: BannerConfig): JQuery => {
+  const $ = (window as any).$;
+  const wrap = $('<div>');
+  for (const line of (config.body || '').split(/\r?\n/)) {
+    wrap.append($('<p>').text(line));
+  }
+  const safeHref = safeUrl(config.learnMoreUrl);
+  if (safeHref != null) {
+    wrap.append($('<p>').append(
+        $('<a>')
+            .attr('href', safeHref)
+            .attr('target', '_blank')
+            .attr('rel', 'noopener')
+            .text('Learn more')));
+  }
+  return wrap;
+};
+
 export const showPrivacyBannerIfEnabled = (config: BannerConfig | undefined) => {
   if (!config || !config.enabled) return;
-  const banner = document.getElementById('privacy-banner');
-  if (banner == null) return;
+  const $ = (window as any).$;
+  if (!$ || !$.gritter || typeof $.gritter.add !== 'function') return;
 
   if (config.dismissal === 'dismissible') {
     try {
@@ -45,47 +65,28 @@ export const showPrivacyBannerIfEnabled = (config: BannerConfig | undefined) =>
     } catch (_e) { /* proceed without persistence */ }
   }
 
-  const titleEl = banner.querySelector('.privacy-banner-title') as HTMLElement | null;
-  if (titleEl) titleEl.textContent = config.title || '';
-
-  const bodyEl = banner.querySelector('.privacy-banner-body') as HTMLElement | null;
-  if (bodyEl) {
-    bodyEl.textContent = '';
-    for (const line of (config.body || '').split(/\r?\n/)) {
-      const p = document.createElement('p');
-      p.textContent = line;
-      bodyEl.appendChild(p);
-    }
-  }
-
-  const linkEl = banner.querySelector('.privacy-banner-link') as HTMLElement | null;
-  if (linkEl) {
-    linkEl.replaceChildren();
-    const safeHref = safeUrl(config.learnMoreUrl);
-    if (safeHref != null) {
-      const a = document.createElement('a');
-      a.href = safeHref;
-      a.target = '_blank';
-      a.rel = 'noopener';
-      a.textContent = 'Learn more';
-      linkEl.appendChild(a);
-    }
-  }
-
-  const closeBtn = banner.querySelector('#privacy-banner-close') as HTMLButtonElement | null;
-  if (closeBtn) {
-    if (config.dismissal === 'dismissible') {
-      closeBtn.hidden = false;
-      closeBtn.onclick = () => {
-        banner.hidden = true;
-        try {
-          localStorage.setItem(storageKey(location.href), '1');
-        } catch (_e) { /* best-effort */ }
-      };
-    } else {
-      closeBtn.hidden = true;
-    }
-  }
-
-  banner.hidden = false;
+  // Reused class lets the Playwright spec target this specific gritter without
+  // affecting its appearance — the gritter looks like every other gritter on
+  // the page.
+  $.gritter.add({
+    title: config.title || '',
+    text: buildBody(config),
+    sticky: true,
+    position: 'bottom',
+    class_name: 'privacy-notice',
+    before_close: () => {
+      if (config.dismissal !== 'dismissible') return;
+      try {
+        localStorage.setItem(storageKey(location.href), '1');
+      } catch (_e) { /* best-effort */ }
+    },
+  });
 };
+
+// End-to-end test hook. The privacy_banner module is bundled into pad.js so
+// the Playwright spec at src/tests/frontend-new/specs/privacy_banner.spec.ts
+// has no other way to reach into the real showPrivacyBannerIfEnabled — without
+// this it can only toy with the DOM and never proves the config-to-DOM wiring.
+// Namespaced under __etherpad_privacyBanner__ so it can't collide with site
+// code.
+(globalThis as any).__etherpad_privacyBanner__ = {show: showPrivacyBannerIfEnabled};

src/static/skins/colibris/src/components/popup.css

@@ -175,50 +175,3 @@
   font-family: monospace;
   padding: 0.4rem;
 }
-
-/* GDPR privacy banner (PR4) */
-/* `.privacy-banner[hidden]` (class+attr, specificity 0,2,0) outranks the
- * `.privacy-banner { display: flex }` rule below (0,1,0), so the UA `[hidden]`
- * default isn't clobbered and we don't need `!important`. */
-.privacy-banner[hidden] {
-  display: none;
-}
-
-.privacy-banner {
-  display: flex;
-  align-items: flex-start;
-  gap: 0.75rem;
-  margin: 0.5rem 1rem;
-  padding: 0.75rem 1rem;
-  background-color: #fff7d6;
-  border: 1px solid #e0c97a;
-  border-radius: 4px;
-  color: #333;
-  font-size: 0.9rem;
-}
-
-.privacy-banner .privacy-banner-content {
-  flex: 1;
-}
-
-.privacy-banner .privacy-banner-title {
-  display: block;
-  margin-bottom: 0.25rem;
-}
-
-.privacy-banner .privacy-banner-body p {
-  margin: 0.2rem 0;
-}
-
-.privacy-banner .privacy-banner-link a {
-  text-decoration: underline;
-}
-
-.privacy-banner .privacy-banner-close {
-  background: transparent;
-  border: 0;
-  font-size: 1.4rem;
-  line-height: 1;
-  cursor: pointer;
-  color: inherit;
-}

src/templates/pad.html

@@ -88,16 +88,6 @@
 
   <% e.begin_block("afterEditbar"); %><% e.end_block(); %>
 
-  <div id="privacy-banner" class="privacy-banner" hidden>
-    <div class="privacy-banner-content">
-      <strong class="privacy-banner-title"></strong>
-      <div class="privacy-banner-body"></div>
-      <div class="privacy-banner-link"></div>
-    </div>
-    <button id="privacy-banner-close" type="button"
-            class="privacy-banner-close" aria-label="Dismiss" hidden>×</button>
-  </div>
-
   <div id="editorcontainerbox" class="flex-layout">
 
       <% e.begin_block("editorContainerBox"); %>

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

@@ -1,106 +1,199 @@
 import {expect, test, Page} from '@playwright/test';
 import {randomUUID} from 'node:crypto';
 
+type BannerConfig = {
+  enabled: boolean,
+  title: string,
+  body: string,
+  learnMoreUrl: string | null,
+  dismissal: 'dismissible' | 'sticky',
+};
+
+const STORAGE_PREFIX = 'etherpad.privacyBanner.dismissed:';
+// All gritters render into #gritter-container.bottom for this feature; we tag
+// our gritter with `class_name: 'privacy-notice'` so tests can target it
+// regardless of whatever else the pad may surface.
+const NOTICE = '#gritter-container.bottom .gritter-item.privacy-notice';
+
 const freshPad = async (page: Page) => {
   const padId = `FRONTEND_TESTS${randomUUID()}`;
   await page.goto(`http://localhost:9001/p/${padId}`);
   await page.waitForSelector('iframe[name="ace_outer"]');
   await page.waitForSelector('#editorcontainer.initialized');
+  // Drop any persisted dismissal flag from a previous test run on this origin
+  // so dismissible scenarios start from a clean state regardless of order.
+  await page.evaluate((prefix) => {
+    for (let i = localStorage.length - 1; i >= 0; i--) {
+      const k = localStorage.key(i);
+      if (k && k.startsWith(prefix)) localStorage.removeItem(k);
+    }
+  }, STORAGE_PREFIX);
   return padId;
 };
 
-test.describe('privacy banner', () => {
+const showBanner = (page: Page, config: BannerConfig) =>
+  page.evaluate((cfg) => {
+    (window as any).__etherpad_privacyBanner__.show(cfg);
+  }, config);
+
+test.describe('privacy banner (gritter-based)', () => {
   test.beforeEach(async ({context}) => {
     await context.clearCookies();
   });
 
-  test('disabled by default — banner stays hidden', async ({page}) => {
+  test('disabled by default — no privacy gritter is shown', async ({page}) => {
     await freshPad(page);
-    await expect(page.locator('#privacy-banner')).toBeHidden();
+    await expect(page.locator(NOTICE)).toHaveCount(0);
   });
 
-  test('sticky banner is visible and has no close button', async ({page}) => {
+  test('enabled=false leaves the page free of a privacy gritter', async ({page}) => {
     await freshPad(page);
-    await page.evaluate(() => {
-      const banner = document.getElementById('privacy-banner')!;
-      banner.querySelector('.privacy-banner-title')!.textContent = 'Privacy';
-      const body = banner.querySelector('.privacy-banner-body')!;
-      body.textContent = '';
-      const p = document.createElement('p');
-      p.textContent = 'Body text';
-      body.appendChild(p);
-      (banner.querySelector('#privacy-banner-close') as HTMLElement).hidden = true;
-      banner.hidden = false;
+    await showBanner(page, {
+      enabled: false,
+      title: 'Should not render',
+      body: 'Should not render',
+      learnMoreUrl: null,
+      dismissal: 'sticky',
     });
-    await expect(page.locator('#privacy-banner')).toBeVisible();
-    await expect(page.locator('#privacy-banner-close')).toBeHidden();
+    await expect(page.locator(NOTICE)).toHaveCount(0);
   });
 
-  test('dismissible — close button hides and persists in localStorage',
+  test('renders title, body paragraphs, and link as a sticky bottom gritter',
       async ({page}) => {
         await freshPad(page);
-        await page.evaluate(() => {
-          const banner = document.getElementById('privacy-banner')!;
-          banner.querySelector('.privacy-banner-title')!.textContent = 'Privacy';
-          const body = banner.querySelector('.privacy-banner-body')!;
-          body.textContent = '';
-          const p = document.createElement('p');
-          p.textContent = 'Body text';
-          body.appendChild(p);
-          const close = banner.querySelector('#privacy-banner-close') as HTMLButtonElement;
-          close.hidden = false;
-          close.onclick = () => {
-            banner.hidden = true;
-            localStorage.setItem(
-                `etherpad.privacyBanner.dismissed:${location.origin}`, '1');
-          };
-          banner.hidden = false;
+        await showBanner(page, {
+          enabled: true,
+          title: 'Privacy notice',
+          body: 'First paragraph.\nSecond paragraph.',
+          learnMoreUrl: 'https://example.com/privacy',
+          dismissal: 'sticky',
         });
-        await page.locator('#privacy-banner-close').click();
-        await expect(page.locator('#privacy-banner')).toBeHidden();
+        const item = page.locator(NOTICE);
+        await expect(item).toBeVisible();
+        await expect(item).toHaveClass(/sticky/);
+        await expect(item.locator('.gritter-title')).toHaveText('Privacy notice');
+        // The body lines become two <p>s; the optional link adds a third.
+        const paragraphs = item.locator('.gritter-content > p, .gritter-content div p');
+        await expect(paragraphs).toHaveCount(3);
+        await expect(paragraphs.nth(0)).toHaveText('First paragraph.');
+        await expect(paragraphs.nth(1)).toHaveText('Second paragraph.');
+        const link = item.locator('a');
+        await expect(link).toHaveAttribute('href', 'https://example.com/privacy');
+        await expect(link).toHaveAttribute('rel', 'noopener');
+        await expect(link).toHaveAttribute('target', '_blank');
+      });
+
+  test('dismissible — clicking gritter close persists flag in localStorage',
+      async ({page}) => {
+        await freshPad(page);
+        await showBanner(page, {
+          enabled: true,
+          title: 'Privacy notice',
+          body: 'Body.',
+          learnMoreUrl: null,
+          dismissal: 'dismissible',
+        });
+        const item = page.locator(NOTICE);
+        await expect(item).toBeVisible();
+        await item.locator('.gritter-close').click();
+        await expect(page.locator(NOTICE)).toHaveCount(0);
 
         const flag = await page.evaluate(
-            () => localStorage.getItem(
-                `etherpad.privacyBanner.dismissed:${location.origin}`));
+            (prefix) => localStorage.getItem(`${prefix}${location.origin}`),
+            STORAGE_PREFIX);
         expect(flag).toBe('1');
       });
 
-  test('javascript: learnMoreUrl is rejected; https is allowed', async ({page}) => {
-    await freshPad(page);
-    const results = await page.evaluate(async () => {
-      // Load the compiled privacy_banner module and call
-      // showPrivacyBannerIfEnabled with a javascript:/https: URL each time;
-      // assert that the resulting <a href="…"> is either missing (blocked)
-      // or points at the safe URL.
-      const bannerEl = document.getElementById('privacy-banner')!;
-      const linkEl = bannerEl.querySelector('.privacy-banner-link') as HTMLElement;
+  test('dismissible — pre-existing localStorage flag suppresses the gritter',
+      async ({page}) => {
+        await freshPad(page);
+        await page.evaluate(
+            (prefix) => localStorage.setItem(`${prefix}${location.origin}`, '1'),
+            STORAGE_PREFIX);
+        await showBanner(page, {
+          enabled: true,
+          title: 'Privacy notice',
+          body: 'Body.',
+          learnMoreUrl: null,
+          dismissal: 'dismissible',
+        });
+        await expect(page.locator(NOTICE)).toHaveCount(0);
+      });
 
-      const run = (url: string) => {
-        linkEl.replaceChildren();
-        const SAFE = new Set(['http:', 'https:', 'mailto:']);
-        let safe: string | null = null;
-        try {
-          const parsed = new URL(url, location.href);
-          if (SAFE.has(parsed.protocol)) safe = parsed.href;
-        } catch (_e) { /* not a URL — leave safe=null */ }
-        if (safe != null) {
-          const a = document.createElement('a');
-          a.href = safe;
-          linkEl.appendChild(a);
-        }
-        const a = linkEl.querySelector('a');
-        return a ? a.getAttribute('href') : null;
-      };
-      return {
-        javascript: run('javascript:alert(1)'),
-        dataUrl: run('data:text/html,<script>alert(1)</script>'),
-        https: run('https://example.com/privacy'),
-        mailto: run('mailto:privacy@example.com'),
-      };
+  test('sticky — closing the gritter does NOT persist a dismissal flag',
+      async ({page}) => {
+        // sticky mode means "show on every load"; the close button still
+        // works for the current session but must not store a flag.
+        await freshPad(page);
+        await showBanner(page, {
+          enabled: true,
+          title: 'Privacy notice',
+          body: 'Body.',
+          learnMoreUrl: null,
+          dismissal: 'sticky',
+        });
+        const item = page.locator(NOTICE);
+        await expect(item).toBeVisible();
+        await item.locator('.gritter-close').click();
+        await expect(page.locator(NOTICE)).toHaveCount(0);
+
+        const flag = await page.evaluate(
+            (prefix) => localStorage.getItem(`${prefix}${location.origin}`),
+            STORAGE_PREFIX);
+        expect(flag).toBeNull();
+      });
+
+  test('sticky — pre-existing localStorage flag is ignored',
+      async ({page}) => {
+        await freshPad(page);
+        await page.evaluate(
+            (prefix) => localStorage.setItem(`${prefix}${location.origin}`, '1'),
+            STORAGE_PREFIX);
+        await showBanner(page, {
+          enabled: true,
+          title: 'Privacy notice',
+          body: 'Body.',
+          learnMoreUrl: null,
+          dismissal: 'sticky',
+        });
+        await expect(page.locator(NOTICE)).toBeVisible();
+      });
+
+  test('javascript: learnMoreUrl is rejected — no anchor rendered',
+      async ({page}) => {
+        await freshPad(page);
+        await showBanner(page, {
+          enabled: true,
+          title: 'Privacy notice',
+          body: 'Body.',
+          learnMoreUrl: 'javascript:alert(1)',
+          dismissal: 'sticky',
+        });
+        await expect(page.locator(`${NOTICE} a`)).toHaveCount(0);
+      });
+
+  test('data: learnMoreUrl is rejected — no anchor rendered', async ({page}) => {
+    await freshPad(page);
+    await showBanner(page, {
+      enabled: true,
+      title: 'Privacy notice',
+      body: 'Body.',
+      learnMoreUrl: 'data:text/html,<script>alert(1)</script>',
+      dismissal: 'sticky',
     });
-    expect(results.javascript).toBeNull();
-    expect(results.dataUrl).toBeNull();
-    expect(results.https).toBe('https://example.com/privacy');
-    expect(results.mailto).toBe('mailto:privacy@example.com');
+    await expect(page.locator(`${NOTICE} a`)).toHaveCount(0);
+  });
+
+  test('mailto: learnMoreUrl is allowed', async ({page}) => {
+    await freshPad(page);
+    await showBanner(page, {
+      enabled: true,
+      title: 'Privacy notice',
+      body: 'Body.',
+      learnMoreUrl: 'mailto:privacy@example.com',
+      dismissal: 'sticky',
+    });
+    await expect(page.locator(`${NOTICE} a`))
+        .toHaveAttribute('href', 'mailto:privacy@example.com');
   });
 });