mirrors/etherpad-lite
1
0
Fork 0
mirror of https://github.com/ether/etherpad-lite.git synced 2026-05-05 20:26:49 +02:00
Code Issues Projects Releases Wiki Activity
9,667 Commits 238 Branches 135 Tags
Commit Graph

6 Commits

Author SHA1 Message Date
John McLear
5e8704f8d8
feat(gdpr): pad deletion controls (PR1 of #6701) (#7546) 
* docs: PR1 GDPR deletion-controls design spec

First of five GDPR PRs tracked in #6701. PR1 covers deletion controls:
one-time deletion token, allowPadDeletionByAllUsers flag, authorisation
matrix for handlePadDelete and the REST deletePad endpoint, a single
token-display modal for browser pad creators, and test coverage.

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

* docs: PR1 GDPR deletion-controls implementation plan

13 TDD-structured tasks covering PadDeletionManager unit tests, socket
+ REST three-way auth, clientVars wiring, one-time token modal,
delete-with-token UI, Playwright coverage, and PR handoff.

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

* feat(gdpr): scaffolding for pad deletion tokens

PadDeletionManager stores a sha256-hashed per-pad deletion token and
verifies it with timing-safe comparison. createPad / createGroupPad
return the plaintext token once on first creation, and Pad.remove()
cleans it up. Gated behind the new allowPadDeletionByAllUsers flag
which defaults to false to preserve existing behaviour.

Part of #6701 (GDPR PR1).

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

* fix+test(gdpr): lazy DB access in PadDeletionManager + unit tests

Capturing DB.db at module-load time was null until DB.init() ran, which
broke importing the module outside a live server (including from the
test runner). Switch to DB.db.* at call time and add unit tests
exercising create/verify/remove plus timing-safe comparison.

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

* feat(gdpr): three-way auth for socket PAD_DELETE

Creator cookie → valid deletion token → allowPadDeletionByAllUsers flag.
Anyone else still gets the existing refusal shout.

* feat(gdpr): optional deletionToken on programmatic deletePad

* feat(gdpr): advertise optional deletionToken on REST deletePad

* test(gdpr): cover deletePad authorisation matrix via REST

* feat(gdpr): surface padDeletionToken in clientVars for creators only

Revision-0 author on their first CLIENT_READY visit receives the
plaintext token; all subsequent CLIENT_READYs receive null because
createDeletionTokenIfAbsent is idempotent. Readonly sessions and any
other user never see the token.

* i18n(gdpr): strings for deletion-token modal and delete-with-token flow

* feat(gdpr): token modal + delete-with-token disclosure markup

* feat(gdpr): show deletion token once, allow delete via recovery token

* style(gdpr): modal + delete-with-token layout

* test(gdpr): Playwright coverage for deletion-token modal + delete-with-token

* fix(test): auto-dismiss deletion-token modal in goToNewPad helper

The token modal introduced in PR1 blocks clicks for every Playwright
test that creates a new pad via the shared helper. Add a one-line
dismissal so unrelated tests keep passing, and have the deletion-token
spec navigate inline via newPadKeepingModal() when it needs the modal
open to capture the token.

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

* fix(test): dismiss deletion-token modal without focus transfer

Clicking the ack button transferred focus out of the pad iframe, which
made subsequent keyboard-driven tests (Tab / Enter) silently miss the
editor. Swap the click for a page.evaluate() that hides the modal and
nulls clientVars.padDeletionToken directly, leaving focus where it was.

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

* fix(gdpr): PadDeletionManager race + document createPad/deletePad

Qodo review:
- createDeletionTokenIfAbsent() was a non-atomic read-then-write. Two
  concurrent callers for the same pad could both return different
  plaintext tokens while only the later hash was stored, leaving the
  first caller with an unusable recovery token. Serialise per-pad via a
  Promise chain and add a regression test that fires 8 concurrent
  calls and asserts exactly one plaintext is emitted and validates.
- doc/api/http_api.md now documents createPad returning deletionToken
  and deletePad accepting the optional deletionToken parameter.

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

* fix(gdpr): always render delete-with-token in settings popup

The rebase onto develop placed the delete-pad-with-token details inside
the pad-settings-section conditional, which is only rendered when
enablePadWideSettings is true AND the section is toggled visible.
Second-device recovery (typing the captured token on a fresh browser)
must work without pad-wide settings enabled, so move the details out
to sit alongside the existing pad_deletion_token.spec.ts expectations.

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

* fix(gdpr): require valid token when supplied, gate on auth, harden a11y/i18n

- PadMessageHandler: a supplied deletion token must validate; do not fall
  back to the creator-cookie path when the token is wrong (was deleting
  the pad anyway when the creator pasted a wrong token into the field).
- Skip token issuance + UI when requireAuthentication is on (creator
  identity is stable, recovery token is redundant noise).
- Server emits messageKey instead of hardcoded English; both shout
  handlers (inline alert and global gritter) localize via html10n.
- Suppress the global "Admin message" gritter for pad.deletionToken.*
  shouts to avoid the "Admin message: undefined" duplicate.
- Token-modal a11y: role=dialog, aria-modal, aria-labelledby/describedby,
  visually-hidden label on the token input, aria-live on Copy, focus to
  the token input on open and restore on dismiss.
- Style the "Delete Pad with Token" disclosure to match the Delete pad
  button; align the Copy/value row; pad the disclosure label.

Tests: Playwright now covers the creator-with-wrong-token path, asserts
no "Admin message" / "undefined" gritter on denial; backend API test
covers requireAuthentication suppressing the token.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-05-01 13:50:04 +01:00
John McLear
6195289198
feat(gdpr): IP/privacy audit (PR2 of #6701) (#7547) 
* docs: PR2 GDPR IP/privacy audit design spec

Second of five GDPR PRs (#6701). Audit identifies four log-sites that
leak IPs despite disableIPlogging=true, proposes a tri-state ipLogging
setting with a back-compat shim, and specifies a doc/privacy.md that
documents Etherpad's actual IP handling.

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

* docs: PR2 GDPR IP/privacy audit implementation plan

7 TDD-structured tasks: anonymizeIp helper + unit tests, tri-state
ipLogging setting with disableIPlogging deprecation shim, wiring
through 5 leaking log sites, clientVars.clientIp removal, access-log
integration test, doc/privacy.md, and PR handoff.

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

* feat(gdpr): anonymizeIp helper with v4/v6/v4-mapped truncation

* feat(gdpr): tri-state ipLogging setting + disableIPlogging shim

* fix(gdpr): route every IP log site through anonymizeIp

Closes four leaks where disableIPlogging was silently ignored
(rate-limit warn, both auth-log calls in webaccess, import/export
rate-limit warn) and normalises the four that did honour the flag
onto the new ipLogging tri-state via the shared helper.

* chore(gdpr): drop dead clientVars.clientIp placeholder

Server side: remove the literal '127.0.0.1' assignments from both
clientVars and collab_client_vars. Type side: drop clientIp from
ClientVarPayload and ServerVar. pad.getClientIp now returns the same
'127.0.0.1' literal as a plugin-compat shim (pad_utils.uniqueId still
uses it as a prefix).

* test(gdpr): ipLogging modes + disableIPlogging shim

* docs(gdpr): operator-facing privacy and IP handling statement

* fix(gdpr): validate ipLogging at load + regression test for log sites

Qodo review:
- settings.ipLogging is loaded as a trusted union but nothing enforced
  the shape. An unknown value (e.g. a typo or null) silently fell
  through to anonymizeIp's "truncated" branch and emitted partially
  redacted IPs. Fall back to "anonymous" with a WARN at load time.
- New regression test scans the four known log-sites for raw
  req.ip / socket.request.ip / request.ip inside logger calls that
  don't wrap through anonymizeIp / logIp, so a future edit that
  re-introduces a raw IP fails CI.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-05-01 13:47:40 +01:00
John McLear
e39dbde887
feat(updater): tier 1 — notify admin and pad users of available updates (#7601) 
* docs(updater): add four-tier auto-update design spec

Four-tier opt-in self-update subsystem (off / notify / manual / auto / autonomous).
GitHub Releases as source of truth; install-method auto-detection with admin
override; in-process execution with supervisor restart; 60s drain + announce;
auto-rollback on health-check failure with crash-loop guard. Pad-side severe/
vulnerable badge that does not leak the running version. Top-level adminEmail
with escalating cadence (weekly while vulnerable, monthly while severe).

Refs: docs/superpowers/specs/2026-04-25-auto-update-design.md

* docs(updater): add PR 1 (Tier 1 notify) implementation plan

Bite-sized TDD task breakdown for shipping Tier 1 notify only:
- VersionChecker, InstallMethodDetector, UpdatePolicy, Notifier, state modules
- /admin/update/status (admin-auth) and /api/version-status (public, no version leak)
- Admin UI banner + read-only update page + nav link
- Pad-side severe/vulnerable footer badge
- Settings: updates.* block + top-level adminEmail
- Tests: vitest unit + mocha integration + Playwright admin/pad
- CHANGELOG + doc/admin/updates.md

PRs 2-4 (manual/auto/autonomous) get their own plans after PR 1 lands.

* feat(updater): add shared types for auto-update subsystem

* feat(updater): clarify OutdatedLevel and EMPTY_STATE doc, drop path header

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(updater): add semver helpers and vulnerable-below parser

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(updater): tighten semver regex to reject four-part versions

* feat(updater): add state persistence with schema validation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(updater): reject null email and array latest in state validation

typeof null === 'object' meant {email:null} passed the old isValid check,
which would crash downstream Notifier code reading email.severeAt. Likewise,
an array would pass the typeof latest === 'object' branch. Introduce
isPlainObject helper (null-safe, Array.isArray guard) and use it for both
fields. Adds two regression tests covering the exact broken inputs.

* feat(updater): add install-method detector with override

* feat(updater): add policy evaluator

* feat(updater): add GitHub Releases checker with ETag support

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(updater): validate release fields and preserve ETag on prerelease

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(updater): add email cadence decider

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(updater): tagChanged email fires regardless of cadence; drop unused field

* feat(settings): add updates.* and adminEmail settings

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(updater): wire boot hook and periodic checker

Register expressCreateServer/shutdown hooks in ep.json and implement
the boot-wiring module that detects install method, starts the polling
interval and runs the notifier dedupe pass each tick.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(updater): add /admin/update/status and /api/version-status endpoints

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* i18n(updater): add english strings for update banner, page, and pad badge

* feat(updater): add pad footer badge for severe/vulnerable status

* feat(admin-ui): add update banner, page, and nav link

Add UpdateStatusPayload to the zustand store, a persistent UpdateBanner
rendered in the App layout, a /update page showing version details and
changelog, and a Bell nav link — all wired to the /admin/update/status
endpoint added in Task 10.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(updater): add Playwright specs for admin banner/page and pad badge

* docs(updater): document tier 1 settings, badge, email cadence

* refactor(updater): dedupe helpers, fix misleading log, add banner styling

- Export stateFilePath from index.ts and import it in updateStatus.ts (removes local duplicate)
- Import getEpVersion from Settings.ts in both index.ts and updateStatus.ts (removes two local definitions)
- Fix misleading 'backing off' log message — no backoff is implemented, just retries at next interval
- Remove EMPTY_STATE_FOR_TESTS re-export from state.ts; state.test.ts now imports EMPTY_STATE directly from types.ts
- Add .update-banner and .update-page CSS rules to admin/src/index.css

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(updater): address review feedback — async wrap, tier=off skip, poll race, opt-in admin gate

- Wrap /api/version-status and /admin/update/status with a small async helper
  so a rejected promise becomes next(err) instead of an unhandled rejection.
- Short-circuit route registration when updates.tier === 'off' so the heavier
  opt-out also removes the HTTP surface (matches pre-PR behavior for that case).
- Add an in-flight guard around performCheck() so overlapping interval ticks
  can't race on update-state.json writes or duplicate email decisions; track
  the initial setTimeout handle and clear it in shutdown().
- Add updates.requireAdminForStatus (default false) so admins can lock
  /admin/update/status to authenticated admin sessions without disabling the
  updater. Default false preserves current behavior (the running version is
  already exposed publicly via /health). Backend specs cover unauth → 401,
  non-admin → 403, admin → 200.
- Bump admin troubleshooting menu count test 5 → 6 to account for the new
  Update nav link.

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

* fix(updater): address Qodo round-2 review feedback

Round 2 of Qodo review on #7601. Addressing the action-required items:

#1 Badge bypassed pad baseURL — derive basePath the same way
   padBootstrap.js does (`new URL('..', window.location.href).pathname`)
   and prefix the fetch with it. Subpath deployments now reach
   /<prefix>/api/version-status instead of 404ing.

#2 Updater poller could get stuck — `getCurrentState()` is now inside
   the try/finally so a one-time loadState() rejection can't leave
   `checkInFlight=true` and permanently silence polling.

#3 Updates off hung admin page — UpdatePage now self-fetches and
   renders explicit `disabled` (404), `unauthorized` (401/403), and
   `error` states instead of staying on "Loading...". Banner-driven
   prefetch is still honoured if it landed first.

#11 NaN polling interval — coerce `checkIntervalHours` to a number,
   clamp to [1h, 168h], log a warning and fall back to 6h on
   non-finite input. Math.max(1, NaN) === NaN previously meant a
   malformed settings.json could turn the poller into a tight loop.

#13 State validation accepted broken subfields — `isValid()` now
   inspects `latest.{version,tag,body,publishedAt,htmlUrl,prerelease}`,
   `vulnerableBelow[].{announcedBy,threshold}`, and
   `email.{severeAt,vulnerableAt,vulnerableNewReleaseTag}`. A
   hand-edited file with a number where a string is expected is now
   treated as corrupt and reset to EMPTY_STATE rather than crashing
   later in semver parsing or email rendering.

#14 Badge cache stampede — wrap `computeOutdated()` in a single-flight
   promise so concurrent requests at cache expiry await one shared
   computation instead of fanning out into N redundant disk reads.

Plus six new state.test.ts cases covering each new validation guard.

Pushing back on the remaining items:

#4 `updates.tier` defaults to `notify` — intentional. The whole point
   of tier 1 is to surface the "you are behind" signal to admins by
   default. Opt-in defeats the purpose; the existing failure mode
   (admin never hears about a security-relevant release) is exactly
   what this PR is fixing.

#5/#8 Admin status endpoint admin-auth — `currentVersion` is already
   public via `/health`, so wrapping the route in admin-auth doesn't
   reduce the disclosure surface meaningfully. Operators who want it
   gated set `updates.requireAdminForStatus=true` (already wired and
   covered by the comment on the route handler).

#10 Plain `https://` URLs in planning doc — planning markdown is
   viewed in editors and on GitHub where protocol-relative URLs would
   either render literally or break entirely. Keeping `https://`.

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

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
 2026-05-01 20:02:12 +08:00
John McLear
85f9a5f2f5
feat: Open Graph & Twitter Card metadata for pad/timeslider/home (closes #7599) (#7635) 
* docs(spec): Open Graph metadata for pad pages (issue #7599)

Spec for adding og:* and twitter:card meta tags to /p/:pad,
the timeslider, and the homepage so shared links unfurl with
a useful preview in chat apps.

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

* docs(spec): expand OG spec — i18n (locale map + og:locale) and a11y (image:alt)

Address review feedback: socialDescription accepts a per-language map,
og:locale is emitted from the negotiated render language, and image:alt
attributes are emitted for screen readers in chat clients.

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

* feat: emit Open Graph & Twitter Card metadata for pad/timeslider/home

Closes #7599.

Pad URLs shared in chat apps (WhatsApp, Signal, Slack, etc.) previously
unfurled with no preview because the rendered HTML carried no OG or
Twitter Card metadata. This change emits og:title, og:description,
og:image, og:url, og:site_name, og:type, og:locale, og:image:alt and
the equivalent twitter:* tags on the pad page, the timeslider, and the
homepage.

A new settings.json key `socialDescription` controls the description.
It accepts either a plain string applied to every locale or a per-language
map keyed by BCP-47 tag with an optional `default` fallback. og:locale
is emitted from the language already negotiated via req.acceptsLanguages
and og:image:alt provides screen-reader text for chat-client previews.

Pad names from the URL are HTML-escaped before being interpolated into
og:title to prevent reflected XSS via crafted pad IDs.

Tests: src/tests/backend/specs/socialMeta.ts covers the default,
per-locale override, locale fallback, URL decoding, XSS escape, and
the timeslider/homepage variants.

Semver: minor (new setting; templates emit additional tags but no
existing behavior changes).

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

* fix(test): use valid pad-name char in URL-decode test

Spaces aren't allowed in pad names — Etherpad redirected /p/Has%20Space*
to a sanitized name (302), so the og:title assertion failed. Use %2D
("-") instead, which is a valid pad-name character and still exercises
the URL-decode path.

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

* fix(socialMeta): don't double-decode pad name from req.params.pad

Express has already URL-decoded :pad route params before they reach the
handler. Calling decodeURIComponent on the result throws URIError for
pad names containing a literal "%" — e.g. the URL /p/100%25 yields
req.params.pad === "100%", and decodeURIComponent("100%") throws.

This would have prevented the page from rendering for some valid pad
IDs. Drop the redundant decode and add a regression test for the "%"
case.

Reported by Qodo on PR #7635.

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

* refactor(socialMeta): source description from i18n catalog, drop settings key

Per review: the OG description is a translatable string and belongs in
Etherpad's locale files alongside the rest of the UI strings, not in
settings.json. Operators who want to override it per-language continue to
use the standard customLocaleStrings mechanism — no new config surface.

Changes:
- Add "pad.social.description" to src/locales/en.json (default English).
- Export i18n.locales so server-side renderers can look up translations.
- socialMeta.renderSocialMeta now takes a `locales` map and resolves
  renderLang → primary subtag → en, instead of taking a per-locale map
  from settings.
- Remove `socialDescription` from Settings.ts, settings.json.template,
  settings.json.docker (the key never shipped).
- Update tests and spec doc to reflect i18n-sourced description.

Reported by Qodo on PR #7635 (also confirmed feature is fine to land
default-on; no flag needed).

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

* test(socialMeta): add unit tests for pure helpers

21 cases exercising buildSocialMetaHtml and renderSocialMeta directly,
without HTTP/DB. Covers tag enumeration, HTML escaping, og:locale
region formatting, title composition (pad/timeslider/home), description
i18n resolution (exact/primary/en fallback, missing catalog), image URL
(default favicon vs absolute settings.favicon vs alt text), canonical
URL building with query-string stripping, the literal "%" no-throw
regression, and attribute-breakout escape.

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

* fix(socialMeta): defend og:url/og:image against host-header poisoning

Previously og:url and og:image were built from req.protocol +
req.get('host'), both of which can be client-controlled (Host header
directly, or X-Forwarded-* under trust proxy). A crafted Host could
make the server emit OG tags pointing at an attacker's origin —
harmful if any cache fronts the response or if a vulnerable proxy
forwards the headers unsanitized.

Two-layer defense:

1. New optional setting `publicURL` lets operators pin the canonical
   origin used for shared link previews ("https://pad.example"). When
   set, og:url and og:image use it unconditionally. Sanitized at use
   time: must be http(s)://host[:port] with no path, no userinfo, no
   trailing slash; malformed values fall back to the request.

2. When `publicURL` is unset, the request-derived fallback now strictly
   validates the Host header against /^[a-z0-9]([a-z0-9.-]{0,253}[a-z0-9])?(:\d{1,5})?$/i
   and caps the scheme to "http"/"https". A crafted Host (CRLF
   injection, userinfo, "<script>") is replaced with "localhost"
   instead of being echoed into og:url.

Reported by Qodo on PR #7635.

Tests: 5 new unit cases covering publicURL preference, trailing-slash
strip, malformed-publicURL fallback, Host validation, scheme cap.

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

* refactor(socialMeta): tighten types, drop `any`

- `req: any` -> express `Request` (covers acceptsLanguages/protocol/get/originalUrl).
- `settings: any` -> local `SocialMetaSettings` interface narrowed to the three
  fields we actually read (title/favicon/publicURL); avoids coupling to the
  full Settings module surface.
- `availableLangs: {[k: string]: any}` -> `{[lang: string]: unknown}`; only
  keys are read, so values stay deliberately unconstrained.

No runtime change. All 26 socialMeta unit tests still pass.

Per Sam's review on #7635.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-05-01 10:43:29 +01:00
John McLear
7b9a5eb01a
fix(a11y): dialog semantics, focus management, icon labels, html lang (#7584) 
* fix(a11y): negotiate lang/dir per request and set on <html>

Server-renders the html element with `lang` and `dir` matching the
client's Accept-Language header (negotiated against availableLangs from
i18n hooks). Falls back to `en`/`ltr` if no match.

This gives screen readers a correct document language during the brief
window before client-side html10n refines it (l10n.ts already sets both
attributes after locale data loads).

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

* fix(a11y): dialog semantics on popups; fix aria-role typo on userlist

Adds role=dialog, aria-modal=true, and either aria-labelledby (when an
h1 is present) or aria-label (for popups without an h1) to:

  - #settings, #import_export, #embed, #skin-variants (labelledby)
  - #connectivity, #users, #mycolorpicker (aria-label)

Fixes the invalid aria-role="document" attribute on #otherusers; it's
now role=region with aria-live=polite so screen readers announce
collaborator joins/leaves.

Container aria-label values are English-only for now — Etherpad's
html10n implementation only supports localizing specific attributes
(title, alt, placeholder, etc), not aria-label on container nodes.
Localization can follow once html10n grows that affordance.

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

* fix(a11y): focus management and Escape-to-close for popups

Three additions to toggleDropDown / _bodyKeyEvent:

  - Remember the trigger element (document.activeElement) when opening
    a popup, so we can restore focus when it closes.
  - On open, focus the first focusable element inside the popup so
    keyboard users land inside the dialog instead of staying on the
    trigger button.
  - Escape pressed while focus is inside a popup closes it, then the
    restore-focus path runs and the trigger button is refocused.

Replaces the previous behavior where Escape from inside a popup did
nothing; users had to click outside to dismiss.

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

* fix(a11y): make chaticon and chat header controls real buttons

- #chaticon: <div onclick> → <button type=button> with aria-label
- #titlecross / #titlesticky: <a onClick> → <button type=button>
  with aria-label (Close chat / Pin chat to screen)
- Decorative chat-bubble glyph gets aria-hidden=true so it isn't
  read alongside the button label
- #chatcounter labelled "Unread messages"
- Inline onclick attributes moved to chat.init() handlers
- CSS reset on the new buttons (transparent bg, no border, inherit
  font/color) so they match the prior visual design
- :focus-visible outlines for keyboard users

Existing test selectors (#chaticon, #titlecross, #titlesticky) are
unchanged and continue to work — they never relied on element type.

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

* fix(a11y): accessible names for icon-only toolbar/export controls

- Export links (#exportetherpada, #exporthtmla, #exportplaina,
  #exportworda, #exportpdfa, #exportopena): added aria-label so the
  link is announced as e.g. "Export as PDF". The inner icon span
  gets aria-hidden=true so screen readers don't read both the icon
  text and the link label.

- Show-more toolbar toggle (.show-more-icon-btn): converted from
  <span> to <button type=button> with aria-label and aria-expanded.
  The click handler now toggles aria-expanded alongside the
  full-icons class so assistive tech reflects the open/closed state.

- Theme switcher knob: aria-label changed from "theme-switcher-knob"
  (a class-style identifier, not human text) to "Toggle theme".

Aria-label values are English-only for now. Etherpad's html10n
implementation only localizes a fixed attribute list (title, alt,
placeholder, value, innerHTML, textContent); aria-label is not
included, so a clean l10n path requires a follow-up to either
extend html10n or set aria-label client-side after locale loads.

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

* test(a11y): cover dialog semantics, html lang, icon button labels

New Playwright spec verifies the a11y guarantees added by this branch:

  - <html> has a non-empty lang attribute
  - settings/import_export/embed/users popups expose role=dialog,
    aria-modal=true, and either aria-labelledby (when an h1 exists)
    or aria-label (when none does)
  - Escape from inside the settings popup closes it AND restores
    focus to the trigger button
  - Export links each carry a descriptive aria-label
  - #chaticon is a real <button> with aria-label
  - #titlecross / #titlesticky are real <button>s with aria-label
  - #otherusers uses role=region + aria-live=polite + aria-label
    (and the previous aria-role typo is gone)
  - .show-more-icon-btn is a <button> with aria-label and
    aria-expanded

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

* fix(a11y): address Qodo review feedback from PR #7584

1. Users Escape close broken - toggleDropDown('none') intentionally
   skips the users module so switching between other popups doesn't
   hide the user list. That meant Escape couldn't dismiss the Users
   popup either. The Escape branch now checks for #users as the
   focused popup and closes it explicitly (respecting stickyUsers)
   before falling through to the normal close-all path.

2. Embed focus overridden - the rAF auto-focus in toggleDropDown
   grabbed the first focusable descendant, which stole focus from
   command handlers that target a specific control (notably the Embed
   command's #linkinput). rAF now bails out if focus is already
   inside the newly-opened popup.

3. Button click blurs :focus before toggleDropDown captures trigger -
   discovered while investigating the Firefox Playwright failure for
   "settings popup Escape restores focus". Button.bind() calls
   $(':focus').trigger('blur') before invoking the callback, so by
   the time toggleDropDown() captured document.activeElement as the
   restore target it was already <body>. The click handler now
   stashes padeditbar._lastTrigger to the clicked <button> before
   blur runs; toggleDropDown only falls back to activeElement when
   the pre-stash didn't happen (keyboard shortcut path).

4. html10n overwrites aria-label - html10n unconditionally set
   aria-label to the translated string, clobbering explicit aria-label
   on elements that also carry data-l10n-id. setAttribute now only
   fires when the element has no aria-label; explicit author labels
   win, unlabelled translated elements still get a name.

5. Button visual reset - the show-more-icon-btn and #chaticon
   conversions inherited UA default button border/background/padding,
   shifting icon glyphs visibly off-centre. Added appearance /
   background / border / padding resets.

6. Export links test assumes soffice is installed - #exportworda,
   #exportpdfa, #exportopena are removed client-side by pad_impexp.ts
   when clientVars.exportAvailable === 'no'. The test now skips links
   absent at runtime.

Verified locally: all 10 a11y_dialogs specs pass on both Chromium and
Firefox; backend suite remains 799/799 passing; ts-check clean.

* fix(a11y): close popups with no focusable content; unbreak chat-icon layout

Round 2 of #7584 review follow-ups.

1. Users popup Escape still didn't close the dialog (user-confirmed).
   Root cause: _bodyKeyEvent is bound to the OUTER document's body.
   When #users opens, the command handler tries to focus
   #myusernameedit but that input is `disabled`, so focus stays in the
   ace editor iframe. Keydown from inside the iframe does not bubble
   to the outer document, so Esc never reaches _bodyKeyEvent.
   Fix: in the open-popup rAF, if no command handler placed focus
   inside the dialog, focus the popup div itself (with tabindex=-1).
   That keeps subsequent keydown events on the outer document so
   Esc can dismiss the popup. Also broadened the Esc branch to fire
   whenever any popup is `.popup-show`, regardless of where :focus
   lives — some popups legitimately have no focusable content at
   open.
   Added a regression test that opens #users and asserts Esc closes
   it. Passes on both Chromium and Firefox.

2. Chat icon (#chaticon) visual still wrong after the first CSS fix.
   - My previous `border: 0` reset was overriding the intended
     `border: 1px solid #ccc; border-bottom: none` from the earlier
     rule. Removed `border: 0`; the earlier explicit border suffices
     to suppress UA defaults.
   - The `<span class="buttonicon">` inside `#chaticon` was picking
     up the global `.buttonicon { display: flex; }` rule meant for
     toolbar button instances, which broke the inline layout of the
     label + glyph + counter row. Added a scoped
     `#chaticon .buttonicon { display: inline; }` override.

All 11 a11y_dialogs specs pass on Chromium and Firefox. Backend
suite and ts-check remain clean.

* fix(a11y): only stash _lastTrigger for dropdown-opening buttons

Round 3 follow-up. The previous Button.bind() change stashed every
clicked toolbar button as padeditbar._lastTrigger before blurring :focus.
That was necessary for popup-opening buttons (settings, import_export,
etc.) so Escape could return focus to them — but it also fired for
non-popup toolbar buttons (list toggles, bold/italic, indent/outdent,
clearauthorship). For those, the stash held a stale reference that
interfered with subsequent editor interactions and regressed Playwright
tests: ordered_list, unordered_list, undo_clear_authorship.

Fix: only stash when the clicked command is a registered dropdown
(settings, import_export, embed, showusers, savedrevision,
connectivity). Other commands return focus to the ace editor as before
and leave _lastTrigger alone.

Verified locally on Chromium:
  - ordered_list.spec.ts: 6/6 pass (was 4/6)
  - unordered_list.spec.ts: 6/6 pass (was 4/6)
  - undo_clear_authorship.spec.ts: 2/2 pass (was 0/2)
  - a11y_dialogs.spec.ts: 11/11 pass (unchanged)

* fix(a11y): address Qodo review round 4 for PR #7584

#1 Stale aria-label after relocalize
  html10n.translateNode() refused to overwrite any existing aria-label,
  which also skipped updates on language change (pad.applyLanguage()
  re-runs localize). Use a `data-l10n-aria-label="true"` marker: set
  aria-label + marker when html10n populates it, overwrite only if the
  marker is present. Explicit template-supplied aria-labels stay as-is;
  html10n-generated ones refresh on relocalize.

#2 Escape won't close colorpicker
  _bodyKeyEvent caught Escape on any `.popup.popup-show` but only
  closed dropdown popups via toggleDropDown('none'). Popups opened
  outside the editbar framework (#mycolorpicker, toggled directly by
  pad_userlist.ts) stayed open while preventDefault() swallowed the
  key. Now the Escape branch manually closes any popup that
  toggleDropDown('none') cannot reach (non-dropdown ids, plus #users
  unless pinned) and leaves registered dropdowns for toggleDropDown to
  close so its focus-restore sees the transition.

#3 Stale focus restoration
  toggleDropDown('none') restored focus to _lastTrigger even when no
  popup was open on entry, which meant background callers
  (connectivity setup, periodic state handling) could yank focus out
  of the editor to a stale toolbar button. Gated the restore on
  `wasAnyOpen === true` so it only fires when there was a popup to
  close.

#11 English aria-label overrides i18n (export links, chat icon)
  Removed the hard-coded English aria-label from export anchors and
  removed aria-hidden from their inner localized spans. Screen readers
  now get the localized child text as the accessible name (Etherpad,
  HTML, PDF, etc.), matching the visible UI language.
  Removed the English aria-label from #chaticon and #titlesticky as
  well — both have data-l10n-id, so html10n populates a localized
  aria-label via the marker mechanism in #1. #titlecross keeps its
  static aria-label because it has no data-l10n-id yet.

#4 4-space indent in a11y spec
  Two tests had continuation lines at 4-space indent violating the
  repo's 2-space rule. Folded the signatures onto one line.

Updated a11y_dialogs.spec.ts to assert accessible-name presence rather
than hard-coded English for elements whose names now come from the
localized text. Still asserts static English for #titlecross (not
localized yet).

Verified locally (dev server restarted for each round):
  - a11y_dialogs.spec.ts: 11/11 on Chromium, 11/11 on Firefox
  - ordered_list + unordered_list + undo_clear_authorship: 13/13 on Chromium
  - Full backend suite: 799 passing, 0 failing
  - tsc --noEmit clean in our code

#9 Popup behavior documentation: deferred to a follow-up doc PR so
this PR stays focused on the a11y code changes. The new keyboard
behavior (Escape-to-close, focus-restore-to-trigger) is small enough
to summarize in a short doc/ addition.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-04-24 03:04:18 +01:00
John McLear
053f6d8343
fix(#7570): bundle DB drivers, add regression CI (#7572) 
* docs: design spec for issue #7570 (ueberdb2 driver bundling)

Spec for the upstream ueberDB fix (move 10 drivers back from optional
peer deps to dependencies) plus downstream etherpad-lite safety net
(explicit driver list + build-test-db-drivers CI job covering all 10
via presence check and MySQL+Postgres smoke tests).

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

* docs: implementation plan for issue #7570 ueberdb2 driver bundling

Covers upstream ueberDB PR (move drivers from optional peer deps back
to dependencies, publish 5.0.46) and downstream etherpad-lite PR
(bump ueberdb2, defensive driver list, build-test-db-drivers CI job
with presence + MySQL + Postgres stages gating publish).

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

* fix(#7570): bundle DB drivers, add regression CI

- Bump ueberdb2 to ^5.0.47 (upstream ueberDB PR #939 re-bundles drivers
  as real dependencies instead of optional peer deps, fixing the class
  of Docker-prod "Cannot find module" failures).
- Declare all 10 ueberdb2 DB drivers as direct src dependencies as a
  defensive safety net against a future upstream drift.
- Add build-test-db-drivers CI job that blocks the publish job:
    * all-10-drivers presence check in the built prod image
    * end-to-end MySQL smoke (reproduces the #7570 repro)
    * end-to-end Postgres smoke
  Any stage failure blocks Docker Hub / GHCR publish.

Supersedes #7571.

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

* fix(ci): run driver presence test from src/ so node_modules resolves

The presence test ran node from the default cwd (/opt/etherpad-lite),
but the drivers are installed under /opt/etherpad-lite/src/node_modules
by the monorepo workspace. Adding `-w /opt/etherpad-lite/src` makes
Node resolve modules from src/node_modules where pnpm places them.

Matches how the production container itself runs: `pnpm run prod` is
invoked from src/ (cross-env + node --require tsx/cjs node/server.ts).

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-04-20 18:10:01 +01:00