Skip to content

Commit c206e2a

Browse files
JohnMcLearclaude
andcommitted
fix(privacy-banner): gate test hook on webdriver, align doc with sticky behavior
Two follow-ups from Qodo's second review on #7549. Rule violation: __etherpad_privacyBanner__ was published on every pad load even when privacyBanner.enabled was false, so the disabled-by- default feature still added an observable global. Gate the assignment on `navigator.webdriver` — Playwright/ChromeDriver/Selenium set this to true; production browsers do not — so the hook is only present for tests and the disabled path is genuinely zero-side-effect. Bug 3 (sticky still closable): doc/privacy.md previously claimed `dismissal: "sticky"` removes the close button, but the gritter implementation always renders (X). Aligning the doc with reality — sticky now means "shows on every load, but closable for the session" — rather than adding bespoke CSS to a vanilla gritter (matches the "don't style it differently than other gritter messages" preference that drove the gritter migration in 906e145). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8abb7e6 commit c206e2a

2 files changed

Lines changed: 24 additions & 8 deletions

File tree

‎doc/privacy.md‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,21 @@ policy, contact for erasure requests, etc.
7676
}
7777
```
7878

79-
The banner is rendered from plain text (HTML is escaped) with one
80-
paragraph per line. With `dismissal: "dismissible"` the user can close
81-
the banner and the choice is remembered in `localStorage` per origin.
82-
`dismissal: "sticky"` removes the close button so the notice is shown
83-
on every pad load.
79+
The banner is rendered as a persistent gritter notification at the
80+
bottom of the page (it inherits the same look as every other gritter
81+
on the pad — no custom skin needed). The body is plain text (HTML is
82+
escaped); each line becomes its own paragraph.
83+
84+
`dismissal` controls how the close (×) is handled:
85+
86+
- `"dismissible"` (default) — when the user closes the gritter, the
87+
choice is persisted in `localStorage` per origin and the banner is
88+
not shown again on subsequent pad loads.
89+
- `"sticky"` — closing the gritter only hides it for the current
90+
session; the next pad load shows it again. (The close control is
91+
not removed; for an operator-enforced non-closable notice, render
92+
the policy out-of-band — e.g., a skin override or a reverse-proxy
93+
ribbon.)
94+
95+
Unknown `dismissal` values are coerced to `"dismissible"` with a
96+
`logger.warn` at settings load.

‎src/static/js/privacy_banner.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ export const showPrivacyBannerIfEnabled = (config: BannerConfig | undefined) =>
9898
// the Playwright spec at src/tests/frontend-new/specs/privacy_banner.spec.ts
9999
// has no other way to reach into the real showPrivacyBannerIfEnabled — without
100100
// this it can only toy with the DOM and never proves the config-to-DOM wiring.
101-
// Namespaced under __etherpad_privacyBanner__ so it can't collide with site
102-
// code.
103-
(globalThis as any).__etherpad_privacyBanner__ = {show: showPrivacyBannerIfEnabled};
101+
// Gated on navigator.webdriver so the global is invisible in real browsers
102+
// (Playwright/ChromeDriver/Selenium set webdriver=true; humans don't), keeping
103+
// the disabled-by-default feature genuinely zero-side-effect in production.
104+
if (typeof navigator !== 'undefined' && (navigator as any).webdriver) {
105+
(globalThis as any).__etherpad_privacyBanner__ = {show: showPrivacyBannerIfEnabled};
106+
}

0 commit comments

Comments
 (0)