mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-19 01:24:19 +00:00
6085 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
John McLear
|
ea3032c079
|
i18n + a11y for admin disables warning (review feedback) (#7652)
Replace the hardcoded "Disables:" label and the inline title attribute with proper i18n keys (admin_plugins.disables.label, admin_plugins.disables.warning_title) and add role="alert" so screen readers announce the warning instead of treating it as visual noise. Per user review on #7649: "we should display it as a warning only if a plugin disables a test... Also i18n!!! Always remember to do i18n." Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
e028016296
|
feat(admin): surface ep.json disables in /admin plugin browser (#7649)
Companion to ether/ether.github.com#395 — the admin UI's "available plugins" listing now also renders the plugin's declared `disables` (see doc/PLUGIN_FEATURE_DISABLES.md) so an operator about to click Install sees the same warning as a user browsing etherpad.org/plugins: "Disables: chat". - src/node/types/PackageInfo.ts: optional `disables?: string[]` on the registry payload type. - admin/src/pages/Plugin.ts: same on the admin-side PluginDef. - admin/src/pages/HomePage.tsx: render an amber callout under the description when `disables` is present and non-empty. Plugins without a disables field render unchanged. The plugin-registry build pipeline still has to start surfacing `disables` from ep.json into plugins.json/plugins.viewer.json — until that lands, the new callout no-ops everywhere, which is fine. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
b769ab6e54
|
feat(test): feature tags + declared-disables contract for opt-out plugins (#7648)
Plugins that intentionally remove a baseline Etherpad feature
(ep_disable_chat, ep_disable_change_author_name, ep_disable_error_messages,
ep_disable_reset_authorship_colours) currently break core tests for the
removed feature. Their main branches are red, their auto-publish gates
never fire, and Dependabot PRs pile up.
The temptation is to give these plugins an "opt-out of these tests"
flag — but that's a self-serving attestation: a plugin can claim "I
just disable chat, ignore those tests" and quietly break unrelated
functionality on the user's install. etherpad.org/plugins would still
show it green.
This commit introduces a small declared-disables contract that closes
that gap:
1. Core specs grow @feature:* Playwright tags. Initial set:
@feature:chat, @feature:username, @feature:clear-authorship,
@feature:error-gritter. Tags are added test-by-test where the
test exercises a single feature, so the contract stays precise.
2. Plugins declare which feature tags they disable in their ep.json:
{ "name": "ep_disable_chat", "disables": ["@feature:chat"], ... }
3. bin/run-frontend-tests-with-disables.sh enforces the contract via
two passes:
- Pass 1 (regression): every test NOT in the disabled list must
pass. Catches plugins that break things they don't claim to.
- Pass 2 (honesty): every test that IS in the disabled list
must FAIL. Catches plugins that lie about disabling features
they don't actually disable, and stops them from grep-inverting
arbitrary unrelated tests.
4. doc/PLUGIN_FEATURE_DISABLES.md walks the design and migration.
The disables list is in ep.json (publicly visible), so etherpad.org/plugins
can surface "this plugin disables: chat" alongside the green CI badge —
users see what they're losing before they install.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
John McLear
|
90fd9b15b1
|
fix(plugins): updatePlugins.sh actually updates installed plugins (closes #6670) (#7644)
* fix(plugins): updatePlugins.sh actually updates installed plugins (#6670) bin/updatePlugins.sh detected outdated plugins by running `pnpm --filter ep_etherpad-lite outdated --depth=0`, but installed plugins are not registered in src/package.json — bin/plugins.ts adds them via linkInstaller.installPlugin which writes to src/plugin_packages/.versions/<name>@<version>/ and tracks the result in var/installed_plugins.json. pnpm has no view of them, so `outdated` returns empty and the script always reported "All plugins are up-to-date" even when newer versions existed on the registry. PR #7468 fixed npm→pnpm and install→update but kept the same broken detection mechanism, which is why the issue stayed open after that PR landed. Read the plugin list from var/installed_plugins.json instead, then re-invoke linkInstaller.installPlugin(name) for each entry. Calling the installer without a version pin resolves the registry-latest and overwrites the existing pinned copy, so an outdated plugin is brought to head while plugins already at latest are no-ops apart from the pnpm cache hit. Add an `update`/`up` action to bin/plugins.ts so users can also run `pnpm run plugins update` directly, mirroring the existing install/remove/list actions. updatePlugins.sh becomes a one-line wrapper for backwards compatibility. Reproduction (verified): pnpm run install-plugins ep_markdown@11.0.5 # latest is 11.0.18 ./bin/updatePlugins.sh # → 11.0.18 Edge cases tested: no plugins installed, missing installed_plugins.json, already-at-latest re-run. Closes #6670. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugins): validate ep_ prefix and dedupe + add regression test Qodo flagged two issues on the original update() addition: 1. Security — update() trusted every name in var/installed_plugins.json, so a corrupted or hand-edited manifest could coerce the script into installing arbitrary npm packages. pluginfw/plugins.getPackages already gates on the ep_ prefix; mirror that gate here. 2. Reliability — no automated regression test, so a future refactor could silently bring back the broken behaviour. Extract the safe-name filter to filterUpdatablePluginNames in bin/commonPlugins.ts (pure, side-effect-free, prefix configurable, also de-duplicates repeats so a duplicated entry installs once). Use it from plugins.ts update(). Add src/tests/backend/specs/filterUpdatablePluginNames.ts covering: keep prefixed names, drop ep_etherpad-lite, reject non-prefixed entries, de-dupe repeats, tolerate missing/null/non-string name fields, empty input, custom prefix. Manually verified end-to-end on a live install: an installed_plugins.json containing ep_markdown@11.0.5, a duplicate ep_markdown, and a "malicious-package" entry runs `Updating plugins to latest from registry: ep_markdown` (only) and ep_markdown ends up at 11.0.18 — the bad entries are silently filtered out. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
d8befd8491
|
test(ci): widen timing windows for flaky SessionStore + socket.io tests (#7647)
* test: widen timing windows for flaky CI tests on slow runners
SessionStore.ts and socketio.ts dominate develop CI failures (~25–40% of
recent runs). Both fail due to race conditions on slow Windows / loaded
Linux runners — not logic bugs.
SessionStore tests configure session expiry windows of 100ms and then
sleep 110ms before asserting. On a slow runner, the wall-clock between
`set()` and the assertion can exceed 100ms, the timeout in
`SessionStore._updateExpirations()` then sees `exp.real <= now` and
calls `_destroy()`, deleting the DB record before the assertion runs.
The test then reads `null` / `undefined` instead of the expected JSON.
Tripling each affected window (100→300, 110→330, 200→600) keeps the
relative timing semantics identical while leaving enough headroom for
a slow CI runner. Local run is +3s on this spec; cheap insurance for
the global flake rate.
`waitForSocketEvent` in tests/backend/common.ts uses a 1s timeout for
socket.io message round-trips. The socket.io handshake + auth +
CLIENT_READY can exceed 1s on a slow runner; bumped to 5s.
The most-failing tests this addresses:
- SessionStore: get of record from previous run (not yet expired)
- SessionStore: external expiration update is picked up
- SessionStore: shutdown cancels timeouts
- socketio: !authn anonymous cookie /p/pad -> 200, ok
- socketio: authn user /p/pad -> 200, ok
- clientvar_rev_consistency: CLIENT_VARS stays consistent under
concurrent edits during handshake
All 28 SessionStore + 33 socketio tests pass locally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: address Qodo PR feedback — configurable socket timeout, polling for cleanup
Both items raised in Qodo's review of #7647.
1) Hardcoded 5s socket wait
waitForSocketEvent() now takes an optional timeoutMs (default 1000ms,
matching pre-PR behaviour). Only the known-slow connect() and
handshake() paths pass 5000ms — they're the ones blowing the 1s budget
on loaded CI runners. Per-message waits (waitForAcceptCommit and
ad-hoc callers in messages.ts etc.) keep the 1s default so failures
surface fast with the descriptive helper error rather than the generic
Mocha timeout.
2) SessionStore waits still tight
Replaced fixed sleeps with a small `eventually()` poller for the three
"record should be gone after expiry" assertions:
- set of session that expires
- switch from non-expiring to expiring
- get of record from previous run (not yet expired)
These now poll every 25ms up to 2000ms so they pass immediately when
cleanup completes on a fast runner, and tolerate hundreds of ms of
event-loop delay on a slow one. No fixed coupling between sleep
duration and expiry duration.
For the inverse "record should still be there" case in
`shutdown cancels timeouts`, polling doesn't apply (we're verifying
that a cancelled timer did NOT fire, which requires a real wait past
the original expiry). Bumped expires 300→500ms and wait 330→700ms so
setup (set+get+shutdown) has 500ms before the timer would fire (vs.
30ms previously) and the 700ms wait still passes the original expiry.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
John McLear
|
d036cf0e70
|
fix(test): null padDeletionToken before pad init to stop modal focus theft (#7643)
PR #7546 added a one-time pad-deletion-token modal that opens via the clientVars handshake on creator sessions and synchronously focuses its input through setTimeout(0). `goToNewPad`'s previous mitigation hid the modal element after `waitForEditorReady`, but the editor iframe attaches before clientVars arrives, so the hide runs against a still- hidden modal, short-circuits, and the modal opens later mid-test — stealing focus and dropping the next Enter / Tab. Visible on develop in `enter.spec.ts:33` and `indentation.spec.ts:9` across all four Playwright jobs (run 25214868650). Intercept `clientVars` assignment via `page.addInitScript` and null out `padDeletionToken` before `pad.ts`'s `showDeletionTokenModalIfPresent` can read it, so the modal-show short-circuits at the source. The deletion-token spec navigates inline with `page.goto` and does not call this helper, so its modal still appears. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
4bda757304
|
feat(api): public compactPad API + bin/compactPad CLI over existing Cleanup (#7567)
* feat(pad): compactHistory() + compactPad CLI for DB-size reclaim Fixes #6194. Long-lived pads with heavy edit history dominate the DB — the issue describes a ~400 MB Postgres after two months with ~100 users. Etherpad keeps every revision forever, and removing arbitrary middle revisions is unsafe because state is reconstructed by composing forward from key revisions. What's safe: collapse the full history into a single base revision that reproduces the current atext. The existing `copyPadWithoutHistory` already does this for a new pad ID — this PR lifts that same changeset pattern into an in-place operation and wires up an admin CLI. - `Pad.compactHistory(authorId?)` (src/node/db/Pad.ts): composes the current atext into one base changeset, deletes all existing rev records, clears saved-revision bookmarks, and appends the new rev 0. Text, attributes, and chat history are preserved; saved-revision pointers are cleared. Returns the number of revisions removed. - `API.compactPad(padID, authorId?)` (src/node/db/API.ts): public-API wrapper around compactHistory. Reports `{removed}` so callers can log savings. - `APIHandler.ts`: register `compactPad` under a new `1.3.1` version, bump `latestApiVersion`. - `bin/compactPad.ts`: admin CLI. Reports the current revision count, calls compactPad via the HTTP API, and prints how many revisions were dropped. - `src/tests/backend/specs/compactPad.ts`: four backend tests cover the empty-pad no-op, the text-preservation + head=0 contract, saved-revision cleanup, and that subsequent edits continue to append cleanly on top of the collapsed base. The operation is destructive so admins must opt in explicitly; the CLI prints the before-count, and the recommended pre-flight is an `.etherpad` export (backup). Closes #6194 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(compact): delegate to copyPadWithoutHistory via temp-pad swap The initial compactHistory() implementation built a custom base changeset and re-ran appendRevision against a reset atext — but the changeset was packed with oldLength=2 (matching copyPadWithoutHistory's dest-pad init state) while the reset atext was only length 1, so applyToText tripped its "mismatched apply: 1 / 2" assertion and every test failed with a Changeset corruption error. Switch to the tested path instead: copy the pad via copyPadWithoutHistory to a uniquely-named temp pad (inherits all its attribute/pool/changeset correctness), read the temp pad's rev records back, delete the old ones under our pad's ID, write the new records in their place, update in-memory state to match, and remove the temp pad. Errors at any step fall through with a best-effort temp-pad cleanup. Contract shifts slightly: the collapsed pad is head<=1 rather than head=0, matching the shape of a freshly-imported pad (seed rev 0 + content rev 1). Tests updated to assert that invariant plus text-preservation, saved-revision cleanup, and append-after-compact. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(6194): match the head<=1 post-compact contract Tests previously asserted head=0 exactly after compaction; the temp-pad-swap path lands at head=1 (one seed rev plus one content rev) matching the shape of a freshly-imported pad. Relax the assertions to and derive the removed-count from before-head minus after-head, so the tests still catch regressions in text-preservation, saved-revision cleanup, and append-after-compact without being tied to the exact implementation shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(6194): wrap existing Cleanup instead of duplicating it Develop already ships a working revision-cleanup path under `src/node/utils/Cleanup.ts` with two public helpers — `deleteAllRevisions(padId)` (collapse full history via copyPadWithoutHistory) and `deleteRevisions(padId, keepRevisions)` (keep the last N). The admin-settings UI wires these up but neither is exposed on the public API, and there's no CLI for operators who want to run compaction outside the web UI. That's the gap this PR now fills. Changes from the prior revision of this PR: - Drop `pad.compactHistory()` — it re-implemented what `Cleanup.deleteAllRevisions` already does. Remove the duplicate. - `API.compactPad(padID, keepRevisions?)` now delegates to Cleanup: • keepRevisions null/undefined → deleteAllRevisions (full collapse) • keepRevisions >= 0 → deleteRevisions(N) (keep last N) Returns {ok, mode: 'all' | 'keepLast', keepRevisions?}. - APIHandler `1.3.1`: signature updated to take `keepRevisions` instead of `authorId`. - `bin/compactPad.ts`: accepts `--keep N` for the keep-last mode, shows before/after revision counts so operators see concrete savings. - Backend tests rewritten around the public API surface (mode reporting, text preservation, input validation) rather than internal method plumbing that no longer exists. Net: strictly a thin public-API and CLI veneer over already-tested Cleanup helpers. No new low-level logic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(6194): assert content markers, not byte-exact atext Cleanup.deleteAllRevisions internally calls copyPadWithoutHistory twice (src → tempId, tempId → src with force=true), and each round trip normalizes trailing whitespace. That meant my byte-exact atext.text assertion failed in CI: expected: '...line 3\n\n\n' actual: '...line 3\n' Swap the comparisons to use content markers (marker-alpha / beta / gamma, keep-line-N). The test still catches the real regressions — if compactPad lost content those markers would disappear — without coupling to whitespace quirks of the existing Cleanup implementation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(6194): correct API param + document compactPad in http_api docs The 1.3.1 entry in APIHandler registered `['padID', 'authorId']`, but `API.compactPad` takes `(padID, keepRevisions)` and the CLI sends a `keepRevisions` query param. APIHandler.handle dispatches by URL field name, so the previous wiring silently dropped `keepRevisions` and never ran the keep-last branch over HTTP. - Register `['padID', 'keepRevisions']` so the handler forwards the CLI/HTTP arg into the API function. - Add HTTP-level dispatch tests that hit `/api/1.3.1/compactPad` with and without `keepRevisions`. The direct `api.compactPad()` tests bypass the handler and would have missed this regression. - Document compactPad in `doc/api/http_api.md` and `http_api.adoc`, and bump the documented latest version from 1.3.0 to 1.3.1 to match `latestApiVersion`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(6194): add bin/compactAllPads for per-instance bulk compaction `bin/compactPad <padID>` covers the case where you know which pad is fat. For "reclaim space across the whole instance," composing `listAllPads` + `compactPad` yourself is annoying; this script does it. - Walks every pad on the instance and compacts it (full collapse, or `--keep N` keep-last). - Per-pad failures don't abort the run — they're logged, counted, and the script exits 1 if any failed. - `--dry-run` lists pads + revision counts without writing anything, so operators can scope impact before committing. - Reports `before → after` per pad and a total reclaimed count. Deliberately not adding a `compactAllPads` HTTP API: bulk compaction over a single HTTP request means one giant response and a long-held connection. Operators who want this should run it locally, where they can see progress and kill it cleanly. Staleness gating ("only pads older than X days") is tracked separately as a follow-up. Also registers `compactPad` and `compactAllPads` script aliases in `bin/package.json` so they show up next to the other admin CLIs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(6194): cover the bin/compactAllPads loop logic Previous commit added the script but only exercised it by hand. The loop itself — error tolerance, dry-run gating, keep-last passthrough, the empty-instance and listAllPads-failure paths — had no automated coverage. - Refactor compactAllPads.ts to export `runCompactAll(api, opts, logger)` and `parseArgs(argv)`. The CLI shell wires them up to axios+APIKEY for production; tests use an in-memory `CompactAllApi` so we don't need to stand up the apikey-auth path in mocha. - Add 9 specs covering: arg parsing, full-collapse iteration, --keep N passthrough, --dry-run skipping writes, single-pad failure not aborting the run, pre-flight count failure tolerated, a listAllPads failure short-circuiting cleanly, the empty-instance no-op, and a final end-to-end test that runs `runCompactAll` against the real `/api/1.3.1/compactPad` handler over supertest+JWT to catch contract drift between the CompactAllApi shape and the HTTP endpoints. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(6194): address Qodo review — gate, integer check, SSL Three valid concerns from the Qodo review on 75a08a13: 1. **cleanup.enabled gate.** The admin/Cleanup-socket path checks `settings.cleanup.enabled` before doing anything destructive; the public API was bypassing that gate. Now `compactPad` mirrors the admin path's check and returns a clear apierror when disabled, so exposing the API doesn't accidentally widen the cleanup-opt-in surface. 2. **Number.isFinite → Number.isInteger.** `2.5` was finite and non-negative, so the old check let it through into `Cleanup.deleteRevisions`, which does revision-index arithmetic that assumes integer math. Reject at the API boundary instead of silently misbehaving. 3. **SSL-aware baseURL in the bin scripts.** Other bin scripts hardcode `http://`, but the rest of the codebase uses `settings.ssl ? 'https' : 'http'`. The compact CLIs now do the same, so they work against HTTPS deployments. (Other bin scripts carry the same bug but fixing them is out of scope for this PR.) Tests: - New spec: `rejects fractional keepRevisions` (2.5 with the old check passed; the new one rejects). - New spec: `refuses to run when cleanup.enabled is false`. The existing API tests opt in via a before-hook + restore, so they still cover the success path under the new gate. - API docs (`http_api.md` + `http_api.adoc`) document the gate and the new error message. Skipped Qodo concerns: - "Wrong compactPad parameters" — already fixed in 26e12ff7 (the param map now correctly says `keepRevisions`, not `authorId`). - "Unbounded revision deletions" / "No session eviction" / changeset base-length / padCreate hook — these all targeted the earlier on-Pad implementation that was refactored away. The current code wraps `Cleanup.deleteAllRevisions` / `deleteRevisions`, which already handle concurrency, locking, and hook semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
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> |
||
|
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>
|
||
|
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> |
||
|
John McLear
|
63cae17720
|
feat(pad): add theme-color meta to match toolbar on mobile (#7606) (#7636)
* feat(pad): add <meta name="theme-color"> matching toolbar (#7606) Mobile browsers paint the address-bar / status-bar area above the viewport. Without theme-color this is a system color that does not match the Etherpad toolbar, leaving a visible gap above the pad. Render <meta name="theme-color"> server-side so the bar matches the configured toolbar on first paint. Light + dark variants are emitted with prefers-color-scheme media queries when dark mode is enabled. Colors are derived from settings.skinVariants via a new SkinColors helper (mirrors --bg-color in the colibris pad-variants.css). Closes #7606 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(timeslider): emit single theme-color matching configured toolbar Qodo flagged a mismatch: timeslider does not switch skin variants on prefers-color-scheme, so emitting a dark theme-color via media query would leave dark-mode devices with a dark address bar over a light toolbar. Drop the media-query metas on timeslider and emit one unconditional theme-color resolved from settings.skinVariants. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): emit unconditional theme-color so dark-OS users still match Qodo flagged that gating the light theme-color on prefers-color-scheme: light leaves no applicable meta on dark-OS devices when enableDarkMode is false — the address bar then uses a system color while the toolbar stays light. Drop the light media query so the light theme-color is the baseline, and let the prefers-color-scheme: dark meta override it when dark mode is enabled. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(theme-color): align dark meta with client-side super-dark override Two related Qodo findings on the SkinColors helper: - The pad client's dark-mode auto-switch (pad.ts L650) forces super-dark-toolbar regardless of the configured skinVariants, so the prefers-color-scheme: dark meta must always be #485365 — not whichever dark variant the operator configured. - When skinVariants only carries a dark token (e.g. dark-toolbar), the previous helper left the baseline meta at #ffffff, so light-OS users would see white above a dark toolbar. Replace toolbarThemeColors() with configuredToolbarColor() (used as the unconditional baseline) and a fixed DARK_MODE_TOOLBAR_COLOR constant (used in the prefers-color-scheme: dark meta). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(theme-color): server-side only, drop fragile dark media query Address remaining Qodo findings on the theme-color rollout: - (#1) Skip emitting the meta entirely when settings.skinName is not colibris — the helper only knows colibris's --bg-color values, so on no-skin or third-party skins the previous code would emit a white meta over a non-white toolbar. - (#4) Drop the prefers-color-scheme: dark variant. The pad's client-side dark mode is also gated on a localStorage white-mode override that no media query can express, so the dark meta could paint a dark address bar over a still-light toolbar. The single baseline meta always matches what the user sees on first paint. - (#8) Remove the redundant module.exports assignment; rely on the ES named export only (tsx handles the require() interop). - (#9) Iterate the toolbar variants in CSS source order and let the last match win, matching the cascade in pad-variants.css when multiple *-toolbar tokens are present. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
4704d80e82
|
ci: test ep_font_color and ep_hash_auth in with-plugins matrix (#7639)
* ci: add ep_font_color and ep_hash_auth to plugin test matrix These are the #12 and #14 most-installed Etherpad plugins on npm (last 30d) and were the only top-15 plugins not exercised by the withpluginsLinux / withpluginsWindows / Playwright with-plugins jobs. Adding them broadens coverage of the plugin loader against two real-world hooks: aceEditorCSS / aceAttribsToClasses (ep_font_color) and authenticate / handleMessage (ep_hash_auth). ep_hash_auth's authenticate hook is a no-op unless a Basic auth header is sent and a matching settings.users[user].hash exists, so it falls through cleanly with the default test settings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(change_user_color): close users popup before opening chat The "Own user color is shown when you enter a chat" spec leaves the users popup open after picking a color, then calls showChat(). In the with-plugins matrix the popup overlaps #chaticon and intercepts pointer events, so the click in showChat() is retried until the 90s timeout (× 5 retries ≈ 7m), failing both Firefox and Chrome with-plugins jobs. Toggle the users button off and wait for popup-show to drop before clicking the chat icon, matching the close pattern used in a11y_dialogs.spec.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
dependabot[bot]
|
1bbdc8fb9b
|
build(deps): bump jsdom from 29.1.0 to 29.1.1 (#7637)
Bumps [jsdom](https://github.com/jsdom/jsdom) from 29.1.0 to 29.1.1. - [Release notes](https://github.com/jsdom/jsdom/releases) - [Commits](https://github.com/jsdom/jsdom/compare/v29.1.0...v29.1.1) --- updated-dependencies: - dependency-name: jsdom dependency-version: 29.1.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
translatewiki.net
|
60f252916c
|
Localisation updates from https://translatewiki.net. | ||
|
John McLear
|
b8a950ee92
|
fix: delay anchor line scrolling until layout settles (#7544)
* fix: delay anchor line scrolling until layout settles Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: anchor reapply loop cancels on user interaction Addresses Qodo review: the 10s reapply loop could fight the user when they tried to scroll or click away from the anchored line. Listen for wheel/touchmove/keydown/mousedown on both ace_outer and ace_inner documents in capture phase and tear down the interval on first signal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: anchor reapply loop exits early once layout settles + FF rationale Addresses Qodo review on #7544: 1. Requirement gap (#1): Add stability detection to focusOnLine()'s reapply loop. When the target line's offsetTop has not changed for 3 consecutive 250ms ticks (~750ms), stop() is called early instead of running the full 10s window. This means once late content is no longer shifting layout, the loop releases the user immediately rather than waiting out maxSettleDuration. 2. Maintainability (#4): Add a comment explaining why the previous $.animate({scrollTop}) "needed for FF" path was replaced with a direct .scrollTop() call — the settle interval now covers the late-layout case Firefox originally needed animation for. Also adds a test that the reapply loop exits early so a user-initiated scrollTop=0 after ~2s is not reverted by another reapply tick. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(anchor-scroll): tolerance, min-settle window, missing-anchor bail-out Round 3 of Qodo review on #7544: #3 Early exit misses late shifts — image loads / plugin renders past my previous 750ms early-exit window were no longer corrected. Add a `minSettleDuration` of 2s before any early-exit can fire, and bump `stableTicksRequired` from 3 to 4. Hard ceiling stays 10s. #4 Offset equality prevents stability — strict === on `offset().top` never matched in the presence of sub-pixel rounding, so the loop ran the full 10s even on stable layouts. Switch to `Math.abs(...) < 1` tolerance. #7 Invalid anchors spin interval — when `getCurrentTargetOffset()` keeps returning null (the requested line never resolves), the loop used to run for the full 10s doing nothing. Track consecutive misses and `stop()` after `missingTicksRequired` (8 ticks ≈ 2s). Real "inner doc not yet rendered" cases get the first 2s window. Bump the early-exit test's wait from 2s → 3.5s to clear the new `minSettleDuration` + `stableTicksRequired` window before asserting. Pushing back on remaining Qodo items: #1 Defer scroll until layout settles — the design is "scroll once immediately so the user sees the line, then keep correcting". Deferring all scrolling until "stable" (which is unknowable up front) would visibly hang on `#L...` navigation for seconds while nothing happens. The reapply loop is the deferral. #6 FF rationale lost — already addressed in the previous commit (comment on the `scrollTop()` call explaining why the `$.animate({scrollTop})` "needed for FF" path was removed). Qodo's persistent review doesn't track resolution of items that aren't touched by the new commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
cafd60aa21
|
test(playwright): un-skip ep_headings2 spec under WITH_PLUGINS (#7634)
Remove the FRONTEND_IGNORE entry that suppressed ep_headings2/static/tests/frontend-new/specs/headings.spec.ts under WITH_PLUGINS=1. The skip was added in #7628 while the keystroke-drop flake (#7611) was still being chased; #7630 then identified the actual root cause as ep_cursortrace's per-keystroke cursorPosition socket spam saturating Firefox's input pipeline, removed ep_cursortrace from the WITH_PLUGINS plugin set, and added waitForEditorReady() to goToNewPad/goToPad. With both root causes addressed, this skip is likely stale — the spec's own "Option select is changed when heading is changed" test already uses insertText for the second-line typing, so it should clear the same bar that #7630 cleared for ep_markdown and ep_spellcheck (both now passing on develop). Closes ether/etherpad#7626 if CI confirms — the issue's three plugin specs (markdown, spellcheck, headings2) and timeslider_identity_changeset are all addressed once this lands. If headings2 is still flaky after this, FRONTEND_IGNORE comes back with a narrower comment about what specifically still races. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
bbd29683d9
|
ci(frontend-tests): exclude ep_cursortrace + un-flake 30 of 31 #7611 skips (#7630)
* test(playwright): wait for editor editability in goToNewPad/goToPad #editorcontainer.initialized fires after padeditor.init resolves but before ace flips the inner body from `class="static"` / contentEditable=false to editable. Under WITH_PLUGINS load in Firefox that flip can lag long enough that an immediate click + keyboard.type runs against a still-static body and is silently dropped — the body keeps showing the default welcome text and never sees our input. Most of the specs that currently carry `test.skip(WITH_PLUGINS)` markers (#7611) are racing exactly this flip. Block in goToNewPad / goToPad until the inner #innerdocbody is `contenteditable="true"`, so every spec starts from a known-ready editor without each having to add its own ad-hoc waits. Value-driven: exits as soon as ace flips the attribute, no fixed delay. Refactored into a private waitForEditorReady() helper so goToNewPad and goToPad share a single source of truth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): un-skip bold.spec under WITH_PLUGINS The two skipped tests fail because clicking the bold toolbar button right after selectAllText is intercepted by the #toolbar-overlay div (same root cause that needed force:true in clearAuthorship and ep_align). Add force:true to the click and drop the test.skip(WITH_PLUGINS) markers. The keypress variant doesn't click a toolbar button — it relies on the editor being editable when keyboard.press fires. The previous commit (waitForEditorReady in goToNewPad) covers that. Proof-of-concept un-skip; if CI confirms both pass, will expand the same pattern to the rest of the #7611 skip set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): make bold.spec robust to Firefox + WITH_PLUGINS The previous attempt at un-skipping these tests added force:true on the toolbar click but left the legacy selectAllText + keyboard.type sequence in place. Firefox under WITH_PLUGINS load racily drops keystrokes from per-key events, leaving an empty selection that the bold-on-click and Ctrl+B branches both no-op'd against — the asserts then timed out 5 retries deep with no <b> element. Replace the selectAllText + keyboard.type prelude with the standard clearPadContent + writeToPad pair. writeToPad uses insertText (one input event for the whole string) which is the same fix that unblocked ep_align in #7625. Verified locally on Firefox + WITH_PLUGINS=1: 2/2 pass in 15s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): un-skip 4 writeToPad-only specs under WITH_PLUGINS These four specs were marked test.skip(WITH_PLUGINS) for "flaky in with-plugins suite" but only use writeToPad / clearPadContent / goToNewPad — no direct keyboard.type, no toolbar button clicks. The flake was the editor not being ready when the test's first interaction fired (now covered by waitForEditorReady in goToNewPad/goToPad earlier in this branch) plus writeToPad's switch to insertText (#7625). - urls_become_clickable.spec.ts (file-level skip) - unaccepted_commit_warning.spec.ts - undo_clear_authorship.spec.ts - timeslider_follow.spec.ts Just removing the skip lines is enough; no other changes needed. Verified locally on Firefox + WITH_PLUGINS=1: all 40 tests across the four specs pass in 3m1s. urls_become_clickable contributes the bulk (37 tests via parameterised describes). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): un-skip page_up_down and timeslider_line_numbers under WITH_PLUGINS Both specs use writeToPad + keyboard.press for Page Up/Down, End, arrow keys, and the like — no per-character keyboard.type, no toolbar button clicks. The flake was the editor not being ready when the spec's first interaction fired (now covered by waitForEditorReady earlier in this branch) plus writeToPad's switch to insertText (#7625) for the multi-line setup. - page_up_down.spec.ts (3 skips) - timeslider_line_numbers.spec.ts (1 skip) Verified locally on Firefox + WITH_PLUGINS=1: 5/5 tests pass. enter.spec.ts deliberately left skipped — its Enter-in-a-loop test (line 33) drops keypresses under load and needs a value-driven per-iteration verify, separate change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): un-skip chat/list_wrap/clear_authorship; re-skip undo_clear_authorship Three more files cleared after the editor-ready helper landed: - chat.spec.ts (2 skips) — both clicks target settings-popup checkboxes, not toolbar buttons; the toolbar-overlay isn't in play, so just dropping the skips is enough. - clear_authorship_color.spec.ts (1) — uses the existing clearAuthorship helper, which already runs with force:true. - list_wrap_indent.spec.ts (1) — adds force:true to the .buttonicon-insertorderedlist click that fires after selectAllText (same pattern as bold.spec). Reverts the un-skip on undo_clear_authorship.spec.ts: that one spawns two browser contexts and races against User B's writeToPad landing in the second pad. Hit a real flake locally where User B's text never appeared. Needs a per-user "wait for text to commit" before the assertion. Re-add the skip until that fix is in. Verified locally on Firefox + WITH_PLUGINS=1: 16 passed across the three un-skipped files (one undo_clear_authorship retry flaked, hence the revert). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): un-skip alphabet/delete/select_focus_restore under WITH_PLUGINS - alphabet.spec.ts (1) — swapped page.keyboard.type for writeToPad - delete.spec.ts (1) — same swap - select_focus_restore.spec.ts (1) — left keyboard.type in place (the test specifically verifies that focus returns to the editor after a toolbar select change; replacing with writeToPad would re-focus the body via a click and mask the bug being asserted). Editor-ready wait alone is enough here. Verified locally on Firefox + WITH_PLUGINS=1: 3/3 pass in 23s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): un-skip bold_paste + undo_redo_scroll under WITH_PLUGINS - bold_paste.spec.ts (1) — already used writeToPad; just dropping the skip is enough now that the editor-ready helper landed. - undo_redo_scroll.spec.ts (2) — replaced the `for (45 lines) { keyboard.type; keyboard.press('Enter') }` loop with a single writeToPad of `lines.join('\\n') + '\\n'`. writeToPad drives input via insertText (one input event per line) which Firefox under WITH_PLUGINS load handles without dropping events. The Ctrl+Z scroll-to-caret behaviour the test asserts is unchanged — each line still lands in its own changeset for the undo module to reverse. Verified locally on Firefox + WITH_PLUGINS=1: bold_paste passes clean; undo_redo_scroll passes via the existing per-spec `retries: 2` config (the scroll timing race exists pre-change and is what motivates the retries). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): un-skip unordered_list 'enter for the new line' under WITH_PLUGINS - Add force:true on the .buttonicon-insertunorderedlist click to bypass #toolbar-overlay (same pattern as clearAuthorship and bold.spec). - Replace the keyboard.type('line 1'); keyboard.press('Enter'); keyboard.type('line 2'); keyboard.press('Enter'); sequence with a single writeToPad('line 1\\nline 2\\n') — insertText per line + Enter between, which Firefox under WITH_PLUGINS load handles without dropping events. The trailing newline preserves the final Enter the original spec relied on. Verified locally on Firefox + WITH_PLUGINS=1: passes in 8s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): un-skip all 4 ordered_list tests under WITH_PLUGINS - issue #4748 + #1125: add force:true on .buttonicon-insertorderedlist clicks (toolbar-overlay interception after selection); collapse the per-line keyboard.type + keyboard.press('Enter') sequences into single writeToPad calls with embedded newlines. - issue #5160 and #5718 already used force:true and writeToPad throughout; just dropping the skip is enough now that the editor-ready helper landed. Verified locally on Firefox + WITH_PLUGINS=1: 11 passed (4 ordered_list + 5 unordered_list, plus 2 sub-describes). 1m24s total. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): un-skip all 4 indentation tests under WITH_PLUGINS Same pattern as bold/ordered_list/unordered_list: - force:true on .buttonicon-indent / .buttonicon-bold / .buttonicon-outdent clicks (toolbar-overlay interception after a text selection). - Replace per-line keyboard.type + keyboard.press('Enter') sequences with single writeToPad calls using \\n separators. - Replace single-character keyboard.type calls (':', '(', '[', '{{') with keyboard.insertText for consistency. The keypress and indent/outdent button tests were already passing without WITH_PLUGINS skips — only the four tests that race the toolbar click + typing sequence were skipped. With force:true and writeToPad they're stable. Verified locally on Firefox + WITH_PLUGINS=1: 12 tests pass across indentation, ordered_list, unordered_list, list_wrap_indent (matched by the indent grep). 1m11s total. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): un-skip enter.spec 'enter is always visible' under WITH_PLUGINS The test fired 15 keypress('Enter') calls in a tight loop with no per-iteration verify. Under Firefox + WITH_PLUGINS load the editor's input pipeline can't always keep up while plugin hooks are warming, so a few presses get dropped and the final `expect(div.count).toBe(numberOfLines + originalLength)` fails with too few lines. Add a value-driven `expect(div).toHaveCount(originalLength + i + 1)` after each press. The loop only advances once the editor has acknowledged the previous Enter, so dropped events become slow events instead of lost ones. Verified locally on Firefox + WITH_PLUGINS=1: passes in 11s (would have been 1.5m timeout previously). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): un-skip undo_clear_authorship under WITH_PLUGINS The two-user test was racing on User B's keyboard.type('Hello from User B') and 'Still connected!' — Firefox + WITH_PLUGINS load drops keystrokes from per-key events, leaving the second pad with truncated text that the body1 round-trip assertion never matches. Replace both keyboard.type calls with keyboard.insertText (single input event). Cannot use writeToPad here because the test relies on the caret position established by the preceding End + Enter — a writeToPad would re-click the body and reset focus. Verified locally on Firefox + WITH_PLUGINS=1: both tests pass clean in 30s (previously failed all retries at 1m+ each). The test.describe.configure({retries: 2}) is kept as belt-and-braces for the multi-context server propagation race that this test exercises legitimately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): un-skip collab_client 'bug #4978 regression test' under WITH_PLUGINS The test's replaceLineText helper used keyboard.type(newText) to insert the replacement string after a Backspace clear. Firefox under WITH_PLUGINS load drops keystrokes from per-key events, leaving the line with truncated text that the cross-context assertions (body1.toHaveText(user2Text), body2.toHaveText(user1Text)) never match. Switch the type to keyboard.insertText (single input event) — same fix that unblocked ep_align in #7625 and the other typing-races in this branch. The selectText + Backspace + insertText pattern still exercises the legitimate collab race the test asserts (concurrent edits over the COLLABROOM). Verified locally on Firefox + WITH_PLUGINS=1: passes in 15s. This was the last of the 31 test.skip(WITH_PLUGINS, '#7611') markers in src/tests/frontend-new/specs/. The branch goal of zero #7611 skips is met. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): use stable l10n selector for OL toolbar button Qodo flagged the .first() call in #4748's setup as DOM-order dependent: a future plugin that adds another element carrying the .buttonicon-insertorderedlist class would silently change which button the test clicks. Switch to button[data-l10n-id='pad.toolbar.ol.title'] (the localizationId declared in src/node/utils/toolbar.ts), which is unique to the core ordered-list toolbar entry. Drop the now-unnecessary .first(). The class-based locator remains in #5160, #5718, and the indent/ outdent sub-describes; those don't strict-mode-match more than one element today, but a follow-up could swap them too for consistency if reviewers want. Verified locally on Firefox + WITH_PLUGINS=1: passes in 7s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): tighten writeToPad Enter delivery + fix toolbar overlay regressions Three fixes for the failures that surfaced once #7630 ran in CI on Firefox + WITH_PLUGINS at the full matrix: 1. **writeToPad** now value-waits per Enter and retries up to 3 times if the editor doesn't acknowledge a new line. Long multi-line writes (e.g. timeslider_follow's #4389 setup with ~120 newlines) were dropping Enters faster than the previous single-press loop tolerated. The retry surfaces the canonical "expected N, got M" timeout if all 3 attempts fail. 2. **unordered_list.spec.ts**: every `.buttonicon-*` toolbar click now uses {force: true}. Two of the un-skipped tests intermittently missed the click under load because #toolbar-overlay intercepts pointer events after a text selection (same pattern as bold, ep_align, et al.). Body clicks (clicks inside the iframe pad body) are unaffected and stay as plain `.click()`. 3. **timeslider_follow.spec.ts** "regression test for #4389": re-skipped under WITH_PLUGINS with a specific note. The 120-Enter setup races plugin load even with the new writeToPad retry — re-press attempts overshoot the exact line count when a "dropped" Enter eventually lands. Needs a fundamentally different setup approach (REST API import, clipboard paste, etc.) to un-skip reliably; out of scope here. Net: 30 of the original 31 #7611 skips remain removed (was 31/31 before; the one re-skip is a documented known-aggressive case). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): revert writeToPad per-Enter retry — overshoots cause more failures The per-Enter value-wait + retry I added in fc45d71e5 was meant to catch dropped Enters in long multi-line writes, but in CI it made things worse: when a "dropped" Enter eventually landed during the retry's short poll window, the next iteration's exact line-count expectation was off by one and the retry loop overshot, breaking tests that previously passed (urls_become_clickable, language, inner_height all hit toHaveCount mismatches that didn't exist before). Revert to the simpler insertText + bare keyboard.press('Enter') loop. Tests with extreme line counts (timeslider_follow #4389, ~120 Enters) stay re-skipped from the prior commit; everything else accepts the same intermittent flake the helper exhibited before this fix attempt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): re-skip 8 tests that need deeper rework to un-skip Honest scope adjustment after CI surfaced load-dependent failures that local single-run verification missed. The previous batches worked at low concurrency but flake at the full Playwright matrix under WITH_PLUGINS: - bold_paste.spec.ts — clipboard / paste race between specs - collab_client.spec.ts (bug #4978) — multi-context cross-pad propagation under load - enter.spec.ts (enter is always visible) — 15-Enter loop drops presses faster than the per-iteration value-wait can recover - timeslider_follow.spec.ts (content as it's added) — 66 sequential Enters across 6 writeToPad calls - undo_clear_authorship.spec.ts (describe-level) — multi-context; the cross-pad text-arrival assertion races - undo_redo_scroll.spec.ts (describe-level) — 45-line writeToPad setup; scroll-position assertion needs stable layout - unordered_list.spec.ts (Keeps unordered list on enter) — toolbar click + writeToPad with embedded newline races All carry inline comments explaining the specific load issue and referencing #7611 so a follow-up that introduces a REST-driven or clipboard-paste setup mechanism can target them concretely. Net: 23 of 31 #7611 skips removed (74%). The deferred 8 share two underlying limitations that need infrastructure work: 1. No reliable way to drive >10 sequential Enters under load without occasional drops 2. No reliable cross-context propagation wait helper Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * DO-NOT-MERGE bisect plugins: Firefox×HALF-A + Firefox×HALF-B One CI run, both halves of the standard plugin set, both on Firefox (which is the project that reliably trips the flake we're chasing). Playwright Firefox with plugins → HALF A: ep_align, ep_author_hover, ep_cursortrace, ep_font_size, ep_headings2 Playwright Chrome with plugins → HALF B: ep_markdown, ep_readonly_guest, ep_set_title_on_pad, ep_spellcheck, ep_subscript_and_superscript, ep_table_of_contents (job runs --project=firefox here too) Decision matrix on next CI: - Both fail → load alone is the cause; deeper rework needed. - Only A fails → culprit is in HALF A (5 candidates). - Only B fails → culprit is in HALF B (6 candidates). - Both pass → flake threshold sits between 5–6 plugins; the culprit is whichever 2-plugin pair from the full set tips the load above threshold; iterate. Revert this commit before merging — it's purely a CI probe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * DO-NOT-MERGE bisect plugins iter 2: A1 (align,author_hover) vs A2 (cursortrace,font_size,headings2) Iteration 1 isolated to HALF A. Splitting: Playwright Firefox with plugins → A1: ep_align, ep_author_hover Playwright Chrome with plugins → A2: ep_cursortrace, ep_font_size, ep_headings2 (still --project=firefox) Decision matrix: - Both fail → load alone tips it; ≥2 of these 5 are needed. - Only A1 fails → culprit is ep_align or ep_author_hover. - Only A2 fails → culprit is ep_cursortrace, ep_font_size, or ep_headings2. - Both pass → flake threshold is between 2 and 3 plugins from A, revisit splitting (could be a specific pair). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * DO-NOT-MERGE bisect plugins iter 3: A2a (cursortrace) vs A2b (font_size, headings2) Iteration 2 isolated to A2 (cursortrace+font_size+headings2). Iter 3 singles out ep_cursortrace: Playwright Firefox with plugins → A2a: ep_cursortrace Playwright Chrome with plugins → A2b: ep_font_size, ep_headings2 (still --project=firefox) Decision matrix: - Only A2a fails → ep_cursortrace is the culprit (1 plugin alone tips it). - Only A2b fails → culprit is ep_font_size or ep_headings2. - Both fail → load tips at >=1 plugin from this set; investigate each individually. - Both pass → load tips at >=3 plugins; revisit splitting. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * DO-NOT-MERGE bisect plugins iter 4 (confirm): all-minus-cursortrace Iter 3 isolated to ep_cursortrace alone. Confirming by running the inverse — every other plugin in the standard set, no ep_cursortrace — on TWO Firefox runs in parallel: Playwright Firefox with plugins → align, author_hover, font_size, headings2, markdown, readonly_guest, set_title_on_pad, spellcheck, subscript_and_superscript, table_of_contents Playwright Chrome with plugins → same 10 plugins (still --project=firefox per probe) Both pass → ep_cursortrace is conclusively the culprit. Either fails → load is the cause and the bisection mis-attributed (would need to investigate why iter 3 cursortrace-only failed: maybe a flaky one-off). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci(frontend-tests): exclude ep_cursortrace from with-plugins set Bisected via 4 CI iterations on this branch. ep_cursortrace's `aceEditEvent` hook (static/js/main.js in the plugin) fires on every keyboard event — handleClick, handleKeyEvent, idleWorkTimer — and unconditionally sends a `cursorPosition` socket message via `pad.collabClient.sendMessage` per call. Under the test harness's writeToPad bursts (insertText + Enter loops) that stream of socket messages saturates the editor's input pipeline in Firefox specifically, causing intermittent keystroke drops and the entire class of #7611 flakiness this PR was originally chasing. Confirmation runs: - 11-plugin set including ep_cursortrace → fails on Firefox - HALF B (5 plugins, no cursortrace) → passes - HALF A (5 plugins, with cursortrace) → fails - A1 (align, author_hover) — no cursortrace → passes - A2 (cursortrace, font_size, headings2) → fails - A2a (cursortrace alone, 1 plugin) → fails - A2b (font_size, headings2, no cursortrace) → passes - 10-plugin set, all minus ep_cursortrace → passes (×2 jobs) Drop ep_cursortrace from the frontend-tests.yml plugin set and restore all the un-skips that this PR pessimistically re-skipped during the load-symptom whack-a-mole. The plugin itself needs a debounce/throttle around its socket send before it can come back into the test set; tracked separately in the ep_cursortrace repo. Backend tests / docker / etc remain on the original 11-plugin set since they don't trip the same input-pipeline race. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
9a9659c110
|
feat(editor): add showMenuRight URL param to hide right-side toolbar (#7553)
* feat(editor): add showMenuRight URL param to hide right-side toolbar Adds a showMenuRight URL/embed parameter. When set to false, the right-side toolbar (.menu_right — import/export, timeslider, settings, share, users) is hidden. Default behavior (menu shown) is unchanged. Motivated by read-only / announcement-pad embeds where viewers shouldn't see those controls, but the same server hosts editable pads where the buttons must remain available (so globally disabling them in settings.json is not a fit). Closes #5182 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(editor): auto-hide menu_right on readonly pads, accept showMenuRight=true override Addresses Qodo review feedback on #7553: 1. Readonly pads now hide the right-side toolbar automatically. The original issue (#5182) was specifically about readonly embeds; the previous implementation only honoured an explicit `?showMenuRight=false` URL parameter, which meant that vanilla readonly pads still showed import/export/timeslider/settings/share/users controls — all noise for viewers who can't interact with the pad anyway. 2. Callers who still want the menu visible on readonly pads can opt back in with `?showMenuRight=true`. The URL-param callback now accepts both values instead of just `false`. 3. The Playwright spec's `browser.newContext() + clearCookies()` pattern was a no-op because the test navigated with the existing `page` fixture (different context). Switch to `page.context().clearCookies()`, and cover both the auto-hide and the explicit-override paths on a readonly-URL navigation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(7553): use actual readonly-URL selector in Playwright spec The previous test looked up (capital-I) and called inputValue() on it. The real element is (lowercase) and it's a toggle checkbox, not a URL field. The readonly URL itself is in `#linkinput`, updated live when the readonly checkbox is checked. Wire the test to that flow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(7553): wait for share popup before clicking readonly checkbox Playwright's stability check kept retrying the click while the popup was animating open ("element is not stable"). Wait for #embed.popup-show and use click({force: true}) so a trailing CSS transform doesn't retrigger the instability backoff. Also wait for #linkinput to update to the readonly URL before reading it — the checkbox change is asynchronous. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
884ac93b4e
|
feat(editor): add IDE-style line ops (duplicate / delete) (#7564)
* feat(editor): add IDE-style line ops (duplicate / delete) Addresses #6433 — the issue asked for VS-Code-style multi-line editing for collaborative markdown editing. Full multi-cursor support would need a rep-model rewrite; this PR lands the two highest-value single-cursor line ops now so users get the actual ergonomic wins without that lift: - Ctrl/Cmd+Shift+D: duplicate the current line, or every line in a multi-line selection. Duplicates land directly below the original block, so the caret visually stays with the original content — same as VS Code / JetBrains. - Ctrl/Cmd+Shift+K: delete the current line (or every line in a multi-line selection), collapsing the range including its trailing newline. Handles edge cases: last-line selections consume the preceding newline; a whole-pad selection leaves one empty line behind (Etherpad always expects at least one). Both ops run through `performDocumentReplaceRange`, so they're collaborative-safe: other clients see the change arrive as a normal changeset, and the operation is a single undo entry. Wire-up: - `src/node/utils/Settings.ts`: extend `padShortcutEnabled` with `cmdShiftD` / `cmdShiftK` (both default true so fresh installs get the feature without config; operators who pin shortcut maps can disable them individually). - `src/static/js/ace2_inner.ts`: new `doDuplicateSelectedLines` / `doDeleteSelectedLines` helpers, exposed on `editorInfo.ace_*` so plugins and tests can invoke them programmatically, and keyboard handlers for Ctrl/Cmd+Shift+D and Ctrl/Cmd+Shift+K. Test plan: Playwright spec covers the three interesting paths (single-line duplicate, single-line delete, multi-line duplicate). Closes #6433 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(6433): type the bodyLines helper parameter * fix(6433): preserve char attributes on duplicate + correct whole-pad delete Addresses Qodo review feedback on #7564: 1. `doDuplicateSelectedLines` was inserting raw line text via `performDocumentReplaceRange`, which carries only the author attribute — every other character-level attribute on the source line (bold, italic, list, heading, link) was dropped, and in some cases Etherpad's internal `*` line-marker surfaced as literal text. Rewrite to build the changeset directly: walk each source line's attribution ops from `rep.alines[i]`, split the line text at op boundaries, and call `builder.insert(segment, op.attribs)` once per op. Each attribute segment from the source ends up on the duplicate verbatim. Wrapped in `inCallStackIfNecessary` for the standard fastIncorp + submit cycle. 2. `doDeleteSelectedLines` whole-pad case deleted from `[0, 0]` to `[0, lastLen]` even when the selection spanned multiple lines, leaving later lines in place and sometimes producing an invalid range when `lastLen` exceeded line 0's width. Change to `[end, lastLen]` so every selected line is cleared, with one empty line retained for the final-newline invariant. 3. Added `ace_doDuplicateSelectedLines` / `ace_doDeleteSelectedLines` entries to `doc/api/editorInfo.md` so plugin authors can discover the new surface. 4. New Playwright spec asserting `<b>` tags survive duplication. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * revert(6433): drop the attributed-duplicate changeset, keep whole-pad delete fix The attributed-changeset rewrite for doDuplicateSelectedLines tripped over the insertion-past-final-newline edge case — CI caught the basic single-line duplicate regressing (gamma → [alpha, beta, gamma] with no new gamma appearing because the hand-rolled changeset ended up invalid at the end-of-pad boundary). performDocumentReplaceRange handles that edge case internally, but only with a uniform author-attribute insert. Revert duplicateSelectedLines to the simpler performDocumentReplaceRange form that CI was happy with. Flag the attribute-preservation gap explicitly in the code so a follow-up can bolt on a proper attributed insert without re-inventing the end-of-pad handling. Whole-pad delete fix and editorInfo.md docs stay. Attribute-preservation test in line_ops.spec.ts is removed along with the broken code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
6c9ed46957
|
test: use selectAllText helper instead of raw Control+A in timeslider spec (#7629)
Under Firefox + WITH_PLUGINS the keyboard.down/press/up('Control')
chord races with the focus delegation into the inner ace iframe and
can drop either the Control or the A keystroke, so the subsequent
Backspace deletes a single character rather than the line and the
"delete everything" revision the test relies on never gets created.
selectAllText runs inside the inner frame's selection model, which
isn't subject to that race.
Resolves the firefox failure on
'timeslider playback advances through all revisions including
identity changesets' tracked in #7626.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
SamTV12345
|
c55007361c
|
chore: updated node to supported 22,24,25 (#7628)
* chore: updated node to supported 22,24,25 * chore: updated node to supported 22,24,25 * chore: updated node to supported 22,24,25 * chore: updated node to supported 22,24,25 * chore: upgrade deb * chore: upgrade dockerfile * chore: use explicit node * chore: use node 22 * chore: use node 22 |
||
|
SamTV12345
|
74d1715c1b
|
chore: updated clients to esm (#7627) | ||
|
John McLear
|
f6a56ec2cb
|
test(playwright): use insertText so Firefox stops dropping keystrokes (#7625)
writeToPad has been calling page.keyboard.type, which fires one keydown/keyup per character against the contenteditable. Under WITH_PLUGINS load Firefox's input pipeline can't keep up with the per-event firing while plugin hooks are still warming, and randomly swallows characters from the tail of the string — pad ends up with e.g. "aligned tex" instead of "aligned text". The dropped character is irrecoverable: there is no event to retry against. Switch to page.keyboard.insertText, which dispatches a single input event per call. Etherpad's incorporateUserChanges loop reads the resulting DOM atomically, so the result is identical to what real typing produces — minus the per-key race. insertText does not translate \n into Enter (it concatenates "One\nTwo" into "OneTwo"), so split on newlines and press Enter between segments to preserve multi-line input that the existing callers (timeslider_line_numbers, page_up_down, etc.) rely on. Verified locally on Firefox + WITH_PLUGINS: - ep_align Alignment: 4/4 pass (previously 0/4 even after retries) - italic.spec: 2/2 pass - timeslider_line_numbers (multi-line): pass Chromium remains green. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
1eea9de08c
|
ci: run frontend tests with /ether plugin set (closes #7608) (#7609)
* ci: run frontend tests with /ether plugin set (closes #7608) Mirrors backend-tests.yml's withpluginsLinux: installs the same 11 ep_* plugins (ep_align, ep_author_hover, ep_cursortrace, ep_font_size, ep_headings2, ep_markdown, ep_readonly_guest, ep_set_title_on_pad, ep_spellcheck, ep_subscript_and_superscript, ep_table_of_contents) and runs Playwright Chromium + Firefox against them. Re-introduces frontend-with-plugins coverage that was lost in commit |
||
|
John McLear
|
7f76aa2b81
|
ci(playwright): discover plugin frontend specs (closes #7622) (#7623)
* ci(playwright): discover plugin frontend specs from node_modules + plugin_packages
Adds two new globs to the Playwright testMatch so any installed
plugin shipping specs at the conventional location is picked up
automatically:
- ../node_modules/ep_*/static/tests/frontend-new/specs/**/*.spec.ts
(covers `pnpm add -w ep_*` workspace installs, e.g. CI's
with-plugins matrix and dev-time pnpm installs)
- plugin_packages/ep_*/static/tests/frontend-new/specs/**/*.spec.ts
(covers admin-UI / live-plugin-manager installs into
src/plugin_packages)
Mirrors the equivalent backend pattern (`mocha ...
../node_modules/ep_*/static/tests/backend/specs/**`) which already
auto-discovers plugin backend specs.
This re-enables coverage that was lost in commit
|
||
Luc Didry
|
b16e4ff6d3
|
🩹 — Avoid duplicate key "types" in tsconfig (#7610) | ||
|
dependabot[bot]
|
7153b97363
|
build(deps): bump oidc-provider from 9.8.2 to 9.8.3 (#7619)
Bumps [oidc-provider](https://github.com/panva/node-oidc-provider) from 9.8.2 to 9.8.3. - [Release notes](https://github.com/panva/node-oidc-provider/releases) - [Changelog](https://github.com/panva/node-oidc-provider/blob/main/CHANGELOG.md) - [Commits](https://github.com/panva/node-oidc-provider/compare/v9.8.2...v9.8.3) --- updated-dependencies: - dependency-name: oidc-provider dependency-version: 9.8.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
73910f099e
|
build(deps): bump express-rate-limit from 8.4.0 to 8.4.1 (#7616)
Bumps [express-rate-limit](https://github.com/express-rate-limit/express-rate-limit) from 8.4.0 to 8.4.1. - [Commits](https://github.com/express-rate-limit/express-rate-limit/compare/v8.4.0...v8.4.1) --- updated-dependencies: - dependency-name: express-rate-limit dependency-version: 8.4.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
b1a1232a2f
|
build(deps): bump jsdom from 29.0.2 to 29.1.0 (#7617)
Bumps [jsdom](https://github.com/jsdom/jsdom) from 29.0.2 to 29.1.0. - [Commits](https://github.com/jsdom/jsdom/compare/v29.0.2...v29.1.0) --- updated-dependencies: - dependency-name: jsdom dependency-version: 29.1.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
1584e0eed0
|
build(deps): bump mysql2 from 3.22.2 to 3.22.3 (#7618)
Bumps [mysql2](https://github.com/sidorares/node-mysql2) from 3.22.2 to 3.22.3. - [Changelog](https://github.com/sidorares/node-mysql2/blob/master/Changelog.md) - [Commits](https://github.com/sidorares/node-mysql2/compare/v3.22.2...v3.22.3) --- updated-dependencies: - dependency-name: mysql2 dependency-version: 3.22.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
75c45377d7
|
build(deps): bump jose from 6.2.2 to 6.2.3 (#7620)
Bumps [jose](https://github.com/panva/jose) from 6.2.2 to 6.2.3. - [Changelog](https://github.com/panva/jose/blob/main/CHANGELOG.md) - [Commits](https://github.com/panva/jose/compare/v6.2.2...v6.2.3) --- updated-dependencies: - dependency-name: jose dependency-version: 6.2.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
translatewiki.net
|
20cb54bb4d
|
Localisation updates from https://translatewiki.net. | ||
|
John McLear
|
0b40bfc784
|
feat(packaging): add Debian (.deb) build via nfpm with systemd unit (v2) (#7583)
* feat(packaging): add Debian (.deb) build via nfpm with systemd unit First-class Debian packaging for Etherpad, producing etherpad_<version>_<arch>.deb artefacts for amd64 and arm64 from a single nfpm manifest. Installing the package gives users: - /opt/etherpad with a prebuilt, self-contained node_modules/ — no pnpm required at runtime, just `nodejs (>= 20)`. - etherpad system user/group, created via `adduser` in preinst. - /etc/etherpad/settings.json seeded from the template on first install, preserved across upgrades, removed on `purge`. Seed rewrites dbType from the template's dev-only `dirty` default to `sqlite`, pointed at /var/lib/etherpad/etherpad.db so fresh installs get an ACID-safe DB without manual config. sqlite is shipped by ueberdb2 (rusty-store-kv), so no additional apt deps are needed. - /var/lib/etherpad owned by etherpad:etherpad, writable under the hardened unit's ProtectSystem=strict. - /lib/systemd/system/etherpad.service — hardened unit (NoNewPrivileges, ProtectSystem=strict, ProtectHome, PrivateTmp, RestrictAddressFamilies) with Restart=on-failure. - /usr/bin/etherpad CLI wrapper running `node --import tsx/esm`. CI (.github/workflows/deb-package.yml) triggers on v* tags, builds both arches via native runners (ubuntu-latest + ubuntu-24.04-arm), smoke-tests the amd64 package end-to-end (install → verify sqlite default → systemctl start → curl /health → purge → confirm user removed), and attaches the artefacts to the GitHub Release. Re-introduces the work from #7559 (reverted in #7582) with two corrections: 1. Package name and all installed paths use `etherpad`, not `etherpad-lite` — matches the repo rename. Kept replaces/conflicts on `etherpad-lite` so any dev builds of the reverted PR upgrade cleanly. 2. Default dbType is `sqlite`, not `dirty`. The template's own comment says dirty is for testing only; shipping it by default to everyone who runs `apt install etherpad` is the wrong tradeoff for a production package. Publishing to an APT repo (Cloudsmith, Launchpad PPA, self-hosted reprepro) is intentionally out of scope — needs a governance decision on who holds the signing key. Recipes are documented in packaging/README.md. Refs #7529, #7559, #7582 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(packaging): address PR review — startup crashes, supply chain, Node LTS Addresses Qodo and SamTV12345 review feedback on #7583: - postinstall: symlink /opt/etherpad/var → /var/lib/etherpad/var so ProtectSystem=strict doesn't block runtime writes (var/js, installed_plugins.json, etc.). Existing ReadWritePaths covers it. - postinstall: seed installed_plugins.json with ep_etherpad-lite so checkForMigration() does not spawn `pnpm ls` on first boot — pnpm is not a runtime dep, and the bundled node_modules already contains every shipped plugin. Prevents network plugin installs at first run. - postremove: clean up the new var symlink on remove. - workflow: verify nfpm .deb sha256 against upstream checksums.txt before sudo dpkg -i (defense in depth). - workflow: bump Node 22 → 24 (current LTS, per SamTV12345). The deb Depends stays at nodejs (>= 20) to match Etherpad's engines.node. - workflow: smoke-test now asserts the var symlink and seeded installed_plugins.json exist post-install. - workflow: publish stable etherpad-latest_{amd64,arm64}.deb aliases alongside the versioned files in the GitHub Release. - README: bump Node guidance to 24, document /releases/latest URL, link to engines.node floor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(packaging): tsx CJS hook, plugin paths writable, glob tag triggers Addresses second-round Qodo review on #7583: - bin/etherpad: switch from `--import tsx/.../esm` to `--require tsx/cjs`. server.ts uses `exports.start = ...` which throws under the ESM loader; the prod script in src/package.json uses tsx/cjs for the same reason. - postinstall: symlink /opt/etherpad/src/plugin_packages → /var/lib/etherpad/plugin_packages and chgrp /opt/etherpad/src/node_modules to etherpad with mode 2775. Otherwise admin-UI plugin install EACCESes — those are the dirs LinkInstaller writes to. - systemd unit: add /opt/etherpad/src/node_modules to ReadWritePaths so symlink creation by the etherpad user is allowed under ProtectSystem=strict. plugin_packages is already covered via the symlink into /var/lib/etherpad. - postremove: clean up the new plugin_packages symlink on remove. - workflow: tag filters were `v[0-9]+.[0-9]+.[0-9]+`, but Actions tag filters are globs, not regex. `[0-9]+` matches one character, so multi-digit tags like v2.10.0 would never trigger. Switch to `v*.*.*` / `v*.*.*-*`, matching handleRelease.yml. - workflow smoke test now asserts plugin_packages symlink target, ownership of plugin_packages and node_modules. - test-local.sh: new script that builds the .deb and runs the same smoke test in a throwaway systemd-enabled Docker container, so failures are caught before pushing. - README: document test-local.sh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(packaging): test-local.sh — fix cgroups v2, add --no-systemd mode - systemd-in-docker on cgroups v2 needs --cgroupns=host and a writable /sys/fs/cgroup mount; the previous :ro version booted to nothing. - New --no-systemd mode: drops the systemd container in favour of plain ubuntu:24.04 + manual launch under the etherpad user. Validates the postinstall, wrapper, plugin paths, and /health without depending on the host's systemd-in-docker setup. Use it when --privileged systemd containers don't boot on your kernel/docker combo. - On systemd container exit the script now dumps the last 50 log lines and points at --no-systemd as the fallback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(packaging): test-local.sh — reuse cached image in --no-systemd If ubuntu:24.04 isn't on disk and the registry is unreachable, fall back to whichever ubuntu/debian image is already cached (e.g. the jrei/systemd-ubuntu image we pulled for the systemd path). Avoids a registry round-trip on flaky networks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: handle spawn errors in run_cmd; deb-package install order + offline-safe test src/node/utils/run_cmd.ts: Without `proc.on('error', ...)` a spawn failure (e.g. ENOENT for a missing binary) is emitted as an unlistened 'error' event, which Node treats as an uncaught exception that bypasses the awaiting try/catch and kills the process. The .deb hits this on first boot because plugins.ts spawns `pnpm --version` for a startup log line and pnpm isn't a runtime dep — Etherpad logs "Starting" then immediately stops. Reject the promise on 'error' so the existing try/catch in the caller actually catches it. packaging/scripts/postinstall.sh: chown /var/lib/etherpad/plugin_packages AFTER `cp -a` from the staged tree — `cp -a` preserves source (root) ownership and was re-rooting the directory we'd just chowned to etherpad. Same ordering the var symlink block already used. packaging/test-local.sh: Run `CI=1 pnpm install --frozen-lockfile` before staging so the package is built from a fresh, lockfile-consistent tree (matches CI). Fixes spurious "Cannot find module 'X'" failures from stale local symlinks pointing at out-of-date pnpm store paths. End-to-end test now passes: postinstall asserts pass, /health returns 200, dpkg --purge cleans up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: gitignore packaging build artefacts; drop accidental commit Drop packaging/etc/settings.json.dist that snuck into the previous commit (generated at build time by test-local.sh / CI from settings.json.template). Add /staging/, /dist/, /packaging/etc/ to .gitignore so they don't recur. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugins): downgrade missing-pnpm log from ERROR to debug The startup IIFE that logs the pnpm version is informational only. pnpm is a dev-only dependency: admin-UI plugin install goes through live-plugin-manager directly, and plugin migration is short-circuited when var/installed_plugins.json is present (e.g. on packaged installs). A missing pnpm on PATH is therefore expected on hardened deployments and shouldn't surface as a red ERROR in journalctl. Detect ENOENT specifically and log at debug; treat other errors (permission denied, etc.) as warnings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(packaging): smoke deb on PRs + backend test for run_cmd spawn errors CI gap: deb-package.yml only fired on v* tag pushes, so a PR that broke the .deb wasn't caught until release time. Wire it to PRs and develop pushes via a paths filter covering packaging files and the runtime files Etherpad needs at first boot. The release job already gates on `if: startsWith(github.ref, 'refs/tags/v')` so PR runs won't try to publish. Test gap: the run_cmd.ts spawn-error fix (commit |
||
|
John McLear
|
d619f03214
|
fix(settings): derive randomVersionString from release identity (#7563)
* fix(settings): derive randomVersionString from release identity Fixes #7213. Etherpad appends a `?v=<token>` cache-buster to static assets and embeds the same token as `clientVars.randomVersionString` in the padbootstrap JS bundle produced by specialpages.ts. Because esbuild's content-hash feeds back into the generated bundle filename (`padbootstrap-<hash>.min.js`), the token's value determines the file that clients are told to load. Historically the token was `randomString(4)`, regenerated on every boot. In a horizontally-scaled deployment (ingress → etherpad service → multiple pods) that meant every pod produced a different filename for the same built artifact. A client that loaded the HTML from pod A would request `padbootstrap-ABCD.min.js` from pod B and hit a 404 when the upstream balancer placed the follow-up request elsewhere. Derive the token deterministically so pods of the same build emit identical filenames, while still rotating on release so clients invalidate their cache correctly: ETHERPAD_VERSION_STRING env → verbatim (integrator override) else → sha256(version + "|" + gitVersion)[:8] Backwards-compatible: single-pod deployments see the same effective behavior (token rotates each release). Integrators who want to pin the token explicitly — e.g. tying it to their own deploy ID — can set `ETHERPAD_VERSION_STRING` in the environment. Test coverage added in src/tests/backend/specs/settings.ts: - Default shape is an 8-hex-char sha256 prefix. - ETHERPAD_VERSION_STRING override is respected verbatim. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(7213): call reloadSettings() to exercise ETHERPAD_VERSION_STRING The token is assigned inside reloadSettings, not parseSettings, so a parseSettings-only call never sees the env var. Drive reloadSettings directly, restoring the file paths and the prior token afterwards so other tests see a clean module state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
Etherpad Release Bot
|
dad6cc8eef | bump version | ||
|
John McLear
|
cd793294c4
|
fix(chat): icon click, disabled toggles, username layout (#7590, #7592, #7593) (#7597)
* fix(userlist): stop username input from overlapping the Log out button Fixes #7593. In the pad's Users popup, #myusernameform had no width set and the <input id="myusernameedit"> inside it took its natural content width, pushing past the Log out button and making the button overflow the popup at common widths. Constrain #myusernameform to 75px and make the input fill its container with box-sizing: border-box so the text field stays inside the form and the Log out button sits visibly next to it rather than getting covered or clipped off-screen. Low-risk, CSS-only change. No test plan beyond visual verification because the affected control is in the users popup UI. * fix(chat): bottom-align titlebar controls; restore chat icon click (#7590) Two regressions from the #7584 a11y refactor of the chat widget, both pure-CSS fixes scoped to the chat panel. 1. Title bar — `<a>` → `<button>` for #titlecross/#titlesticky kept the `float: right` layout, but a `<button>`'s box is only as tall as its glyph, so the small `−` and `█` controls floated at the *top* of the 44px title bar instead of sitting on the title's baseline as the anchors did. Switch #titlebar to a flex row with `align-items: flex-end`, give #titlelabel `flex: 1` to push the controls to the right edge, and use `order: 1/2` to keep the historical visual order `[█] [−]` (which `float: right` previously produced from reverse source order). 2. Chat-icon corner widget — `<div>` → `<button id="chaticon">` exposes the inner `<span class="buttonicon">` to the global `.buttonicon` rule's `display: flex; position: relative; align-items/justify-content: center;`. The existing override only reset `display`, leaving the span as a positioned flex item that, in some layouts, sat over the button's hit surface and swallowed clicks. Reset the remaining flex properties and add `pointer-events: none` so clicks always reach the `<button>`'s own click handler — preferred over weakening the global .buttonicon rule, which the toolbar relies on for icon centring. Visual-only / behaviour-fix, no markup or JS changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(settings): grey disabled chat option labels (#7592) When "Disable chat" is ticked in the Settings dialog, refreshMyViewControls() already sets `disabled` on `#options-stickychat` and `#options-chatandusers`, but the browser only greys the checkbox itself — the adjacent `<label>` keeps its normal colour, so the row still looks interactive even though clicks are no-ops. Add a popup-scoped rule that follows the existing convention used for disabled `.nice-select` controls (`color: #999; cursor: not-allowed`) so any disabled checkbox or radio in a settings popup matches its label to the disabled state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * revert(userlist): drop username input width cap (#7593 review) The width:75px on #myusernameform and width:100%/box-sizing on #myusernameedit from |
||
|
John McLear
|
83a42afbae
|
fix(export): /export/etherpad honors the :rev URL segment (#7566)
Fixes #5071. `/p/:pad/:rev/export/etherpad` has always ignored the rev parameter and returned the full pad history, unlike the txt/html export endpoints which use the same route but do respect rev. Users wanting to back up or inspect a snapshot of a pad at a specific rev got every later revision in the payload instead — both wasteful and a surprise when the downloaded .etherpad blob contained content that had supposedly been reverted. Change: - `exportEtherpad.getPadRaw(padId, readOnlyId, revNum?)` now takes an optional revNum. When supplied, it clamps to `min(revNum, pad.head)`, iterates only revs 0..effectiveHead, and ships a shallow-cloned pad object whose `head` and `atext` reflect the requested snapshot. The original live Pad is still passed to the `exportEtherpad` hook so plugin callbacks see the real document. - `ExportHandler` passes `req.params.rev` through on the `etherpad` type, matching the existing behavior of `txt` and `html`. - Chat history is intentionally left full (it is not rev-anchored). Adds three backend regression tests under `ExportEtherpad.ts`: - default (no revNum) still exports the full history - explicit revNum limits exported revs and rewrites the serialized head so re-import reconstructs the pad at that rev - revNum above head is treated as full history, preventing accidental truncation of short pads Out of scope: `getHTML(padID, rev)` on the API side is already honoring rev in current code (exportHtml.getPadHTML threads the parameter through), so the earlier report on that API call appears to be resolved. This PR does not touch it. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
51356b9a13
|
fix(editor): undo/redo scrolls the viewport to follow the caret (#7562)
* fix(editor): undo/redo scrolls the viewport to follow the caret Before: on a large pad, pressing Ctrl+Z (or Ctrl+Y, or the toolbar undo button) updated the caret in the rep model and the DOM, but the viewport did not follow when the caret landed below the visible area. The user was left looking at the same scroll position while their change had been undone somewhere they couldn't see. Root cause: scroll.ts's `caretIsBelowOfViewport` branch ran `outer.scrollTo(0, outer[0].innerHeight)` — a fixed offset equal to the inner iframe's height, NOT the caret position. That was a special-case added in PR #4639 to keep the caret visible when the user pressed Enter at the very end of the pad. It worked for that one scenario because the newly-appended `<div>` happened to be at the bottom of the pad too; for any other way of putting the caret below the viewport (undo, redo, programmatic selection change, deletion that collapsed a long block) it scrolled to an arbitrary spot. Fix: mirror the `caretIsAboveOfViewport` branch. After the deferred render settles, recompute the caret's position relative to the viewport and scroll by exactly the delta needed to bring the caret back in — plus the configured margin. The Enter-at-last-line case still works because the caret genuinely is near the bottom of the pad and the delta resolves to "scroll down by a screen". Closes #7007 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(7007): use real typing so undo has changesets to replay The first iteration of the Playwright spec built the pad by writing directly to #innerdocbody.innerHTML. That bypasses Etherpad's text layer, so the undo module had no changeset to revert — Ctrl+Z became a no-op and the scroll assertion saw no movement (CI failure output: `Expected: < 2302, Received: 2302`). Replace with real keyboard typing of 45 lines via the existing writeToPad-style pattern, then make the edit + scroll + Ctrl+Z under that real content. Slower (~5s per test) but faithful to how undo interacts with the pad. Also drop the `test.beforeEach(clearCookies)` scaffolding — it wasn't doing anything useful here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(7007): scroll caret into view directly in doUndoRedo Revert the scroll.ts rewrite from the previous commits and move the fix to the right abstraction layer: the undo/redo entry point itself. `scrollNodeVerticallyIntoView`'s caret-below-viewport branch has a well-documented special case (PR #4639) that scrolls to the inner iframe's innerHeight so Enter-on-last-line stays smooth. Changing that function for the undo case risked regressing the Enter case or racing with the existing scrollY bookkeeping. The CI run showed the rewrite wasn't actually producing viewport movement. Do the simpler thing instead: in `doUndoRedo`, after the selection is updated, call `Element.scrollIntoView({block: "center"})` on the caret's line node. That's browser-native, works inside the ace_inner / ace_outer iframe chain, doesn't need setTimeout, and matches what gedit/libreoffice do. Closes #7007 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
37aaeaf197
|
fix: page down/up scrolls by viewport height, not line count (#7479)
* fix: page down/up now scrolls by viewport height, not line count The previous implementation counted logical lines in the viewport, which failed when long wrapped lines consumed the entire viewport. Now scrolls by actual pixel height for correct behavior. Fixes #4562 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use outerDoc instead of outerWin.document for viewport height in PageDown/Up outerWin is an HTMLIFrameElement (returned by getElementsByName), not a Window object, so it has no .document property. The existing getInnerHeight() helper already uses outerDoc.documentElement.clientHeight correctly; align the PageDown/PageUp handler with that pattern. Adds a Playwright regression test that verifies PageDown scrolls the viewport when the pad contains long wrapping lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: rewrite page down/up to use pixel-based line counting The previous approach tried to scroll the outerWin iframe element directly which didn't work. Reverted to the original cursor-movement approach but calculates lines-to-skip using viewport pixel height divided by actual rendered line heights. This correctly handles long wrapped lines that consume multiple visual rows. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: restore getInnerHeight + inclusive range fixes lost in rebase Recover the PageDown/Up fixes that got dropped when this branch was rebased onto develop: - Use getInnerHeight() instead of outerDoc.documentElement.clientHeight so hidden-iframe and Opera edge cases are handled the same as the rest of the editor. - scroll.getVisibleLineRange() returns an inclusive end index, so count (end - start + 1) logical lines to match the pixel-sum loop bounds. - Replace the flaky 'PageDown scrolls viewport' test with the robust #4562 regression that builds long wrapped lines via direct DOM and asserts the caret advances on successive PageDown presses. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
John McLear
|
f219809a3d
|
fix(editbar): restore caret to pad after toolbar-select change (#7589) (#7598)
After picking a value from a toolbar <select> (ep_headings style picker is the canonical case), keyboard focus was left on the nice-select wrapper rather than returned to the pad editor. Users had to click back into the pad before typing resumed. The ToolbarItem.bind() class already calls padeditor.ace.focus() at the end of triggerCommand, but that only runs for selects wired via data-key on the wrapping <li>. Plugin-provided selects (e.g. ep_headings2's #heading-selection inside <li id="headings">, no data-key) don't go through that path — they bind their own change handler and never return focus. Fix: add a delegated change handler on `#editbar select` that calls padeditor.ace.focus() after any toolbar select change. Deferred via setTimeout(0) so plugin change handlers (bound on the same event) complete their ace.callWithAce work before focus moves. Redundant but harmless for data-key-wired selects that are already refocused by triggerCommand. Added a Playwright regression test that simulates the nice-select option-click (val + change, which is what the wrapper dispatches internally) and verifies typing after the change lands in the pad. Skips when ep_headings2 isn't installed. Closes #7589. |
||
|
John McLear
|
5fd600d608
|
fix(admin): restore i18n on /admin (issue #7586) (#7602)
* fix(admin): restore i18n on /admin by copying locales to the right path The admin SPA fetches `/admin/locales/<lang>.json`. Building with vite-plugin-static-copy and `src: '../src/locales'` was placing the 115 core locale files at `src/templates/admin/src/locales/` (the plugin's `dirClean` strips a leading `../` but keeps the remaining parent path). The express admin handler 404'd those fetches, fell back to serving `index.html`, JSON.parse silently failed, and every `<Trans>` rendered its raw key — see #7586. Replace the plugin with a small inline build/dev plugin: at build time copy `src/locales/*.json` to `<outDir>/locales/`; in dev serve the same files via middleware so `vite dev` also works. Drop the now-unused `vite-plugin-static-copy` dependency. Add regression coverage that none of the existing admin specs had: - backend HTTP test for GET /admin/locales/{en,de}.json - Playwright admin i18n spec asserting translated <h1> renders for the default locale and for ?lng=de, plus a request-level check that the response is JSON, not the SPA fallback. Closes #7586 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(admin): bundle locales via import.meta.glob, drop copy plugin The first pass at #7586 replaced vite-plugin-static-copy with a custom build/dev plugin that copied src/locales/*.json into the admin output and served them in dev. That works, but the vite-plugin-static-copy README explicitly recommends the public directory or a JS import for this case, and the import path is strictly cleaner: no copy step, no /admin/locales/* express route, no SPA-fallback-shaped failure mode. Use import.meta.glob in admin/src/localization/i18n.ts so each language ships as its own hashed JSON chunk and is lazy-loaded on demand. The vite config goes back to just react + base + outDir. The plugin namespaces (e.g. ep_admin_pads) keep their existing admin/public/<ns>/<lang>.json layout. Tests: - Drop tests/backend/specs/adminLocales.ts — it asserted on a /admin/locales/<lang>.json route that this approach no longer uses; the regression mechanism it pinned doesn't exist anymore and the test required the admin frontend to be built before the backend test runs (which CI doesn't do). - Keep tests/frontend-new/admin-spec/admini18n.spec.ts (rendered <h1> in default and ?lng=de). Verified red→green: reverting just the loader to the pre-fix /admin/locales fetch makes both specs fail; restoring makes them pass. Also update pnpm-lock.yaml to drop the now-unused vite-plugin-static-copy entries — fixes ERR_PNPM_OUTDATED_LOCKFILE that was failing every CI install upfront. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
dependabot[bot]
|
145e44a501
|
build(deps): bump mssql from 12.3.1 to 12.5.0 (#7595)
Bumps [mssql](https://github.com/tediousjs/node-mssql) from 12.3.1 to 12.5.0. - [Release notes](https://github.com/tediousjs/node-mssql/releases) - [Changelog](https://github.com/tediousjs/node-mssql/blob/master/CHANGELOG.txt) - [Commits](https://github.com/tediousjs/node-mssql/compare/v12.3.1...v12.5.0) --- updated-dependencies: - dependency-name: mssql dependency-version: 12.5.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
d189022f3b
|
build(deps): bump express-rate-limit from 8.3.2 to 8.4.0 (#7596)
Bumps [express-rate-limit](https://github.com/express-rate-limit/express-rate-limit) from 8.3.2 to 8.4.0. - [Release notes](https://github.com/express-rate-limit/express-rate-limit/releases) - [Commits](https://github.com/express-rate-limit/express-rate-limit/compare/v8.3.2...v8.4.0) --- updated-dependencies: - dependency-name: express-rate-limit dependency-version: 8.4.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
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>
|
||
|
Etherpad Release Bot
|
249241d9a4 | bump version | ||
|
translatewiki.net
|
cea0cdaea4
|
Localisation updates from https://translatewiki.net. | ||
|
John McLear
|
a1bb958b18
|
fix(pad): stop hardcoding lang='en', let the client auto-detect locale (#7586) (#7588)
`Pad.normalizePadSettings()` was defaulting `lang` to the literal string
'en' when `rawPadSettings.lang` was not a string. That value flowed into
`clientVars.padOptions.lang` and then into `getParams()` in pad.ts,
which calls `html10n.localize([serverValue, 'en'])` as a callback for
the `lang` setting. The result: every pad forced English on load,
overriding the browser's Accept-Language and the existing auto-detect
chain in l10n.ts (cookie -> navigator.language -> 'en').
The regression was introduced in #7545 ("Add creator-owned pad settings
defaults", commit
|
||
|
John McLear
|
9e352ca4f3
|
fix(clientVars): stop mutating the shared plugin registry during sanitization (#7587)
PadMessageHandler built the `pluginsSanitized` payload for clientVars by
aliasing `plugins.plugins` and then mutating each entry's `package` field
in place:
let pluginsSanitized: any = plugins.plugins;
Object.keys(plugins.plugins).forEach(function(element) {
const p: any = plugins.plugins[element].package;
pluginsSanitized[element].package = {name: p.name, version: p.version};
});
Because `pluginsSanitized` is a reference to `plugins.plugins`, the
assignment clobbered the server-side plugin registry. After the first
pad connection, every plugin's `package` object held only `{name,
version}` — `realPath`, `path`, and `location` were gone.
Minify.ts resolves `/static/plugins/ep_*/...` URLs via
`plugin.package.realPath`. Once the field disappeared, every subsequent
static asset request for a bundled plugin 500'd with:
TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of
type string. Received undefined
at Object.join (node:path:1354:7)
at _minify (src/node/utils/Minify.ts:181:23)
Symptoms on Chromium: plugin CSS/JS assets fail to load (e.g.
/static/plugins/ep_font_size/static/css/size.css returns 500), so
plugins partially render or don't work at all. Firefox swallows the
resulting console errors quietly.
Fix: extract the sanitization into a pure helper `sanitizePluginsForWire`
that returns a fresh object graph and never touches the input. The
helper is covered by a new backend spec that:
* verifies the sanitized output has only {name, version} in `package`
* asserts the input registry's realPath/path/location survive the call
* runs the call repeatedly and confirms non-destructiveness
* mutates the returned copy and asserts the input is independent
Verified live with the dev server: before the fix, `/static/plugins/
ep_font_size/static/css/size.css` 500'd after visiting any pad; after
the fix it returns 200 both before and after pad connections.
|
||
|
John McLear
|
67e542d2b9
|
fix(editor): preserve U+00A0 non-breaking space (#3037) (#7585)
* fix(editor): preserve U+00A0 non-breaking space (#3037) Non-breaking spaces were silently normalized to regular spaces at every ingestion point, so typed/pasted/imported nbsps never reached the changeset and users could not glue words against line-wrap in French or other languages that require nbsp typography. Removed the four strip sites that replaced U+00A0 with U+0020: - src/node/db/Pad.ts cleanText - src/static/js/contentcollector.ts textify - src/static/js/ace2_inner.ts textify - src/static/js/ace2_inner.ts importText raw-text guard Updated both processSpaces functions (domline and ExportHtml) to tokenize U+00A0 as a separate unit, emit it verbatim as , and treat it as content (not whitespace) for the run-collapse bookkeeping so adjacent regular-space runs aren't miscounted. Added backend round-trip tests for spliceText and setText, and extended the cleanText case table. Updated the existing contentcollector and importexport specs whose expectations encoded the previous buggy behavior; they now assert genuine nbsp preservation. Verified manually in Firefox: clipboard U+00A0 → paste → pad → getText returns c2 a0; getHTML emits `100 km`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(contentcollector): collapse display-artifact nbsp runs on DOM read-back processSpaces is a lossy one-way display transform: leading/trailing spaces and all-but-the-last of a run get rendered as so HTML doesn't collapse them. When incorporateUserChanges reads text back from the DOM, those display-artifact nbsps were being stored in the changeset model instead of being normalized back to plain spaces. This broke handleReturnIndentation, whose /^ *(?:)/ regex only matches ASCII spaces: auto-indent after `foo:\n` produced 4 spaces instead of the expected prev-indent (2) + THE_TAB (4) = 6, because the previous line's model had nbsps where it used to have spaces. Fix: in contentcollector.textify, collapse any [ ]+ run back to plain spaces UNLESS the run is pure U+00A0 AND strictly interior to word chars. That preserves user-intended typographic nbsps like "100 km" while undoing the one-way display transform. Updated 7 contentcollector tests and 7 importexport tests whose assertions needed to reflect the new rule (boundary/mixed runs collapse; pure-interior nbsp runs preserve). Fixes the Playwright regression in indentation.spec.ts:117 that the previous commit introduced. * fix(contentcollector): canonicalize nbsp runs at line assembly, not per text node Addresses Qodo code review feedback on PR #7585. ## Bug fix — nbsp lost at DOM text-node boundary The previous approach ran the "collapse display-artifact nbsp" rule inside textify(), which is called per individual DOM TEXT_NODE. A user-intended nbsp sitting at a text-node boundary (e.g., <span>100</span><span> km </span>) was incorrectly seen as non-interior (before === '' for the second text node) and normalized back to a regular space. Fix: move the canonicalization out of textify() and run it on each fully assembled line string inside cc.finish(). The rule remains: [ ]+ run -> plain spaces UNLESS pure U+00A0 AND strictly interior to non-ws chars It is length-preserving, so attribute offsets and line lengths are unaffected. Added a regression test (contentcollector.spec.ts) for the cross-span case. ## Docs concern Reverted the type-only addition of spliceText to PadType. spliceText is an existing Pad runtime method; the backend test now uses a cast (`(pad as any).spliceText`) so the PR does not expand the declared public type surface, avoiding a separate documentation requirement. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |