etherpad-lite

mirror of https://github.com/ether/etherpad-lite.git synced 2026-07-18 00:57:55 +00:00

Author	SHA1	Message	Date
John McLear	b19ad89eb0	Fix offline/air-gapped Docker boot: stop pnpm self-provisioning the pinned version (#7911 ) (#7918 ) * fix(docker): don't let pnpm self-provision a pinned version on offline boot (#7911) The official Docker image installs pnpm directly via npm (corepack was dropped for Node 25+). Standalone pnpm still honours the "packageManager" pin in package.json: the image's pnpm intentionally lags that pin (pnpm 11.1.x enforces a minimum-release-age policy the frozen-lockfile build can't satisfy), so pnpm treats every invocation — including the informational `pnpm --version` probe Etherpad runs at startup — as a request to download and run the pinned build. Behind a corporate firewall / in an air-gapped install that download fails: [WARN] plugins - Failed to get pnpm version: Error: Command exited with code 1: pnpm --version which is what #7911 reported. Fix — neutralise the gap instead of closing it (closing it would break the frozen-lockfile build on 11.1.x): - Dockerfile build stage sets `pnpm_config_pm_on_fail=ignore` (the pnpm 11 successor to managePackageManagerVersions), inherited by the development and production runtime stages. pnpm then uses the installed pnpm instead of fetching the pinned one. It does not change which pnpm runs the build-time install, so the frozen-lockfile build is unaffected. - plugins.ts startup probe and the updater's pnpm-on-PATH checks run with the same flag, so the fix also covers non-Docker offline installs and the probe can never fail-loud. Add a backend spec that fails CI if the offline guard is dropped while the image pnpm differs from the package.json pin. Verified with a standalone (non-corepack) pnpm: a "packageManager" mismatch makes `pnpm --version` exit 1 by default (tries to fetch the pinned build), and exit 0 reading the local version with pm_on_fail=ignore. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: scope pnpm offline-guard check to the runtime-inherited build stage Address Qodo review: the regression spec matched ENV pnpm_config_pm_on_fail anywhere in the Dockerfile, so it would still pass if the guard were removed from the `build` stage (which the runtime stages inherit) but left in the throwaway `adminbuild` stage — reintroducing the offline failure. Extract the `build` stage block and assert the ENV is present there specifically. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-09 09:31:00 +01:00
John McLear	ef205bf2ac	fix(theme-color): dark address bar + no light-flash on dark-OS load (#7606 ) (#7909 ) * fix(theme-color): emit media-scoped dark variant for iOS Safari (#7606) The theme-color meta only had a single light value rendered server-side; dark mode was applied purely by JS (skin_variants.ts) after page load. iOS Safari colors the address bar at parse time and does not reliably repaint when JS mutates the meta later, so dark-mode iPhone users kept a white address bar above a dark toolbar (the green Chromium Playwright test masked this because Chrome does honor the dynamic update). Emit a prefers-color-scheme media-scoped pair server-side so the correct color is chosen at first paint without JS: <meta name="theme-color" content="#ffffff" media="(prefers-color-scheme: light)"> <meta name="theme-color" content="#485365" media="(prefers-color-scheme: dark)"> - Add SkinColors.darkToolbarColor() (reuses toolbarColorForTokens). - Expose enableDarkMode via getPublicSettings so the templates can gate the dark variant on it (no dark variant when dark mode can't be reached). - Apply to both pad.html and timeslider.html. - updateThemeColorMeta now updates every theme-color meta so a manual #options-darkmode toggle still wins over the media scoping on desktop/Android. - Backend + frontend tests updated to assert the media-scoped pair and the enableDarkMode-off case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(theme-color): prevent the light-mode flash on dark-OS load (#7606) The theme-color meta fix corrects the address-bar tint, but dark-OS users still saw the whole page painted light before the JS bundle ran and applied the dark skin classes in postAceInit — a visible flash on every browser, not just the mobile address bar. Add a tiny blocking inline script in <head>, before the stylesheet, that applies the dark skin classes to <html> synchronously during parse when the client is in dark mode (matchMedia + no localStorage white-mode override). The condition mirrors pad.ts's auto-switch, which still runs on init to wire up the #options-darkmode toggle and theme the editor iframes (those don't exist yet at parse time). Gated on the same enableDarkMode + colibris check as the dark theme-color variant. Applied to pad.html and timeslider.html. Verified in Chromium: at domcontentloaded a dark-OS client's <html> already carries super-dark-editor/dark-background/super-dark-toolbar (no flash), and a light-OS client is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(changelog): note the dark-mode address-bar + flash fix (#7606) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(theme-color): guard pre-paint script on #skinvariantsbuilder; ignore updater state Address PR review: - Copilot: the inline pre-paint dark-mode script must skip the auto-dark switch on the #skinvariantsbuilder hash, matching pad.ts — otherwise it forces super-dark classes on a dark-OS client and fights the variants builder UI. Added the guard to pad.html and timeslider.html and a backend assertion so it can't regress. - Qodo: ignore var/update-state.json (runtime updater cache) so the server run that regenerates it can't dirty the tree or be committed accidentally. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-08 21:54:08 +02:00
John McLear	78943da92b	fix(editor): tag inserts with clientVars author until userAuthor propagates (Firefox authorship flake) (#7910 ) * fix(editor): tag inserts with clientVars author until userAuthor propagates Fixes the intermittent Firefox `clear_authorship_color` flake ("clear authorship colors can be undone to restore author colors"). Root cause: the inner editor's `thisAuthor` starts '' and is only populated when collab_client's `setProperty('userAuthor', userId)` reaches the inner frame — that call is queued by the outer ace wrapper via pendingInit until the iframe loads, so it is applied asynchronously. Under Firefox timing the first keystrokes can beat it, so freshly typed text is tagged author=''. An empty author canonicalizes to no-author, producing an unattributed insert (`Z:1>5+5$Hello`) that the server's pad-corruption guard rejects ("submitted an insert without an author attribute"), dropping the whole USER_CHANGES and losing authorship. Undo then cannot restore an author color and the test sees `<span class="">`. Fix: a `getLocalAuthor()` helper that falls back to `clientVars.userId` (the same author id, available synchronously in the inner frame — already used as `myId` elsewhere) whenever `thisAuthor` is still empty, applied at the three new-text insert sites. The intentional clear-authorship path (`['author','']`) and the server-side guard are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(editor): also seed line-attribute author to close the same race The first commit fixed text inserts, but line-attribute changes (lists, headings, alignment) emit a line-marker insert tagged with AttributeManager.author, which likewise starts '' and is only set when the async setProperty('userAuthor') lands. An early list/heading could therefore emit an unattributed line-marker insert and hit the same server-side rejection. Seed documentAttributeManager.author from getLocalAuthor() (clientVars fallback) right after it is constructed; the userauthor handler keeps it in sync after. Audited the other author-tagging paths: all ace2_inner text-insert sites go through getLocalAuthor()/authorizer now, and contentcollector (paste/import) derives authorship from the pasted DOM's author- classes via className2Author — a different mechanism, not this async race — and self-guards on `if (state.author)`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(editor): read the local author from the TOP pad window, not the inner frame v1 of this fix fell back to `window.clientVars?.userId`, but that did NOT resolve the flake — CI reproduced the identical failure and the server log still showed the unattributed-insert rejection. Ground truth via direct DOM measurement of a running pad: the inner editor iframe's `window.clientVars` is NEVER populated (undefined at t=0/1/3/6s for the life of the pad), so the v1 fallback always returned ''. The author id lives on the TOP pad window (`window.top.clientVars.userId`), set by pad.ts when the CLIENT_VARS message arrives — which necessarily precedes editor creation, so it is reliably available from the inner frame the moment the editor can accept input. getLocalAuthor() now reads window.top.clientVars.userId (guarded with try/catch for cross-origin embedded pads). thisAuthor still takes precedence once the queued setProperty('userAuthor') lands. All text-insert sites and the line-attribute seed already route through getLocalAuthor(), so both paths are covered. Verification: ts-check passes; fix source confirmed by direct measurement (the top window reliably holds the author at editor-init time). Local end-to-end repro was not possible — the dev server's require-kernel bundle did not reflect working-tree edits — so this is validated against the measured source + CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(editor): walk ancestor frames for the author instead of window.top Addresses a correctness gap flagged in review: reading window.top.clientVars fails for same-origin embeds, where window.top is the host page (accessible, so no exception is thrown) and has no Etherpad clientVars — getLocalAuthor() then returns '' and the unattributed-insert bug recurs. Walk up the ancestor chain (inner -> ace_outer -> pad window) and return the first frame that has clientVars.userId. This stops at the pad window and never depends on window.top, so it is correct for normal pads, same-origin embeds, and cross-origin embeds (the try/catch ends the walk if an ancestor is cross-origin, though the pad window — always same-origin with the editor frames — is reached first). Behavior in the non-embed case is unchanged (pad window is 2 hops up). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(collab): stamp the author onto outgoing inserts in collab_client Replaces the earlier getLocalAuthor attempts (v1-v3, reverted from ace2_inner.ts), which all failed because they tried to read an author that genuinely does not exist yet: the local author id (clientVars.userId) only arrives with the CLIENT_VARS socket message, and under load (Firefox + plugins) the editor can become editable and the user can type before that message lands. At that instant there is no author anywhere client-side, so the editor emits an unattributed insert that the server's pad-corruption guard rejects — dropping the change and losing authorship (the clear_authorship_color flake). Fix where the author IS reliably available: collab_client only exists after CLIENT_VARS has arrived, so its `userId` is always populated. A new pure helper stampAuthorOnInserts() rewrites the outgoing wire changeset so every '+' op carries an author, stamping userId onto any that lack one, just before it goes on the wire (both the USER_CHANGES send and the reconnect/further-changeset path). Independent of editor-init timing. Verification: - Unit test (stampAuthorOnInserts.test.ts, 5 cases): stamps the exact flake changeset `Z:1>5+5$Hello` -> authored + checkRep-valid + text preserved; leaves already-attributed inserts and keep/remove-only changesets unchanged; never invents an author when userId is empty; preserves other attributes. - ace2_inner.ts reverted to develop (the editor is unchanged; the fix is fully localized to the send path). - Local run of the clear_authorship_color spec with the fix: all pass, no server-side unattributed-insert rejections; full backend-new vitest green (the unrelated backend-tests-glob env-only failure aside). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-08 20:41:13 +01:00
John McLear	7ea9970648	Hardening: API request handling, random IDs, and plugin loading (#7906 ) * Hardening: API request handling, token generation, and plugin loading - pad_utils.randomString: generate the random IDs via crypto.getRandomValues (CSPRNG) instead of Math.random. - OAuth2Provider: constant-time password comparison and a uniform failure delay on the OIDC interaction login; own-property user lookup. - API.appendChatMessage: require the pad to already exist (getPadSafe), consistent with the other content API methods. - RestAPI /api/2: forward only the authorization header rather than merging all request headers into the API field set. - LinkInstaller: validate plugin dependency names before building filesystem paths from them. - admin file server: return a generic error message and log details server-side. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review: timingSafeEqual on raw bytes; align /api/2 auth fallback - OAuth2Provider.constantTimeEquals: compare raw UTF-8 bytes with crypto.timingSafeEqual instead of hashing them first. Resolves the CodeQL "password hash with insufficient computational effort" alert while keeping a content-independent comparison (length difference is covered by the uniform failure delay). - RestAPI /api/2: fall back to the authorization header whenever the field is falsy (not only null), matching the openapi.ts handler so the two routers authenticate identically (Qodo). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * appendChatMessage: throw explicit error instead of getPadSafe (review) Per review: replace the getPadSafe(padID, true) existence check with an explicit `throw new CustomError('padID does not exist', 'apierror')` so chat messages can't create pads, without fetching the pad. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-07 19:17:39 +02:00
John McLear	e1476cf100	fix(history-mode): lay the history iframe in the editor's flex slot (#7903 ) In-pad history mode positioned the timeslider iframe (#history-frame-mount) as an absolute, inset:0 overlay over #editorcontainerbox. That filled the editor area but took the iframe out of flow, so any in-flow side panel in #editorcontainerbox — e.g. ep_webrtc's video column (#rtcbox) — ended up hidden beneath the overlay. In live mode the same panel sits beside the editor, so the two modes disagreed (ether/ep_webrtc rendered nothing in the timeslider). Make the iframe occupy the same in-flow flex slot the live editor uses (flex: 1 1 auto in the row-flex #editorcontainerbox) so side panels lay out identically in both modes. This surfaced a latent bug: `body.history-mode #editorcontainer { display: none }` (specificity 0-1-1) was being outranked by the two-id layout rule `#editorcontainerbox #editorcontainer { display: flex }` (0-2-0), so the live editor was never actually removed from flow — the absolute overlay merely painted over it. With the iframe now in normal flow that no longer hides the editor, so the hide rule is given matching specificity (body.history-mode #editorcontainerbox #editorcontainer) to win the cascade. Added a padmode.spec.ts regression test that injects a left-pinned in-flow side panel, enters history mode, and asserts the live editor is display:none, the iframe is not absolutely positioned, and the iframe lays out beside the panel filling the editor's remaining width (no overlap). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-05 15:49:45 +01:00
John McLear	189ea4a29b	fix: URL-encode pad names in admin 'Open' button and recent pads (#7865 ) (#7895 ) * fix: URL-encode pad names in admin 'Open' button and recent pads (#7865) - encodeURIComponent in admin PadPage 'Open' button href - decodeURIComponent when reading pad name from URL pathname in pad_userlist.ts and colibris/pad.js (recent pads storage) - encodeURIComponent in colibris/index.js recent pads href; display text uses stored name directly (no double-decode) - add recent_pads spec asserting encoded URLs - add share dialog spec asserting URL encoding of special chars * fix: address Qodo review on recent-pads encoding and admin Open button - Normalize legacy URL-encoded recentPads names before re-encoding the href in colibris/index.js, preventing double-encoding (%2F -> %252F) of entries stored by older versions. - Add noopener,noreferrer to the admin 'Open' window.open call to prevent reverse tabnabbing, matching the pattern used elsewhere. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-05 14:02:35 +01:00
John McLear	7b33892a76	css: restore URL wrapping in pad editor (#7894 ) (#7896 ) * css: restore URL wrapping in pad editor (#7894) Long URLs in the pad editor overflow instead of wrapping because the global a { white-space: nowrap } rule in pad.css overrides the wrapping properties set on #innerdocbody. Add explicit white-space, word-wrap, and overflow-wrap to #innerdocbody a so URLs wrap inside the editor while preserving no-wrap behavior for links elsewhere in the UI. Fixes #7894 * test: add regression test for long URL wrapping in pad editor (#7894) * test: add regression test for long URL wrapping in pad editor (#7894)	2026-06-05 13:44:15 +01:00
John McLear	9e8f9cb513	fix: pad-wide view settings apply to the creator's own view (#7900 ) (#7902 ) * fix: pad-wide view settings apply to the creator's own view (#7900) The pad creator is never "enforced upon themselves" (isPadSettingsEnforcedForMe is false whenever canEditPadSettings is true), so getEffectivePadOptions always merges their personal view overrides (cookies) on top of the pad-wide options. As a result, a creator who had at some point toggled e.g. their personal "Read content from right to left" carried a stale rtlIsTrue=false cookie that silently masked the pad-wide value they later set — toggling the pad-wide control (and then enforcing it) appeared to do nothing on the creator's own screen. Fix: when the creator changes a pad-wide view option, sync their personal pref to the chosen value so their own view adopts the pad-wide setting immediately. This deliberately does NOT change the precedence model — the creator can still override the setting afterwards via the "My view" controls, so the existing behaviour where a creator keeps personal overrides while enforcing settings for other users (pad_settings.spec.ts) is preserved. Adds a frontend test reproducing the stale-personal-cookie scenario. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: address Qodo review — clear cookies on the page's own context (#7900) browser.newContext()+clearCookies() cleared cookies on a throwaway context (not the page fixture under test) and leaked the context. Use page.context().clearCookies() so the regression test reliably starts without a stale rtlIsTrue pref. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-05 13:41:13 +01:00
John McLear	9af77e7e3a	timeslider: respect showAuthorshipColors and padFontFamily settings (#7899 ) * timeslider: respect author colors, font, line numbers from pad editor - nice-select.ts: dispatch native change event after jQuery trigger so addEventListener-based bridges (pad_mode.ts) fire (jQuery 3.7.1 trigger() does not dispatch native DOM events) - timeslider.ts: fix font-family reset (jQuery 3 ignores null css value); add showAuthorColors + showLineNumbers + padFontFamily support; wire cookie read/write for all three settings - broadcast.ts: gate author color CSS on .authorColors class - pad_mode.ts: bridge author colors, font family, line numbers checkboxes from pad settings into embedded timeslider iframe - add Playwright test for showAuthorshipColors * timeslider: tidy settings-bridge wiring Refactor only — no behaviour change. pad_mode.ts: collapse the five ad-hoc listener stores (playbackChange- Listener, followChangeListener, authorColorsListeners, fontFamily- Listeners, lineNumbersListeners) into the single outerControlListeners list via a shared bindOuter() helper, so every outer-control listener is registered and torn down through one uniform path. The three view-setting bridges (author colours, font family, line numbers) become one data-driven bridgeView() over their element ids instead of three near-identical closures. timeslider.ts: hoist applyPadFontFamily to a top-level helper next to applyShowAuthorColors (keeping the '' reset for jQuery 3), and co-locate the three BroadcastSlider view-setting methods that were split across the function. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-05 13:39:41 +01:00
John McLear	61aee16cf6	fix: RTL content option no longer flips the whole page (#7900 ) (#7901 ) * fix: RTL content option no longer flips the whole page (#7900) The pad's RTL content option (rtlIsTrue) is a per-pad setting that should only change the direction of the editor's text contents. The setter in ace2_inner.ts wrote the direction to the top-level `document.documentElement` (`document` there resolves to the main pad page, since the editor iframes are derived from it), which flipped the entire page — toolbar and chrome included. The page direction is owned solely by the UI language (l10n.ts sets it from html10n.getDirection()). Apply the content direction to the inner editor document (`targetDoc.documentElement`) instead, so enabling RTL content leaves the surrounding interface untouched. Adds a frontend test asserting the inner editor flips to RTL while the top-level <html> dir stays ltr. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: address Qodo review — robust frame access and locale-independent page-dir assertion (#7900) - Use a frameLocator chain (Playwright auto-waits) instead of page.frame('ace_inner')! which can race with inner-editor readiness. - Don't hardcode dir='ltr' on the top-level <html> (the UI language can legitimately pick rtl). Capture the initial page dir and assert it is unchanged after toggling the pad RTL option — that is the real invariant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-05 13:24:26 +01:00
John McLear	bfaf442935	fix(pad): URL view-option params lost to padeditor.init race (#7840 ) (#7843 ) * fix(pad): URL view-option params lost to padeditor.init race (#7840) `?showLineNumbers=false` and `?useMonospaceFont=true` were being silently clobbered shortly after pad load. Same race that affected `?rtl=false` before #7464: 1. _afterHandshake → getParams() sets settings.LineNumbersDisabled (or useMonospaceFontGlobal, noColors). 2. _afterHandshake calls padeditor.init(view).then(postAceInit) — async; ace iframes still loading. 3. Sync tail of _afterHandshake hits the URL-param overrides and calls changeViewOption('showLineNumbers', false) etc. These queue setProperty('showslinenumbers', false) in Ace2Editor's actionsPendingInit queue (loaded=false). 4. ace.init resolves → loaded=true → queue flushes → URL-driven value applied. 5. padeditor.init resumes past its own await and calls setViewOptions(initialViewOptions) — initialViewOptions is built from clientVars.initialOptions.view (server defaults ∨ cookie), which does NOT carry the URL preference. The resulting setProperty('showslinenumbers', true) runs against loaded=true ace and immediately re-shows the gutter. #7464 noticed this race for RTL and moved the override into postAceInit. The neighbouring blocks for showLineNumbers / noColors / useMonospaceFontGlobal were left at the synchronous-tail site — generalise the same fix to all three. Direct-browser users typically had a `prefs` cookie with showLineNumbers=false from a prior in-pad toggle, so the initialViewOptions value happened to match the URL param and the race was unobservable. Cross-context iframe embeds (the reporter's configuration) start with no cookie, so the server default true fights the URL false and the race becomes visible. Adds src/tests/frontend-new/specs/url_view_options.spec.ts covering the showLineNumbers=false / =true / useMonospaceFont=true cases on initial-load navigation (the path where the race actually fires). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(types): annotate navigateWithParam helper params Fixes CI ts-check: parameters page/padId/param implicitly had `any`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-25 16:10:12 +02:00
John McLear	29dac6bfcc	fix(pad): redesign outdated-version notice (#7799 ) (#7804 ) * docs: design spec for #7799 outdated-notice redesign Per-pad first-author gating, dismissable gritter, minor-or-more rule, drop vulnerable UI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: implementation plan for #7799 outdated-notice redesign 12 bite-sized tasks, TDD-first where applicable; closes the spec end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): add isMinorOrMoreBehind, drop major/vulnerable helpers Adds isMinorOrMoreBehind(current, latest) which returns true only when the latest release is at least one minor version ahead (patch-only deltas return false). Removes isMajorBehind, parseVulnerableBelow, and isVulnerable from versionCompare.ts — callers in updateStatus.ts, VersionChecker.ts, and index.ts will be updated in subsequent tasks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(updater): drop vulnerable-below directive and state field Remove VulnerableBelowDirective type, UpdateState.vulnerableBelow field, and all related scraping/checking logic (parseVulnerableBelow, isVulnerable imports). Clean up Notifier, OpenAPI schema, and all test fixtures to match. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(updater): drop residual EmailSendLog vulnerable fields Remove `vulnerableAt` and `vulnerableNewReleaseTag` from the `EmailSendLog` interface, `EMPTY_STATE`, and the `isValidEmail` validator — these backed the removed `vulnerable`/`vulnerable-new-release` email kinds and are now dead code. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(updater): add firstAuthorOf helper Export firstAuthorOf() from updateStatus.ts — finds the lowest-numbered author attrib in a pad's pool, skipping empty-string placeholders. Covered by 6 vitest cases in tests/backend-new/specs/hooks/express/. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(updater): add resolveRequestAuthor helper for HTTP GET Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(updater): pad-aware /api/version-status with first-author gating Replace global badge cache with a per-(padId, authorId) LRU cache. The new response shape is {outdated: 'minor' \| null, isFirstAuthor: boolean}; the old 'severe'/'vulnerable' enum is dropped entirely. computeOutdated now resolves the pad's first author and compares it against the session author before returning outdated:'minor', so the notice is only shown to the person who created the pad. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(updater): switch isSevere signal from major-only to minor-or-more behind Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(updater): end-to-end coverage for /api/version-status Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(openapi): /api/version-status pad-aware shape and gating Add the /api/version-status GET operation to the admin OpenAPI spec with the new pad-aware response shape: outdated enum reduced to [minor]\|null, isFirstAuthor boolean, and an optional padId query param. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(pad): remove unused #version-badge template and CSS * feat(pad): replace persistent badge with first-author outdated gritter Renames pad_version_badge.ts → pad_outdated_notice.ts and rewrites it as a fire-and-forget gritter notice that only shows when the API reports outdated=minor AND the current user is the pad's first author. Wires the new maybeShowOutdatedNotice() call into pad.ts immediately after showPrivacyBannerIfEnabled(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(pad): playwright coverage for outdated notice gritter Six Playwright specs exercise maybeShowOutdatedNotice: null response, isFirstAuthor:false guard, positive appearance + text, X-dismiss, 500 server error tolerance, and 8 s auto-fade. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(pad): outdated-notice redesign + drop vulnerable-below docs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(test): remove stale specs for deleted #version-badge surface Delete the GET /api/version-status describe block from the legacy mocha spec (asserted outdated:null and outdated:'severe' — both no longer match the new response shape). The new vitest spec at tests/backend-new/specs/hooks/express/updateStatus.test.ts covers this surface comprehensively. Delete src/tests/frontend-new/specs/pad-version-badge.spec.ts entirely: all three tests reference the #version-badge DOM element removed in Task 8 and stub 'severe'/'vulnerable' enum values that no longer exist. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: clean stale references to vulnerable/severe in types, emails, docs - Remove OutdatedLevel type (null\|'severe') from types.ts — no consumers remain after the badge redesign removed the severe tier. - Fix Notifier severe-email body: was "more than one major release behind" but isSevere now fires on minor-or-more, so update to "at least one minor release behind the latest published version". - Drop "vulnerability directives" from the /admin/update/status OpenAPI description; replace with the actual response fields. - Remove stale vulnerableBelow field from UpdateStatusPayload in admin/src/store/store.ts — server no longer sends it. - Fix docs/admin/updates.md: "pad-side badge" → "pad-side notice". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-18 12:23:40 +01:00
John McLear	acc7a5dd4f	fix(l10n): silence spurious 'could not translate element content' warning on form controls (#7797 ) * fix(l10n): short-circuit form controls in translateNode (no spurious warning) `<select data-l10n-id="...">` with `<option>` element children — the pattern used by ep_headings2, ep_align, ep_font_size, ep_font_family, … — used to drop into the textContent branch of html10n.translateNode and hunt for a text-node child to overwrite. There is none, so the loop exited with `found = false` and emitted: Unexpected error: could not translate element content for key ep_headings.style The SELECT/INPUT/TEXTAREA aria-label fallback already lived inside the same else-branch, after the warning, so the accessible name landed correctly but the noisy console line still fired on every pad load. Move the form-control case into its own `else if`, before the text-node hunt: aria-label is the only sensible localization target for these elements (a <select>'s text is its <option> labels, not its own name). Closes the console warning reported on Etherpad 3.1.0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): actually exercise the form-control short-circuit branch Qodo's review on #7797 caught two real test bugs: 1. `pad.toolbar.bold.title` ends in `.title`, which is in html10n's attribute allowlist. translateNode picks `prop = 'title'`, takes the first branch (node[prop] = str.str), and never reaches the textContent path where the warning lives. The test would pass without my fix. Switched the key to `pad.loading` — same stable pad-bundle translation, but the suffix isn't in the allowlist, so `prop` defaults to `textContent` and the test actually exercises the regression path. 2. `page.waitForTimeout(50)` is a fixed sleep, but `html10n.localize` runs its work inside a `build()` callback, so completion timing depends on the loader. Replaced with a deterministic `html10n.mt .bind('localized', …)` await. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-17 19:59:00 +01:00
John McLear	a4261c2895	fix(a11y): drop role=textbox / aria-multiline from innerdocbody (#7778 ) (#7782 ) Murphy's 2026-05-16 re-test of #7255 reported "you still can't cycle through the text properly line by line to press links and such". The narrower toolbar/measurement fixes in #7777 don't address this — it's caused by the editor body advertising textbox semantics. role="textbox" + aria-multiline="true" pin NVDA/JAWS into focus mode for the whole pad. In focus mode arrow keys move the caret one character at a time, the P/H/K rotor shortcuts are suppressed, and links don't surface in the links list. That matches Murphy's symptoms exactly. contenteditable="true" by itself is enough to tell AT this is editable. Without the textbox role, NVDA/JAWS browse the content as document-mode HTML — line-by-line arrow nav, headings rotor, links list all return. aria-label / aria-describedby stay so the pad is still announced as "Pad content" with the keyboard hint on focus. This is the lighter alternative to the AT-only read mirror originally sketched in #7778 — ARIA-only, no DOM restructuring, no plugin impact. Refs #7255 #7777	2026-05-16 18:35:32 +01:00
John McLear	278acb10cb	Drop swagger-ui, document telemetry, add opt-outs (#7524 ) (#7757 ) * docs: design spec for #7524 drop swagger-ui + privacy opt-outs Three-deliverable plan: vendor RapiDoc to replace swagger-ui-express (Scarf-injecting), add privacy.updateCheck and privacy.pluginCatalog opt-outs for our two outbound calls, and ship PRIVACY.md as a public stance doc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: implementation plan for #7524 swagger-ui + privacy opt-outs Twelve TDD-flavoured tasks: privacy settings shape, UpdateCheck + installer opt-outs (each with a failing-test-first cycle), admin backend/UI plumbing, dependency drop, vendored RapiDoc, PRIVACY.md, final verification matrix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(privacy): add privacy block to settings shape Adds privacy.updateCheck and privacy.pluginCatalog, both defaulting to true so behavior is unchanged until operators opt out. Refs #7524 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(privacy): honour privacy.updateCheck=false in UpdateCheck check() and getLatestVersion() now early-return when the setting is off. Logs once on first skip. The admin "update available" panel already tolerates an undefined latestVersion. Refs #7524 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(privacy): honour privacy.pluginCatalog=false in installer Extracts the gate into pluginCatalogGuard.ts so it can be unit-tested under vitest without dragging in the CJS require() chain from installer.ts. getAvailablePlugins() now throws the tagged disabled error before any fetch. Refs #7524 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(privacy): emit results:catalogDisabled when pluginCatalog off Short-circuits the four catalog-driven socket events. The install/ uninstall events are untouched so operators can still install by plugin name even when the catalog is disabled. Refs #7524 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bin): stalePlugins reads updateServer and honours privacy flag Was hardcoding static.etherpad.org and ignoring opt-out. Now exits 0 cleanly when privacy.pluginCatalog=false. Refs #7524 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(settings): document privacy block in settings template Refs #7524 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api-docs): replace swagger-ui-express with RapiDoc shell Drops the swagger-ui-express dep (third-party Scarf telemetry pixel, see swagger-api/swagger-ui#10573) and serves /api-docs with a static HTML shell that mounts <rapi-doc>. /api-docs.json is unchanged. The vendored RapiDoc asset is added in the next commit so the tree is broken for one diff hunk — pair this with the rapidoc-min.js commit during review. Refs #7524 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(api-docs): vendor RapiDoc 9.3.4 (MIT) as static asset Pinned bundle with checksum in VERSION. Replaces swagger-ui-dist which shipped a Scarf telemetry pixel. Disables RapiDoc's bundled Google Fonts request via load-fonts="false" plus explicit regular-font/mono-font system stacks — RapiDoc's CSS @font-face rules would otherwise fetch Open Sans from fonts.gstatic.com at render time. Also fixes the /api-docs route's res.sendFile to use an absolute path resolved via settings.root (the previous {root: 'src/static'} was resolved from CWD which is already src/, producing src/src/static). Refs #7524 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(admin): banner when plugin catalog is disabled Subscribes to results:catalogDisabled and renders a localized info banner on the plugins page. install/uninstall still function via CLI. Refs #7524 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: PRIVACY.md and README/CHANGELOG pointers Publishes Etherpad's stance on telemetry: two documented, opt-out outbound calls; no third-party analytics; no install-time phone-homes in our deps. Refs #7524 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(admin): await checkPluginForUpdates and emit array on error Qodo flagged that checkUpdates emitted the unresolved Promise (missing await) and emitted {} for updatable on the error path, both breaking the admin UI's expected string[] shape. Pre-existing bug surfaced when the surrounding block was edited for the privacy.pluginCatalog gate. Refs #7524 * feat(api-docs): swap RapiDoc for Scalar (actively maintained) Per @SamTV12345's review on #7757: RapiDoc has been effectively unmaintained for a while. Scalar (https://github.com/scalar/scalar) is MIT-licensed, actively developed, and ships a self-contained standalone bundle that works the same way for our purposes. Privacy posture is preserved by configuring the embed: - withDefaultFonts: false (no fonts.scalar.com woff2 fetch) - telemetry: false (defensive) - agent.disabled: true (no api.scalar.com/vector/* calls) - mcp.disabled: true (no MCP integration) - showDeveloperTools: 'never' - hideClientButton: true Verified with headless Chromium: page loads /api-docs, mounts Scalar, renders the Etherpad OpenAPI document, and makes zero requests to any host other than localhost. Vendor: - src/static/vendor/scalar/standalone.js (@scalar/api-reference 1.57.2) - src/static/vendor/scalar/VERSION (sha256 pinned) - src/static/vendor/scalar/LICENSE (MIT) Removed: - src/static/vendor/rapidoc/* Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-16 18:34:22 +01:00
John McLear	8c4f974122	fix(pad): keep menu_right visible on readonly pads by default (#7783 ) PR #X (issue #5182) added a client-side `$('#editbar .menu_right').hide()` for readonly pads, opt-out via `?showMenuRight=true`. The intent — clean chrome for iframe-embedded readonly announcement pads — was good but the implementation hid the userlist toggle along with import/export. That has two unwanted effects on non-embed deployments: * Plugins like ep_guest inject their "Log In" button into `#myuser` inside the userlist popup, which lives under `.menu_right`. When the guest user (readOnly: true) lands on a readonly pad, the button they need to escape readonly is hidden. Chicken-and-egg. * Settings, embed, home, timeslider, showusers are all legitimately useful for readonly viewers and got removed alongside the actual write-only controls. The server already does the right thing without help from the client: * src/node/utils/toolbar.ts:282-290 strips `savedrevision` from the right toolbar when isReadOnly is true. * src/static/css/pad/popup_import_export.css:1 has `.readonly .acl-write { display: none }`, which hides the Import column of the import/export popup. Export stays visible (and is legitimately useful in readonly). Drop the client-side blanket hide. The iframe-embed use case from #5182 is still served by `?showMenuRight=false` (the existing handler at src/static/js/pad.ts:91-107), and `?showControls=false` continues to hide the entire editbar for callers who want even the left menu gone. Tests: rewrite `hide_menu_right.spec.ts`. The "readonly pad hides .menu_right by default" assertion is inverted; a new test confirms `?showMenuRight=false` still hides on readonly pads. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-16 17:53:20 +01:00
John McLear	2a865a3a4a	feat(admin): show "requires newer Etherpad" when installing incompatible plugin (#7763 ) (#7771 ) * feat(admin): show "requires newer Etherpad" when installing incompatible plugin (#7763) Old admins on out-of-date Etherpad installations get no feedback when they click Install on a plugin that needs a newer core. live-plugin-manager doesn't honor engines.node, and the admin UI dropped the error payload that adminplugins.ts already emits. This wires up an end-to-end signal: - pluginEngineCheck.ts: pure helper comparing a plugin's engines.node range against process.version, with a stable error code (PLUGIN_REQUIRES_NEWER_ETHERPAD) and a message that avoids leaking the Node-version implementation detail. Unparseable ranges fall through as compatible so the preflight is opportunistic, not a gate. 8 unit tests cover the happy + edge paths. - installer.ts: install() now best-effort fetches the published plugin's engines.node from npmjs.org, runs the preflight, and short-circuits with the typed error before invoking live-plugin-manager. Also wraps the body in try/catch so the error reaches the socket callback (it was silently dropped on every install failure today, including network errors). - HomePage.tsx: surfaces finished:install.error as a toast, using a dedicated i18n key when the code is PLUGIN_REQUIRES_NEWER_ETHERPAD and a generic fallback otherwise. - en.json: two new strings, parameterized by {{plugin}} and {{error}}. The message admins see is intentionally about Etherpad, not Node — upgrading Etherpad pulls the Node requirement along with it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugins): don't restart server when every install in the batch failed Qodo PR review on #7771 caught a side effect introduced earlier in this PR. Before this PR, install() never invoked its callback on error, so a failed install left the task counter inflated and onAllTasksFinished() never ran — masking, but not fixing, the bug. Once install() correctly propagates errors to its cb, the counter hits zero on failure too, and onAllTasksFinished() fires hooks.aCallAll('restartServer'). A no-op preflight rejection (EngineIncompatibleError) would then disconnect every connected pad. Fix: extract wrapTaskCb + task state into InstallerTaskQueue and track whether at least one task in the current batch succeeded. Only fire the "all finished" side effect when something actually changed. Failed-only batches do nothing. Seven unit tests cover the matrix: single success, single failure (the regression), mixed batch (still restarts), all-failed batch, batch reset, null cb, two-task drain. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugins): time-bound the engines preflight registry fetch Qodo PR review on #7771 flagged that fetchPluginEnginesNode awaits fetch() with no timeout. A stalled DNS lookup or hung connection to registry.npmjs.org would block install() forever — the finished:install socket event would never fire and the admin UI would stay spinning with no error to surface. Wrap the fetch in AbortSignal.timeout(5000). On any failure (network, HTTP error, abort) fetchPluginEnginesNode returns undefined, which the preflight then treats as "no engines info → compatible," so a slow registry never blocks an install that would otherwise succeed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-16 13:09:17 +01:00
John McLear	924059257d	fix(a11y): name role=toolbar regions, hide linemetricsdiv from AT (#7255 ) (#7777 ) * fix(a11y): name role=toolbar regions, hide linemetricsdiv from AT (#7255) Two regressions called out in the 2026-05-16 follow-up on #7255, after the firefox accessibility inspector flagged them: (1) The "Ether X" announcement between the editor and chat button was the outer ace iframe (titled "Ether") plus a single 'x' text leaf the renderer appends to outerdocbody for line-height measurement (linemetricsdiv in ace.ts). Add aria-hidden=true on creation so AT skips the measurement node entirely. Same approach we used for sidediv in PR #7758. (2) The two formatting/actions <ul role="toolbar"> regions and the history-mode role=toolbar div had no accessible name. Lighthouse + the firefox a11y panel both flagged this. Putting data-l10n-id directly on the <ul> would either destroy its <li> children (textContent branch) or not populate aria-label (the html10n auto-aria-label code path skips non-form-control elements), and a hidden <span> child inside the <ul> would be invalid HTML. Solution: three visually-hidden <span> labels sitting just before #editbar, each carrying data-l10n-id for translation, referenced from the toolbars via aria-labelledby. Apply the same treatment to .show-more-icon-btn, whose aria-label was previously hardcoded English (no data-l10n-id, so untranslated). Adds Playwright assertions for linemetricsdiv aria-hidden and the resolved accessible-name text of each toolbar. Updates the existing show-more test to expect aria-labelledby (it previously asserted hardcoded English aria-label). Refs #7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): reuse translated history-controls label key (Qodo PR review) Qodo flagged: adding `aria-labelledby="editbar-history-label"` on #history-controls overrode the `aria-label` that pad_mode.ts sets from `pad.historyMode.controlsLabel`. That key is already translated in multiple locales (en/de/nl/...); the new `pad.editor.toolbar.history` key was English-only, so non-English users would regress from a localized history-toolbar name to the English fallback. Point the hidden label span at the existing translated key instead of minting a new one, drop the new key from en.json, and update the Playwright expectation to match the translated string ("Pad history controls"). The aria-label that pad_mode.ts still writes to #history-controls is now redundant (aria-labelledby wins) but harmless, and leaving it preserves the runtime relocalization path. Refs ether/etherpad#7777 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): mark toolbar li/a wrappers presentational (Lighthouse, #7255) Lighthouse's axe-core `listitem` rule fires on every toolbar button because role="toolbar" on the <ul> overrides its implicit role="list", leaving the <li> children "orphaned" by axe's heuristic. Murphy's 2026-05-16 follow-up on #7255 attached the Chrome DevTools Lighthouse panel screenshot of this exact failure. Marking the <li>+<a> wrappers role="presentation" tells axe-core they are layout scaffolding for the toolbar role, while the inner <button> keeps its semantics for AT. Same treatment for SelectButton's <li> wrapper. The Separator's <li> also gets aria-hidden=true so AT does not announce an empty list item between toolbar buttons. CSS and JS selectors that still target `.toolbar ul li` continue to work — role="presentation" only affects the accessibility tree, not the DOM tree. No visual or behavioral change for sighted users. Adds a Playwright spec that walks every rendered toolbar <li>/<a> and asserts role="presentation" so future toolbar.ts tweaks can't silently re-introduce the Lighthouse failure. Refs ether/etherpad#7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): label #online_count for AT (#7255 - "number next to the user icon") Murphy's 2026-05-16 follow-up cut off mid-bullet ("It's not clear what the number next to the …"). Best guess: the user-count badge in the showusers toolbar button. Currently it exposes a bare digit to AT — "5" with no context — because the visible badge text is also the entire accessible content. Append a localized aria-label generated from a new pad.userlist.onlineCount key (plural macro for one / other) whenever the count updates, so AT announces "5 connected users" instead of the bare digit. Add role=status and aria-live=polite so the count change is announced inline without forcing the user to refocus the button. Visible badge digit unchanged. html10n.get is null-safe (falls back to an English template so AT never gets back "undefined" before the locale bundle has loaded). Adds a Playwright spec verifying role/aria-live and that the aria-label contains "connected user". Refs ether/etherpad#7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): tighten #online_count + plugin-emitted toolbar <li>s (#7255) Two small follow-ups on top of the toolbar/online-count work: (1) #online_count had no accessible label on solo-author pads. updateNumberOfOnlineUsers — which writes the localized aria-label — only fires on userJoin/userLeave/status change, never on initial single-author load. Call it at the end of init() so the badge ships with its label on first paint. Also bind html10n's 'localized' event so non-English users get the translated label after the locale bundle arrives (matches the keyboard-hint / history-toolbar pattern). Harden the Playwright spec to use polling toHaveAttribute instead of one-shot getAttribute. (2) Sweep role="presentation" onto plugin-emitted toolbar <li>s in pad_editbar.ts init(). Core's toolbar.ts emits its <li>s with the role already, but plugins (ep_headings2, ep_align, ep_font_*, ep_print, ...) ship their own editbarButtons.ejs templates that emit <li> directly, so Lighthouse's listitem rule kept firing on the "with plugins" test runs. Runtime sweep covers anything in the editbar at init time, no plugin coordination needed. Refs ether/etherpad#7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-16 13:08:33 +01:00
John McLear	79f525b0a7	fix(a11y): action Qodo review on PR #7758 (#7764 ) Three bugs and one test-flake risk that Qodo flagged on `c4ea12d`: (1) The keyboard hint element was created with hidden=true. Per the ARIA spec, the HTML hidden attribute removes the element from the accessibility tree, so the body's aria-describedby pointer resolved to a node with no exposed name and screen readers ignored the hint. Removed the hidden attribute; the hint lives in the inner document's <head> where it isn't rendered anyway, so there's no visual cost. (2) Hint text was set once with html10n.get(), which returns undefined when translations haven't finished loading. Since Ace2Editor.init() races with html10n.localize(), the hint could permanently read "undefined" (or be empty). Seed with a hardcoded English fallback so the hint is always usable, and refresh from html10n on the 'localized' event so the fallback is replaced once translations land. (3) Same html10n.bind('localized', refreshHint) re-runs on every runtime language change, fixing the stale-language bug where applyLanguage() wouldn't update the inner-iframe hint. (4) The "skip link is first Tab target" test asserted that document.activeElement === <body> as a precondition. That invariant is fragile — banners, modals, plugins can grab focus on load (the goToNewPad helper documents one such workaround). Replaced the precondition with an explicit blur of whatever is focused, so the assertion is purely about tab order. Qodo's #4 (skip link not feature-flagged) is intentional: accessibility fixes shouldn't be opt-in. A flagged-off skip link wouldn't help anyone, and "preserve pre-existing behavior" doesn't apply to a WCAG 2.4.1 compliance fix. Replying separately on the PR thread. Refs #7255 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-15 20:03:16 +01:00
John McLear	21e1ae2fa3	security: allow integrator sessionID cookie to be HttpOnly (#7045 ) (#7755 ) * security: allow integrator sessionID cookie to be HttpOnly (#7045) The integrator-set sessionID cookie was forced to be non-HttpOnly because Etherpad's own client JS read it via document.cookie and forwarded it in the socket.io CLIENT_READY payload, exposing it to XSS. Mirror the GDPR PR3 author-token migration: read sessionID from the socket.io handshake's Cookie header in PadMessageHandler.handleClientReady, falling back to the legacy message-level field with a one-time deprecation warning per socket. Drop the client-side Cookies.get('sessionID') reads in pad.ts and timeslider.ts so the field is no longer sent by current clients. Existing integrators that set sessionID without HttpOnly keep working unchanged; the field on the message becomes optional and integrators should now mark the cookie HttpOnly; Secure; SameSite=Lax. Closes #7045 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security): treat undecodable handshake cookies as absent (Qodo #7755) decodeURIComponent() throws URIError on malformed values like `%ZZ`. The unguarded call in PadMessageHandler.handleClientReady's readCookie() let a single bad cookie abort CLIENT_READY for that socket, allowing unauthenticated peers to spam server error logs and lock themselves out of pads. Catch URIError and treat the value as absent so the legacy message-level field still serves as a fallback. Other error classes still propagate. Add a backend test that asserts a `sessionID=%ZZ` cookie no longer aborts the handshake. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-15 19:44:55 +01:00
John McLear	7a1d9519f6	fix(a11y): skip-to-content link + hide line numbers from screen readers (#7255 ) (#7758 ) * fix(a11y): skip-to-content link + hide line numbers from AT (#7255) PR #7451 landed editor-region labelling but didn't address two remaining screen-reader complaints from the original report: 1. No way to bypass the toolbar. Murphy noted that screen-reader users had to swipe through ~18 toolbar buttons to reach the editor (WCAG 2.4.1 Bypass Blocks). Adds a skip link as the first child of <body>, hidden offscreen until keyboard focus reveals it. Click handler in pad.ts routes through ace_focus so the inner contenteditable actually receives focus — a plain href="#editorcontainer" anchor only scrolls. 2. Line numbers read individually as "1, 2, 3, ...". The sidediv is visual scaffolding for sighted users; it carries no useful information for AT. Adds aria-hidden="true" on creation in ace.ts so screen readers skip it entirely. Also fixes two issues found while implementing this: - The Escape/Alt+F9 hint that PR #7451 added in the inner iframe's body was being wiped by Ace2Inner.init() (line splices manage body children). Move it to the inner <head> — aria-describedby resolves by ID anywhere in the same document. Localize the text via html10n.get('pad.editor.keyboardHint'). - Skip link and keyboard hint text are both routed through new locale keys (pad.editor.skipToContent, pad.editor.keyboardHint) so they translate via the existing html10n pipeline. Adds two Playwright assertions for the new behavior. Refs #7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): remove editor auto-focus so skip link is Tab-reachable PR #7758 added a skip-to-content link as the first body child, but on local testing it was unreachable via Tab from the URL bar — the standard WCAG 2.4.1 entry path. Root cause: postAceInit calls padeditor.ace.focus() on load, which moves focus into the editor iframe. Tab inside the editor inserts an indent character (handled by ace2_inner's key handler) rather than bubbling to the parent page, so the skip link sat in the DOM but no user could ever reach it. Drop the load-time auto-focus. Initial focus is now on <body>, and the first Tab focuses #skip-to-content as expected. Users now click or Tab into the editor; existing Escape → toolbar exit (PR #7451) and the new Enter-on-skip-link path both still work. Cost: the long-standing "start typing immediately on a fresh pad" UX goes away — one extra click to start editing. This matches the modern accessible default for rich text editors (Slack, Discord, Google Docs all require an explicit focus before accepting input) and removes a keyboard trap that violated WCAG 2.1.2 even after PR #7451's Escape escape hatch. Adds a Playwright assertion that fresh-page Tab focuses the skip link. Refs #7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: focus the editor before typing in three specs These three Playwright tests started failing after the autofocus removal in `ef95498` because they reached for keyboard input without first clicking into the editor: - enter.spec.ts: `lastLine.focus()` is a no-op on a <div> (not natively focusable), so Enter went to <body> instead of the contenteditable. Replaced with `.click()`. - indentation.spec.ts: `selectText()` triple-clicks but the focus event didn't reliably reach the inner contenteditable in CI; added an explicit `.click()` first. - collab_client.spec.ts: each test opens its own browser contexts via goToPad, which does not click into the editor; selectText() in replaceLineText was the only thing landing focus, and that proved flaky. Added `body.click()` at the top of replaceLineText. All three previously relied on the page-load auto-focus that `ef95498` removed. The new pattern (click first, then type) is what writeToPad and clearPadContent already do — these specs now match. Refs #7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): move skip link above eejs body block so plugins can't outrank it The WITH_PLUGINS Playwright suite caught a regression in the previous commit: ep_set_title_on_pad's eejsBlock_body hook prepends a <div id='pad_title'> that contains a focusable <a href="">Loading...</a> into the body block, putting that anchor ahead of #skip-to-content in DOM tab order. First Tab landed on the title link instead of the skip link, breaking the WCAG 2.4.1 entry path. Moving the skip link before <% e.begin_block("body"); %> takes it out of the plugin-modifiable region — eejsBlock_body hooks can prepend to the block content but cannot reach above the block's own start tag. Verified locally with ep_set_title_on_pad installed: On load: BODY After Tab 1: A#skip-to-content After Tab 2: A (pad_title's Loading link — plugin content) Refs #7255 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-15 19:23:52 +01:00
John McLear	451bd9c3eb	feat(pad): scrub history in-place on the pad URL (#7659 ) (#7710 ) * feat(pad): scrub history in-place on the pad URL (#7659) Clicking the timeslider toolbar button now keeps the user on /p/:pad and toggles a hash-based history mode (#rev/N) instead of navigating to a separate /timeslider page. The pad shell — chat, users panel, settings, plugin chrome — stays mounted across the transition. A sticky banner plus a sepia tint on the toolbar make it unmistakable that what is visible is historical, not live. Implementation: - New PadModeController (src/static/js/pad_mode.ts) owns enter/exit, the URL hash, browser back/forward, and a mutation-observer bridge from the inner timeslider's revision label/date into the outer banner. Esc and a Return-to-live button both exit history. - pad.html grows a banner element and an iframe mount slot. The live ACE iframe stays mounted but hidden during history; on exit the socket is still alive, so the user snaps straight back to the current state without a reconnect. - The /p/:pad/timeslider route 302-redirects to the pad page for direct visits (legacy bookmarks), and serves the timeslider HTML for the in-pad iframe when called with ?embed=1. The embedded variant hides the redundant title and return-to-pad button via CSS; the slider, settings, and export controls stay reachable. - Legacy #NN shortlinks are preserved through the redirect by the browser and translated to #rev/NN client-side. Tests: - New backend spec asserts the 302 redirect, pad-name preservation, and the ?embed=1 path still serves the timeslider HTML. - New padmode.spec.ts exercises toolbar entry, return-to-live, browser back, and direct /timeslider URL handling. Asserts the rendered localized banner string, not just element presence. - Existing timeslider specs that hit /p/:pad/timeslider directly now pass ?embed=1 to bypass the redirect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): make history iframe fill the editor area (#7659) Without an explicit positioning model the history-frame-mount inherited half-width from a phantom flex parent and the embedded timeslider rendered at 640×625 instead of the full editor area. Switch to the same absolute-fill model the live ACE iframe uses by making #editorcontainerbox the positioning anchor when in history mode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): address Qodo review and CI failures (#7659) Concrete review fixes for PR #7710: - Tighten the embed query check from `if (!req.query.embed)` to `req.query.embed !== '1'` so values like `?embed=0` no longer bypass the redirect. - Fix the `#rev/latest` mapping: the parser yields -1 for "latest", which the iframe sync handler was clamping to 0 and so jumping the embedded timeslider to revision 0. Resolve "latest" to the inner BroadcastSlider's upper bound instead. - Update existing backend tests (`socialMeta`, `specialpages`) that hit `/p/:pad/timeslider` directly — they now pass `?embed=1` like the rest of the suite. Without this fix three pre-existing tests failed CI (302 instead of 200). - Document the route change in `doc/skins.md` and `doc/skins.adoc`: direct visits redirect; iframe consumers use `?embed=1`. - Back out a stray `data-theme="editorial"` attribute and the hardcoded Google Fonts `<link>` tags from `pad.html` that leaked into the branch from an unrelated working-tree change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(pad): consolidate chrome and replay chat/users in history mode (#7659) Picks up the rough edges left by the initial in-place history mode: the embedded timeslider iframe was rendering its own duplicate Settings and Export buttons, and the chat panel + users list still showed live state while the editor scrubbed back in time. Chrome consolidation - Hide the entire inner editbar's right-side toolbar and modal popups in embedded mode (slider stays). Outer pad shell now owns Settings, Export, Share, Users, Chat across both modes. - Outer Settings popup grows a "History playback" section (visible only when scrubbing) with playback speed + follow-contents. Both bridge to the iframe's BroadcastSlider state. - Outer Export anchors are rewritten to /p/<pad>/<rev>/export/<type> on each scrub and restored on exit, so Save As exports the visible historical revision. Chat replay - Each chat message is annotated with data-timestamp at render time. In history mode, messages newer than the scrubbed revision's timestamp are display:none'd; a "Chat as of HH:MM" header sits above the chat log. - Restores cleanly on exit (inline display cleared, header removed). Users replay - Live users table is replaced with the embedded timeslider's authors-at-this-revision label while scrubbing; restored on exit. Plumbing - Expose padContents on window in broadcast.ts so the outer pad can read currentTime after each scrub without postMessage. - Expose BroadcastSlider on window in timeslider.ts so the outer pad can register an onSlider callback to drive replay UI. Tests - New padmode specs cover: history-only Settings section, hidden embedded chrome, chat filter + replay header, Export href rewriting + restore, authors-row swap + restore. - timeslider_line_numbers cookie-persistence test updated to bypass the now-hidden inner Settings popup (programmatic checkbox). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): theme propagation, hide inert buttons, plugin loading (#7659) Picks up rough edges from the in-place history mode that turned up in real usage: Theme / dark mode - skin_variants.updateSkinVariantsClasses now also walks the history iframe (and its ace_outer/ace_inner) so toggling dark mode while scrubbing re-themes the embedded view in lockstep. - timeslider.ts inherits the parent's skinVariant tokens (super-dark-* / dark-* / full-width-editor) on first paint when it detects it is embedded — same-origin guarantee, falls through silently if not. Toolbar UX - Hide #editbar .menu_left (Bold/Italic/Lists/Indent/Undo/...) and the show-more chevron while in history mode. Those buttons target the hidden live editor and would do nothing useful; rendering them disabled-looking implied state the user doesn't have. Right-side menu (Settings / Share / Users / Chat / Home) stays at full opacity and fully interactive. Slider position - Pin the embedded #editbar to the bottom of the iframe so the outer banner and the slider can't visually compete for the same band of pixels. Reserve padding-bottom on the iframe's editorcontainerbox so the editor never scrolls under the slider. Plugin loading in timeslider - timeSliderBootstrap.js now pre-loads plugin modules into a Map and passes them to plugins.update(), mirroring padBootstrap.js. Without this the loadFn fallback called require(path) at runtime, which the esbuild-bundled timeslider couldn't resolve, so client_hooks like ep_headings2's aceRegisterBlockElements silently failed to register and historical revisions rendered without plugin chrome. Tests - New padmode specs cover: outer toolbar's left/right asymmetry, slider pinned to bottom, dark-mode class propagation into the history iframe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(pad): move history slider into the outer toolbar (#7659) The slider previously rendered inside the embedded iframe — first at the top (where it visually competed with the banner), then briefly at the bottom (where the chat icon overlapped it). Both were wrong. Move the controls into the outer toolbar's left zone, where #editbar .menu_left is hidden in history mode and the slider can occupy the full width without colliding with anything. - pad.html grows a #history-controls div (slider + play/pause/step buttons + timer) inside #editbar, between menu_left and menu_right. Hidden by default; revealed via body.history-mode CSS. - pad.css swaps #editbar .menu_left out for #history-controls in history mode (display:none / display:flex). - timeslider.css fully hides the embedded iframe's #editbar — the outer toolbar now owns the slider, and the iframe is purely the editor surface. - pad_mode.ts wires the outer controls as a remote control: the range input calls inner BroadcastSlider.setSliderPosition, the play button calls BroadcastSlider.playpause, step buttons forward clicks to the inner #leftstep/#rightstep so they share the existing logic. An onSlider subscription mirrors inner state back into the outer slider value, timer label, and play-button .pause class. Tests - Existing timeslider.spec asserts the outer controls are visible. - New padmode specs cover: inner editbar fully hidden, outer toolbar swap (menu_left → history-controls), and outer slider drives the iframe's revision via BroadcastSlider. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): exempt embedded history iframe from userdup kick (#7659) When the in-place history iframe opens its socket, the server's duplicate-author kick treats it as a stale tab and disconnects the parent pad's live socket — toolbar-overlay drops over the editor and Settings/Share/Users/Chat all stop responding. Mark the iframe's connection with `embed=1` in the socket.io handshake query, record it on sessionInfo, and skip the kick whenever either side is embedded. - timeslider.ts: detect `?embed=1` (and parent !== window) on the iframe URL, pass through as a query parameter to socketio.connect. - PadMessageHandler: read socket.handshake.query.embed on CLIENT_READY, set sessionInfo.embed; the duplicate-author kick now skips when either the connecting session OR the existing session is embedded. Behavior preserved - Two real tabs (both non-embedded): older tab still gets kicked. - Authenticated sessions still bypass the kick entirely. - Live pad socket survives entry into history mode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): a11y of history toolbar controls (#7659) The new history controls (slider + play/step/timer) had hardcoded English aria-labels, which html10n won't replace because they were present without the data-l10n-aria-label marker. Screen readers in non-English locales would have heard English. Drop the static aria labels and let html10n.translateElement populate aria-label from the data-l10n-id translation, matching how the rest of the toolbar works. - pad.html: remove hardcoded aria-label on play/step buttons and the range input; keep titles (hover tooltip) and data-l10n-id. Add role="toolbar" + data-l10n-id on the controls container so the toolbar landmark is announced. Mark play button as a toggle with aria-pressed reflecting playback state. - en.json: add pad.historyMode.controlsLabel and pad.historyMode.sliderLabel for the toolbar landmark and the slider. - pad_mode.ts: keep aria-pressed in sync with the inner playback state on every revision update. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(pad): a11y + responsive for history controls (#7659) Two issues with the previous a11y attempt: the data-l10n-id on icon buttons was setting their textContent (drawing "Playback / Pause Pad Contents" on screen next to the glyph), and there was no responsive treatment so the timer + slider could overflow narrow viewports. - pad.html: drop data-l10n-id from the icon buttons. They're now empty <button>s. Localized title (hover tooltip) and aria-label (screen reader name) are populated by pad_mode.localizeControls() using the existing timeslider.* keys, with an html10n.bind subscription so language switches re-localize. - Mark #history-timer as hide-for-mobile. - pad.css: dedicated @media (max-width: 800px) and 480px rules shrink padding, gap, and button widths so play + slider + step buttons stay on a single toolbar line at narrow viewports. Mirrors the legacy timeslider's responsive behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(pad): inline Follow + Playback speed, match toolbar height (#7659) Two follow-ups from real testing: - Move "Follow pad content updates" (now "Follow") and "Playback speed" out of the Settings popup and inline them in the history-mode toolbar, alongside the slider + play/step buttons. They were always needed while scrubbing; one extra click into Settings was friction. Removed the now-empty #history-settings-section. - The history controls toolbar was visibly shorter than the live toolbar because the icon buttons sat as bare <button> elements without the live editbar's <li><a> wrapping. Add explicit min-height (40px) and per-button padding so the toolbar is the same vertical size in both modes — switching between live and history no longer reflows. - Differentiate "iframe-mounted history view" from "direct ?embed=1 visit". Only the former hides the inner timeslider editbar — direct visits keep their full chrome so existing test/legacy entry points stay independently usable. Marker: timeslider.ts adds an `iframe-mode` class on body when window.parent !== window; CSS scopes the hide to that combo. Tests - padmode spec asserts Follow + Speed live in the toolbar (not the Settings popup) and are visible in history mode, hidden in live. - timeslider.spec direct-?embed=1 flows continue to pass because the inner editbar is no longer hidden when not iframe-mounted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> fix(pad): use absolute path for legacy /timeslider redirect (#7659) CI Firefox failed the legacy-URL redirect test (1 of 32 jobs); Chromium passed. The redirect Location header was a relative `../padname`, which both browsers resolve to /p/padname for `/p/padname/timeslider`. Firefox flaked on it once consistently. Switch to an absolute path including the proxy prefix so the resolution is unambiguous across browsers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): accept 304 on legacy timeslider redirect (#7659) CI Firefox failed `expect(res.status()).toBe(200)` because Firefox issues a conditional GET when the redirect target is the same URL the test just loaded via goToNewPad — the server returns 304 Not Modified and the test treats that as a regression. Chromium happens to send fresh requests so it stayed green. Accept either 200 or 304 — both are valid completed navigations to the pad page; what we actually care about is the pathname assertion above. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(pad): eye toggle for Follow, fix line-number alignment (#7659) Two refinements from real testing in 9002: Follow as an eye toggle - Replace the labeled checkbox with an inline-SVG eye icon. The eye is always rendered; a diagonal slash is overlaid via SVG <line> only when the underlying (visually hidden) checkbox is unchecked. Default state is on (auto-following) so the eye renders unobstructed. - Localized hover tooltip + aria-label flips with state — html10n populates "Following pad changes — click to stop following" vs "Not following pad changes — click to follow", and pad_mode.ts re-applies on every change event so screen readers narrate the action the click would take. - Hidden checkbox keeps the existing pad_mode.ts bridge code working (still reads .checked) and lets <label for="…"> handle the click. Line-number alignment fix (broadcast.ts) - The first-line height formula was `nextDocLine.offsetTop - innerdocbody.padding-top` which only computes the right value when innerdocbody is the offsetParent. In the in-pad history iframe, outerdocbody contributes its own padding-top to the offsetTop chain, so the first gutter row was 20px too tall and every subsequent line drifted out of alignment. Use the consistent `next.offsetTop - current.offsetTop` formula for every iteration — same result in the standalone timeslider, correct result in the embedded one. - New padmode spec asserts every gutter row's top matches the editor line's top within 2px, in iframe-mounted history mode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-10 16:21:56 +01:00
John McLear	df0ecec834	a11y: localize aria-label on form-control elements (<select>, <input>, <textarea>) (#7713 ) #7584 introduced auto-population of aria-label from a translation when an element has data-l10n-id and no author-supplied aria-label. That branch only fires for elements with no children (or with data-l10n-id ending in a recognized attribute suffix like .title). Form-control elements break the assumption: a <select> always has <option> children, an <input>/<textarea> may have implicit value content. The textContent branch handles them, but the aria-label fallback wasn't called from there. Plugins like ep_font_size, ep_headings2, and ep_hljs end up with a localized translation applied to a <select> but no accessible name on the element itself. Calls populateAriaLabel() from the textContent branch when the node is a <select>, <input>, or <textarea>. Keeps the same "author-supplied aria-label wins on first pass; the data-l10n-aria-label marker lets us refresh values we wrote" semantics from #7584. Adds Playwright coverage in src/tests/frontend-new/specs/html10n_form_controls_aria.spec.ts: - aria-label is populated on <select> with data-l10n-id - aria-label is populated on <textarea> with data-l10n-id - author-supplied aria-label is preserved on first pass	2026-05-10 12:23:33 +01:00
John McLear	efb8328084	feat(updater): tier 2 — manual-click update from /admin/update (#7607 ) (#7704 ) * docs(updater): PR 2 (Tier 2 manual-click) implementation plan 20-task TDD plan for shipping the manual-click update flow on top of the Tier 1 (notify) work merged in #7601. Covers UpdateExecutor, RollbackHandler, SessionDrainer, lock + trustedKeys, four admin endpoints (apply / cancel / acknowledge / log), admin UI updates, integration tests against a tmp git repo, and a manual smoke runbook for the spec's "before each tier ships" gate. Plan deliberately scopes signature verification to an opt-in stub (updates.requireSignature: false default) to avoid blocking on a separate release-signing project. Plan: docs/superpowers/plans/2026-05-08-auto-update-pr2-manual-click.md Spec: docs/superpowers/specs/2026-04-25-auto-update-design.md Issue: ether/etherpad#7607 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): extend state + settings for Tier 2 manual-click Adds ExecutionStatus discriminated union, bootCount, and lastResult to UpdateState, plus the preApplyGraceMinutes/drainSeconds/diskSpaceMinMB/ requireSignature/trustedKeysPath knobs that Tier 2's executor needs. loadState backfills the new fields on Tier 1 state files so existing installs keep working. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): PID-based update.lock with stale-pid reaping Single-flight guard for Tier 2's UpdateExecutor. Atomic O_CREAT\|O_EXCL acquire; on EEXIST, sends signal 0 to the recorded PID and reaps if dead. Unparseable / partially-written lock files are treated as stale rather than fatal so a half-written lock from a SIGKILL'd parent doesn't lock the install out forever. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): verifyReleaseTag — gpg-via-git stub for Tier 2 preflight Default updates.requireSignature=false: log a warning and return ok with reason=signature-not-required. Set true to make preflight refuse a tag whose signature does not verify under the system keyring (or trustedKeysPath via GNUPGHOME). Etherpad's release process does not yet sign tags consistently; turning the check on by default would break Tier 2 for every admin and forcing a release-signing change is out of scope for this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): preflight check pipeline for Tier 2 Pure orchestrator over injected probes for install-method, working tree, disk space, pnpm presence, lock state, remote tag existence and signature verification. Cheap-and-definitive checks run first; first failure short-circuits with a typed reason that the route layer will surface in the preflight-failed admin banner. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): rolling update.log helpers (appendLine + tailLines) Direct file-append + size-based rotation rather than a log4js appender — avoids re-configuring log4js on top of the user's existing logconfig. appendLine creates parents, rotates at 10MB (configurable), keeps 5 backups by default. tailLines reads the last N lines for /admin/update/log. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): SessionDrainer + handshake guard Drainer schedules T-60 / -30 / -10 broadcasts and resolves at T=0; isAcceptingConnections() flips off for the duration. PadMessageHandler consults the flag at the start of CLIENT_READY and disconnects new joiners with reason "updateInProgress" — existing sockets are unaffected. Drains shorter than 30s collapse the early timers to fire ASAP rather than queue past the drain end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): UpdateExecutor — snapshot, fetch/checkout/install/build, exit 75 Pure-DI orchestrator: spawnFn, copyFile, readSha, saveState, exit are all injected so unit tests run the full pipeline without spawning real children or mutating the real install. Streams stdout/stderr to update.log via the now-best-effort appendLine helper (swallows fs errors so the executor itself never breaks on read-only / unwritable log dirs). Failure paths transition to rolling-back and return — the route layer hands off to RollbackHandler which owns the rollback exit, so we don't double-exit and lose tail lines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): RollbackHandler — health-check timer + crash-loop guard checkPendingVerification arms a 60s timer at boot when state is pending-verification and increments bootCount; bootCount>2 forces an immediate rollback (crash-loop guard). markVerified persists the verified state and stops the timer. performRollback restores the backup lockfile, runs git checkout <fromSha> and pnpm install, lands on rolled-back or rollback-failed (terminal) on sub-step failure, exits 75 either way so the supervisor restart brings the new state up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): wire RollbackHandler into boot + UpdatePolicy honours rollback-failed - expressCreateServer now invokes checkPendingVerification before polling starts so a previous boot's pending-verification either re-arms the health-check timer or, when bootCount has climbed past the crash-loop threshold, forces an immediate rollback. - server.ts calls markBootHealthy after state hits RUNNING so /health-being-up is the implicit happy-path signal that cancels the rollback timer. - /admin/update/status surfaces execution + lastResult + lockHeld so the admin UI can render the right Apply / Cancel / Acknowledge state. - UpdatePolicy gains an `executionStatus` input. While it equals 'rollback-failed', canAuto / canAutonomous are denied (reason: rollback-failed-terminal); manual stays on because clicking Apply IS the intervention the terminal state needs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): apply / cancel / acknowledge / log endpoints Strict admin-only POSTs that drive Tier 2's manual-click flow: - POST /admin/update/apply: acquire lock, persist preflight, run preflight, drain $drainSeconds, executeUpdate (which exits 75 on success), or run performRollback on a failure path (also exits 75). - POST /admin/update/cancel: cancel a pre-execute drain/preflight, write cancelled lastResult, release lock. - POST /admin/update/acknowledge: clear terminal states (preflight-failed, rolled-back, rollback-failed) back to idle. lastResult is preserved so the admin still sees what happened. - GET /admin/update/log: tail var/log/update.log (200 lines) for the in- progress UI. Strict admin auth. Also: - socketio hook exports getIo() so the apply endpoint can broadcast the drain shoutMessage outside the regular hook surface. - ep.json registers updateActions after admin/updateStatus. - 11 mocha integration tests cover auth, policy denial, execution-busy, acknowledge-clears-terminal, log content-type. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): admin UI Apply/Cancel/Acknowledge + live log stream UpdatePage renders the right action set based on execution.status: Apply when idle/verified and policy allows, Cancel during preflight/draining, Acknowledge on terminal preflight-failed / rolled-back / rollback-failed. While the executor is in flight (preflight/draining/executing/rolling-back) the page polls /admin/update/log + /admin/update/status once a second and shows the rolling tail; polling stops automatically when the run terminates. lastResult and policy denial reasons surface localised copy. Buttons disable themselves while a network round-trip is in flight to dodge double-clicks. New i18n keys live under update.page.{apply,cancel, acknowledge,log,execution,policy.,last_result.}, update.execution., update.banner.terminal.rollback-failed, and update.drain.{t60,t30,t10}. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> feat(updater): pad shoutMessage renders update.drain.* via html10n broadcastShout now sends {messageKey, values, sticky} so the existing pad-side shout pipeline can route through html10n.get(). The renderer gains a values pass-through so update.drain.t60 etc. interpolate {{seconds}}, and gives updater shouts a different gritter title (the banner.title localised string) so users know it's a system event rather than a generic admin message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): rollback uses git checkout -f + integration suite over tmp git repo RollbackHandler now does git checkout -f <fromSha> BEFORE overlaying the backup lockfile. Without -f, git refuses checkout when there are unstaged modifications to files it would overwrite — exactly the case after a partial executor run that mutated the working tree. With -f the partial mutation is discarded and the working tree returns to fromSha cleanly. The backup-lockfile copy is still done (belt-and-braces) but tolerates ENOENT since checkout already restored the right lockfile. The new integration suite at src/tests/backend/specs/updater-integration.ts exercises the full pipeline against a disposable git repo: happy path, install-fail rollback, build-fail rollback, crash-loop guard, and a target-sha-doesn't-exist rollback-failed terminal case. 5 mocha tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(updater): Playwright admin Apply / Cancel / Acknowledge flow Stubs /admin/update/status (and /admin/update/apply for the apply path) at the route level so we can assert UI transitions without actually running an update. Four scenarios: - Apply button POSTs and re-fetches status (>=2 status fetches total). - install-method-not-writable hides the button and shows localised denial copy. - rollback-failed terminal state shows the Acknowledge button and the "Manual intervention required" lastResult copy. - lockHeld=true hides Apply even when policy.canManual is on. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(updater): admin banner shows rollback-failed terminal alert When execution.status === 'rollback-failed' the banner switches to a role=alert with the strong update.banner.terminal.rollback-failed copy and overrides the regular "update available" framing — an admin who left the system in this state needs to fix it before any other admin work matters. Other terminal states (preflight-failed, rolled-back) are informational and surface on the page itself, not the banner. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(updater): Tier 2 admin docs + manual smoke runbook + CHANGELOG doc/admin/updates.md gains a full Tier 2 section: prerequisites (git install + process supervisor with sample systemd unit), Apply flow with timings, every failure mode and the resulting state, the four endpoints, and the signature-verification opt-in. Settings table picks up the new updates.* knobs. docs/superpowers/specs/2026-04-25-auto-update-runbook.md is the manual smoke runbook the design spec calls for: disposable VM, systemd unit, every observable transition (happy path, install/ build-fail rollback, crash-loop guard, rollback-failed terminal, cancel during drain) plus a sign-off checklist for the release cut. CHANGELOG Unreleased section explains the supervisor requirement and points readers at the runbook. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(updater): note docker-friendly update flows as follow-up work Tier 2 refuses Apply on installMethod=docker because in-container mutation doesn't survive a container restart. Adds a future-work note covering the two reasonable paths for an in-product docker Apply button (instructions-only vs deploy-webhook) and explicitly rules out mounting /var/run/docker.sock as a footgun. Watchtower gets a pointer for admins who want fully autonomous docker updates today. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(updater): address Qodo review (1-6) + Playwright strict-mode CI fix 1. Tier 2 endpoints now gate on tier in {manual, auto, autonomous} — notify and off return 404 to match the prior PR-1 behaviour. Gate is evaluated per-request via app.use middleware so a settings.json reload takes effect without a full restart, and so integration tests can flip the tier dynamically. Adds a regression test that exercises 404 at tier=notify across all four endpoints. 2. cancel/apply race fixed: /admin/update/cancel no longer releases the lock — apply's finally block owns it for the request's lifetime. Apply now reloads state after preflight and aborts with 409 cancelled-during- preflight if execution.status is no longer 'preflight' for the same targetTag. Prevents a second apply from sneaking in while the first is still running its slow checks, and prevents the post-cancel apply from continuing into drain/execute. 3. SessionDrainer now restores acceptingConnections=true at drain completion (not just on cancel). The lock + persisted execution.status prevent a fresh apply from racing in — the in-memory flag was redundant safety that turned into a wedge if the executor threw post-drain. Adds a unit test asserting the flag is restored after natural drain end. 4. PadMessageHandler drain guard switched from socket.json.send (a socket.io v2/v3 API that may not exist on v4) to socket.emit('message', ...) for consistency with the other disconnect paths in the file. 5. Spawn 'error' handlers added to runStep helpers in UpdateExecutor and RollbackHandler, plus the gpg verify-tag spawn in trustedKeys. Without them, a missing/unexecutable binary leaves the promise hanging forever and the update flow stuck in-flight. SpawnFn type extended to allow on('error', ...) listeners cleanly. Spawn errors now resolve with code 1 + the error message in stderr, so the existing failure-detection branches fire normally. 6. executeUpdate body wrapped in try/catch. An exception from readSha, saveState, copyFile, or any step now lands in a rolling-back persist + returns failed-checkout, so the route's post-executor rollback path picks it up. State can no longer wedge at 'executing'. The catch's inner saveState is itself try/wrapped so a write-after-write failure doesn't crash the route either. CI: Playwright update-page-actions strict-mode violation fixed. Both the banner and the lastResult <p> contain "Manual intervention required"; selector now scopes to p.last-result-rollback-failed for the lastResult assertion specifically. 129 vitest unit tests + 23 mocha integration tests passing; ts-check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(updater): address Qodo #7 (status leak) + #8 (short-drain values) #7. /admin/update/status now redacts diagnostic strings for unauth callers even when requireAdminForStatus is left at its default (false). Status enum + outcome enum are kept (the admin banner / pad-side badge need them to render the right UI) but execution.reason / execution.fromSha / execution.targetTag and the same fields on lastResult are stripped. Authed admin sessions still get the full payload — they're looking at their own server's diagnostics. Two new mocha tests cover both paths: "redacts execution.reason / lastResult.reason for unauth callers" and "returns full diagnostic payload to authed admin sessions". #8. SessionDrainer no longer schedules T-30 / T-10 broadcasts when the configured drainSeconds can't honour them. Previously, with drainSeconds < 30 the T-30 timer fired at zero remaining but the broadcast still claimed "30 seconds" — misleading. Now T-30 only schedules when drainSeconds > 30 and T-10 only when > 10. Admins picking a short drain get fewer announcements but each carries an accurate countdown. The opening announcement now reports the configured drain length rather than a hardcoded 60. Two updated unit tests: drainSeconds=15 (skips T-30, still fires T-10) and drainSeconds=5 (skips both). 131 vitest unit + 26 mocha integration tests passing; ts-check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(updater): address Qodo follow-up — tag injection, rollback rejections, state validation Qodo posted three new concerns after the first fix push. 1. Git tag option injection (security). The release tag from GitHub's tag_name flowed into `git checkout` / `git verify-tag` as a positional arg. A tag starting with '-' would be parsed as an option and could bypass signature verification or change checkout semantics. Mitigated in three layers: - New refSafety helper (isValidTag / assertValidTag / refsTagsForm) enforces a strict subset of git's check-ref-format spec: rejects leading '-' or '.', whitespace, control chars, and ~ ^ : ? * [ \\ and the '..' sequence. - VersionChecker validates tag_name before persisting to state, so a malformed value from a misconfigured githubRepo never lands on disk. - UpdateExecutor calls assertValidTag and uses the refs/tags/<tag> form for git checkout. trustedKeys also validates and adds '--' to git verify-tag for an end-of-options marker. updateActions does an up-front isValidTag check on state.latest.tag so a corrupt state file gets a clean 409 instead of a 500. 2. Unhandled rollback rejections. checkPendingVerification was firing `void deps.saveState(...)` and `void performRollback(...)` without .catch(), so an fs error during boot's rollback path would bubble out as an unhandled rejection. Both callsites now go through fireSaveState / fireRollback helpers that catch and log; rollback rejections fall through to a best-effort terminal-state write + exit 75 so the supervisor can re-try the next boot with bootCount++. 3. Execution state under-validated. isValidExecution previously checked only that `status` was a known enum value, so a hand-edited state file with `{execution: {status: 'pending-verification'}}` (missing fromSha / targetTag / deadlineAt) would pass validation and reach RollbackHandler with undefined refs. The validator now consults a per-status required-fields map mirroring the ExecutionStatus union in types.ts and rejects empty strings as well as missing fields. Same tightening applied to lastResult.outcome (must be in the allowed enum, not just any string). Six new unit tests cover hand-edited corruption. 145 vitest + 26 mocha tests green; ts-check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-10 09:00:07 +01:00
John McLear	c47ffd5705	feat(export): native DOCX export via html-to-docx (opt-in) (#7568 ) * feat(export): native DOCX export via html-to-docx (opt-in) Addresses #7538. The current DOCX export path shells out to LibreOffice, which means every deployment that wants a Word download either installs soffice (~500 MB) or loses that export. This PR adds a pure-JS alternative: render the HTML via the existing exporthtml pipeline, then feed it to the `html-to-docx` library in-process to produce a valid .docx buffer — no soffice required, no subprocess spawn, no temp file dance for the DOCX case. Behavior: - `settings.nativeDocxExport` (default `false`) gates the new path so existing deployments see zero behavior change. - When enabled, `type === 'docx'` requests skip the LibreOffice branch, run `html-to-docx(html)`, and return the buffer with the `application/vnd.openxmlformats-officedocument.wordprocessingml.document` content-type. - If the native converter throws, the handler falls through to the existing LibreOffice path — so flipping the flag on is safe even on a mixed-installation where soffice is still present as a backstop. - Other export formats (pdf, odt, rtf, txt, html, etherpad) are unchanged. Files: - `src/package.json`: `html-to-docx` dep (pure JS, no binary reqs) - `src/node/handler/ExportHandler.ts`: new DOCX branch gated on the setting, with fall-through on error - `src/node/utils/Settings.ts`, `settings.json.template`, `settings.json.docker`, `doc/docker.md`: wire up the new setting + env var (`NATIVE_DOCX_EXPORT`) - `src/tests/backend/specs/export.ts`: two new tests — asserts the exported buffer is a valid ZIP (PK\x03\x04 signature) and the response carries the correct content-type — both with `settings.soffice = 'false'` to prove the path doesn't need soffice at all. Out of scope for this PR: - Native PDF export (would need a PDF rendering step — separate undertaking, and the issue acknowledges the `pdfkit`/puppeteer size trade-off). Closes #7538 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(7538): skip native DOCX test when html-to-docx isn't installed The upgrade-from-latest-release CI job installs deps from the previous release's package.json (before this PR adds html-to-docx) and then git-checkouts this branch's code without re-running pnpm install. Under that one workflow the new test can't find the module and fails on the LibreOffice fallback, masking that the native path actually works in every normal install. Guard the describe block with require.resolve('html-to-docx'); Mocha's this.skip() on before cascades to the sibling its. Regular backend tests (pnpm install against this branch's lockfile) still exercise it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(7538): spec for soffice-free DOCX/PDF export and DOCX import Captures the agreed scope expansion of PR #7568: replace the flag-gated native DOCX path with a soffice-first selection cascade, add native PDF export via pdfkit + a small htmlparser2-driven walker, and add native DOCX import via mammoth. Also defines a shared HTML sanitizer (stripRemoteImages) used by both export converters to close the SSRF surface that Qodo flagged on the html-to-docx path. The spec drops the nativeDocxExport setting and its env var; with soffice configured, behavior is unchanged, and with soffice null, docx/pdf export and docx import all work in-process. odt/doc/rtf (and pdf import) keep needing soffice and are documented as such. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(7538): implementation plan for native DOCX/PDF and DOCX import Bite-sized TDD task breakdown of the soffice-free export/import work: rebase, deps, sanitizer, PDF walker, mammoth wrapper, ExportHandler cascade, route guard, ImportHandler branch, UI fix, flag rollback, verification + Qodo reply. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(7538): add pdfkit, htmlparser2, mammoth deps Pure-JS, no native binaries: - pdfkit ^0.18.0 (PDF rendering) - htmlparser2 ^12 (SAX parser used by walker + sanitizer) - mammoth ^1.12 (DOCX -> HTML for native import) - @types/pdfkit ^0.17 (dev) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(7538): add stripRemoteImages HTML sanitizer Drops <img src=> elements pointing at non-data, non-relative URLs to prevent the DOCX/PDF converters from making outbound requests via plugin-modified HTML. Closes Qodo finding #4 against the html-to-docx path; will be wired into both export branches in the cascade refactor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(7538): native PDF export via pdfkit + htmlparser2 walker Renders pad HTML to a PDF Buffer in-process: headings, paragraphs, lists, links, inline emphasis, data:-URI images. Remote images are explicitly skipped at the walker (defense-in-depth on top of the shared stripRemoteImages sanitizer). PDFs are emitted with compress:false so accessibility/SEO indexers that don't FlateDecode can still extract text. Pads are small enough that the size cost is negligible. Walker is 167 LOC, well under the spec's 500-LOC bail-out threshold for switching to pdfmake+html-to-pdfmake+jsdom. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(7538): native DOCX import via mammoth Wraps mammoth.convertToHtml so a soffice-less Etherpad can ingest .docx files. Images are coerced to data: URIs at the converter boundary so the import pipeline never sees a remote src=. Includes a tiny generated DOCX fixture (heading, paragraph, list) under tests/backend/specs/fixtures/ for both this wrapper test and the upcoming end-to-end import test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(7538): soffice-first cascade in ExportHandler Replaces the flag-gated DOCX branch with a deterministic dispatch: soffice if configured, native DOCX/PDF otherwise, 5xx on native error. Both native paths run plugin-modified HTML through stripRemoteImages first. Test changes: - existing native DOCX block now sets soffice=null (was 'false', a truthy non-null string that sidestepped the route guard); fixes Qodo finding #3. - new native PDF integration tests assert %PDF- header and application/pdf content-type with soffice=null. - new negative test: with soffice=null, /export/odt still returns the 'not enabled' message. - the legacy 500-on-export-error test now uses /bin/false so it exercises the soffice error path explicitly (the cascade dropped the ad-hoc 'false' string; .doc has no native path so this still works as a soffice error probe). Integration tests for native DOCX/PDF currently fail because the /export route guard still treats both formats as soffice-only; the next commit fixes that. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7538): allow docx/pdf through export guard without soffice Tightens the no-soffice block to ['odt','doc'] only — formats with no native path. docx and pdf are handed to ExportHandler, which dispatches to the in-process converters. Closes Qodo finding #2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(7538): native DOCX import path in ImportHandler When soffice is null and the upload is .docx, run mammoth and feed the resulting HTML through setPadHTML. Other office formats (pdf/odt/doc/rtf) are explicitly rejected with uploadFailed instead of silently falling through to the ASCII-only path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7538): always show DOCX/PDF export links Native paths (#7538) make DOCX and PDF available regardless of soffice presence, so unconditionally render those links. ODT still gates on exportAvailable. Closes Qodo finding #2 on the UI side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(7538): drop nativeDocxExport flag Selection is now purely soffice-presence-driven (cascade in ExportHandler). The opt-in setting and its NATIVE_DOCX_EXPORT env var are no longer needed -- soffice configured means soffice path; soffice null means native path (DOCX, PDF, and DOCX import). Reverts the additive surface introduced earlier in this PR. Also updates the SOFFICE doc row to reflect that null no longer means 'plain text and HTML only' -- docx/pdf export and docx import now work natively without soffice. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(7538): tighten link annotation assertion CodeQL flagged the loose 'raw.includes("etherpad.org")' as 'incomplete URL substring sanitization' (a false positive in test context, but worth fixing). Match the full /URI (host) form instead -- it's both more accurate (we're verifying the PDF link annotation structure) and CodeQL-clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7538): cleaner DOCX/PDF output + round-trip test coverage DOCX: - New extractBody helper drops <head>/<style> and the leading newline inside <body> so html-to-docx doesn't render CSS or prefix paragraphs with empty space. - New wrapLooseLines pre-processor wraps loose pad lines in <p> before the converter sees them. html-to-docx renders <br> outside <p> as a new <w:p> (full empty line in Word); inside <p> it correctly emits <w:br/> (soft break). Etherpad's HTML uses bare <br> for every line, so this was making single Enters look like double Enters in the Word output. PDF: - Walker SKIP_TAGS rejects head/style/script/title/meta/link content -- prior version dumped CSS into the rendered PDF. - New breakLine() helper combines flushLine() with moveDown(1). pdfkit's text('', false) closes the continued run but does NOT advance the cursor, so consecutive runs were stacking at the same y-coordinate. <br>, end-of-block, and list items now use breakLine(). - ontext collapses runs of whitespace and drops pure-whitespace text nodes so pretty-printed source HTML doesn't render its formatting newlines. Round-trip: - New backend test: pad text -> DOCX export -> DOCX import -> new pad. Asserts content survives the trip. - New PDF sanity test: extracts visible text from the PDF stream and asserts the source pad text appears verbatim. - 6 new unit tests for extractBody and wrapLooseLines plus 1 for PDF walker SKIP_TAGS coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7538): CodeQL ReDoS + import.ts type error - BR_PARA_RE was /(?:\s<br>\s){2,}/ -- two adjacent \s* runs can match the same chars, so on '<br>\t<br>\t<br>...' the regex backtracks exponentially. Re-anchored to match a fixed first <br> followed by one or more additional <br>s, so each whitespace run has exactly one home. - import.ts: fetchBuffer was typed Promise<Buffer> but call sites chained .expect(200) on it, which only works on supertest's Test object. Return the Test (typed any) so the chain is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7538): plugin-aware HTML cleanup, PDF text-align, monospace ep_headings2/ep_align emit one heading-styled blank-line block after every styled line in the pad ('<h1 style=text-align:right></h1>'), which both html-to-docx and our pdfkit walker render as a full empty paragraph. Plus the pdfkit walker had no support for text-align or monospace, so right/center alignment and 'code' lines rendered the same as plain body text. - New dropEmptyBlocks helper strips empty h1-h6/p/code/pre/div/ blockquote wrappers in preprocessing. Iterates so nested empties collapse too. Applied before both DOCX and PDF conversion. - PDF walker now reads style='text-align:left\|center\|right\|justify' on block elements (h1-6, p, div) and passes it as pdfkit's align option. align is applied once per continued run, then reset on flushLine so the next block can pick up its own value. - PDF walker handles <code>, <tt>, <kbd>, <samp> as inline monospace (Courier) and <pre> as block monospace (Courier + breakLine on open/close). 11 new unit tests: - 4 for dropEmptyBlocks (heading wrappers, code, nesting, pass-through) - 1 for PDF text-align (compares the BT matrix x for left vs right) - 2 for Courier in <code> and <pre> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7538): separate adjacent heading-style blocks on import Round-trip bug: ep_headings2 emits <h1>/<h2>/<code> in pad HTML. Mammoth round-trips them as adjacent <h1>A</h1><h2>B</h2><p>C</p> on import. Etherpad's server-side content collector has a default _blockElems set of just {div, p, pre, li}, and ep_headings2 only registers the CLIENT-side aceRegisterBlockElements -- not the server-side ccRegisterBlockElements. So h1/h2/code end up being treated as inline by the importer, and adjacent blocks merge into a single pad line. Fix: insert <br> after </h1>...</h6>/</code> when followed by another block. Server-side workaround keeps this PR self-contained regardless of plugin version. The right long-term fix is to extend ep_plugin_helpers' lineAttribute factory to register both hooks (filed as a follow-up). Tests: - 5 unit tests for separateAdjacentHeadingBlocks - New end-to-end round-trip test asserts H1+H2+P land on three separate pad lines after the import path. Plus the prior PDF text-align/Courier/code commit also included here: - code/tt/kbd/samp inherit text-align from style attribute - pre inherits text-align too Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7538): blank-line round-trip + DOCX code monospace + a==c tests DOCX round-trip dropping blank pad lines: - wrapLooseLines now emits an explicit <p></p> marker for each blank line in a <br> run, instead of collapsing all gaps into a single paragraph break. (N consecutive <br>s -> 1 paragraph boundary + N-2 empty <p></p> markers, mapping to N-1 blank pad lines.) - mammoth's docxBufferToHtml now passes ignoreEmptyParagraphs:false so the empty <w:p> entries survive the import side. mammoth's default of true was silently dropping them. - dropEmptyBlocks no longer strips <p></p> -- that's the meaningful marker for the round-trip. Empty <h1>/<code>/<pre>/<div>/ <blockquote> are still stripped (plugin noise). DOCX <code> rendering as monospace: - New applyMonospaceToCode wraps code/tt/kbd/samp/pre content in a <span style="font-family:'Courier New', monospace">. html-to-docx honors that and emits <w:rFonts w:ascii="Courier New".../>, which Word renders as Courier. The bare <code> tag is otherwise just a no-op for html-to-docx. - Applied only on the DOCX export path (PDF walker already handles monospace via Courier font selection). Round-trip tests: - New a==c suite: txt, etherpad, html, docx -- export from src, import to dst, re-export and compare against the meaningful invariant (line text for binary formats; trimmed body for HTML). - HTML test tolerates one trailing <br> per round-trip because setPadHTML appends a final <p> on import; this is pre-existing core behavior, not our bug. - DOCX test normalizes trailing newline run (same reason). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7538): preserve <w:jc> alignment through mammoth round-trip mammoth doesn't expose Word's paragraph alignment (`<w:jc>`) when it converts a docx to HTML -- there's no equivalent in its style-mapping machinery. To keep alignment through DOCX round-trips we walk the docx's document.xml directly, pull the `w:val` from each `<w:p>`'s `<w:jc>`, and inject `style="text-align:..."` onto the matching block element in mammoth's output by document order. Word's w:jc accepts more values than CSS text-align; we map left/ start, center, right/end, both/justify/distribute and skip the rest (start/end take left/right because we don't track ltr/rtl from the docx for now). Combines with the upstream ep_align PR (ether/ep_align#183) for the full round-trip: this PR makes the mammoth output carry the alignment style; ep_align#183 makes the importer pick it up. Closes the alignment side of #7538. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7538): preserve <a href> inside <code>/<pre> in DOCX export html-to-docx silently drops <a href> children of <code>/<pre> tags (and of styled <span>s, but the <code> wrapper is the active offender here). The pad-export HTML produced by ep_headings2 + ep_align uses <code style='text-align:right'>...<a href>...</a>...</code> for each 'Code'-style line, which lost its links on every DOCX export. Workaround: applyMonospaceToCode now drops the code/pre/tt/kbd/samp wrapper entirely. The non-anchor content gets wrapped in monospace spans; anchors are emitted unstyled so they keep their hyperlink. For block-level usage (<pre>, or <code> with an inline style attr) we emit a wrapping <p> and forward the text-align style. Run BEFORE wrapLooseLines so the <p> doesn't get double-wrapped. Tests added: - inline <code> -> just a styled span (no <code> wrapper) - <code style='text-align:right'> -> <p style> wrap - <pre> -> always block-wrapped - <tt>/<kbd>/<samp> -> inline span only - regression: <a href> inside <code> survives html-to-docx round-trip with both the URL in word/_rels/document.xml.rels AND a <w:hyperlink> in the document body Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7538): HTML import line-break drift between blocks Etherpad's HTML export wraps each pad line in <p>...</p> (or <h1>, <code>, etc.) and then appends a <br> between lines. The closing block tag already ends the line for contentcollector, so the trailing <br> is redundant -- and on import the server collector counts BOTH as line breaks, doubling every blank line between paragraphs and inserting an extra blank between adjacent headings. Two fixes, both gated on the runtime block-element registry so they don't double-trigger when the underlying plugin already handles adjacency: 1. HTML import path now runs the new collapseRedundantBrAfterBlocks helper before setPadHTML. Drops a single <br> immediately following </p>/</h1-6>/</code>/</pre>/</div>/</blockquote>/</ul>/ </ol>/</li>/</table>/</tr>/</td>/</th>. Multiple consecutive <br>s after a block keep all but the first (the rest still represent intentional blank lines). 2. The DOCX-import separateAdjacentHeadingBlocks workaround now checks whether 'h1' is in the runtime ccRegisterBlockElements set before inserting <br>s. When ep_headings2 has the new server hook (per ep_plugin_helpers#14 + the upcoming ep_headings2 PR), the workaround correctly stays out of the way -- otherwise it adds an extra blank line per heading transition. Also fixed a subtle ts-check failure on the import.ts test changes and a leftover implicit-any in ImportDocxNative's alignment preserver. Tests added: - collapseRedundantBrAfterBlocks: 5 unit tests (each block tag, whitespace tolerance, multiple <br> keeping intentional blanks) - HTML import: 'does not introduce a blank line between H1 and H2', 'preserves blank-line count between H1 and H2 (realistic shape)' reproduces the 5-blanks-where-2-expected bug from the user's round-trip pad. 1054 backend tests pass locally (the 6 failures are the pre-existing favicon/webaccess send@1.x dotfile-path issue from running under .claude/, doesn't reach CI). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(7538): skip plugin-dependent HTML import tests on no-plugin CI The two new HTML-import-adjacency tests assume ep_headings2 (or another plugin) has registered h1/h2 as server-side block elements via ccRegisterBlockElements. Without that, contentcollector treats <h1>/<h2> as inline and adjacent ones merge into a single pad line -- making the assertions inapplicable. CI's backend-tests job runs without plugins installed, so guard the describe block with a runtime hooks.callAll() check and skip when h1 isn't a registered block. Local dev with ep_headings2 (and the local plugin patch wiring ccRegisterBlockElements) still exercises both tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-08 18:33:50 +01:00
John McLear	afee7969dd	fix(7696): scrollable settings popup on short viewports (#7703 ) * fix(7696): make settings popup scroll on short viewports Move max-height + overflow:auto out of the mobile-only media query and onto the base .popup-content rule so the Settings popup (and other popups) gain a scrollbar instead of cropping items off-screen when the window is short. Pad-wide Settings is the worst offender because it adds a second column of controls plus a Delete pad button. Adds a Playwright regression test that verifies the popup is scrollable and the Delete pad button is reachable at a 900x500 viewport. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7696): float nice-select dropdowns above scrollable popups Qodo flagged that making .popup-content a scroll container clips absolutely-positioned descendants — so the Settings popup's font and language dropdowns can be truncated when their list extends past the popup's scroll bounds on short viewports. Mirror the existing toolbar workaround: when a nice-select sits inside .popup-content, switch the list to position:fixed (CSS) and place it with viewport-relative coordinates from getBoundingClientRect (JS), respecting the existing reverse class for upward-opening lists. Also relax the regression test per Qodo: drop the brittle scrollHeight > clientHeight assertion in favour of asserting the popup declares overflow-y:auto and proving Delete pad is initially off-screen, then reachable via scroll. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7696): nice-select reverse list disappeared in scrolled popup When a nice-select inside a popup-content scroll container sits in the lower half of the viewport, the JS adds the .reverse class so the list opens upward. The default .reverse rule sets bottom: calc(100% + 5px), which is fine when the list is position:absolute relative to its parent — but with the position:fixed treatment the popup branch uses, that percentage resolves against the viewport and pushes the list ~100vh above the screen, so it appears not to open at all until you scroll to the bottom of the popup (where .reverse no longer triggers). Override the rule for both .toolbar and .popup so .reverse drops back to bottom: auto and JS-set `top` controls placement, with a JS belt-and- braces also setting `bottom: auto` inline. Adds a Playwright regression test that scrolls the settings popup to the bottom, opens the Pad-wide font dropdown, and asserts the list is both visible and inside the viewport. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-08 03:29:09 +08:00
John McLear	85c941fe95	feat(padOptions): pass plugin-namespaced ep_* keys through applyPadSettings (#7698 ) * feat(padOptions): pass plugin-namespaced ep_* keys through applyPadSettings Native pad-wide settings ride a single padOptions object: the server seeds clientVars.initialOptions, the client mutates via pad.changePadOption(), and the existing padoptions COLLABROOM message broadcasts changes. Plugins can't use the same rail today because applyPadSettings (client) and normalizePadSettings (server) silently drop any key not in their hardcoded whitelist. Add a passthrough loop that preserves keys matching /^ep_[a-z0-9_]+$/ on both sides. Plugins can now stash their pad-wide values under their own namespace (e.g. pad.padOptions.ep_table_of_contents = {enabled: true}) and inherit the existing broadcast, persistence, creator-only-write enforcement, and enforceSettings semantics for free. A new src/node/utils/PluginCapabilities module exposes padOptionsPluginPassthrough = true so plugins can feature-detect via require() and fall back to per-user behavior on older cores. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address Qodo review on PR #7698 Four concerns raised by Qodo (qodo-free-for-open-source-projects): 1. Feature flag — AGENTS.MD §52 requires new features behind a flag, disabled by default. Add `enablePluginPadOptions` (default false) gating the passthrough on both server (normalizePadSettings) and client (applyPadSettings, via clientVars). Plugins detect the runtime state through clientVars.enablePluginPadOptions; the static PluginCapabilities flag stays as the "core can do this" signal. 2. Documentation — add a "Plugin-namespaced pad-wide options" section to doc/plugins.md covering capability detection, the runtime flag, the key namespace pattern, and the validation rules. Mirror the flag description in settings.json.template. 3. Unbounded payload — values for ep_* keys are persisted with the pad and broadcast to every connected client, so an unvalidated path was a reliability hazard. Validate every ep_* value: - Must round-trip through JSON.stringify (rejects functions, symbols, BigInt, circular refs). - Per-key serialized size capped at 64 KB. - Combined ep_* size capped at 256 KB per pad. Rejects drop the value with a console.warn line; the rest of the pad settings round-trip cleanly. 4. PadOption type — add `[k: \`ep_${string}\`]: unknown` index signature so the SocketIO message type matches runtime behavior; TS callers no longer need unsafe casts to read plugin-namespaced keys. Also extends the backend test suite with cases covering the runtime flag (off/on), JSON-serializability rejection, per-key cap, and total cap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(snap-tests): assert_grep — use here-string to dodge pipefail SIGPIPE `assert_grep` ran `printf '%s' "$out" \| grep -q -F -- "$needle"` under `set -o pipefail`. When grep matched early it closed its stdin, printf got SIGPIPE on its next write (exit 141), and pipefail propagated the broken-pipe failure to the pipeline — making `if` see non-zero and falling into the FAIL branch even though grep itself succeeded. Failure was timing-dependent: it only fired when `$out` was large enough that printf hadn't flushed before grep exited. CI ubuntu-latest tipped into the racy path on PR #7698 once `settings.json.template` grew by 11 lines (the new `enablePluginPadOptions` flag); the symptom was the `Wrapper unit tests` step reporting `dbType rewritten to sqlite ✗` with "got: /*…" output even though the seeded file did contain the needle. Replace the pipe with a here-string so grep gets its input in one shot with no pipe between processes — no SIGPIPE possible. The fail-message `head -3` is converted to a here-string for the same reason. Repro on a runner whose pipe-buffer flush is slower than grep's first match would have hit the same flake on any PR; the bug isn't about this particular template change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-07 17:17:05 +01:00
John McLear	ab0cff4d95	fix(7606): sync theme-color meta with client-side dark-mode switch (#7690 ) * fix(7606): sync theme-color meta with client-side dark-mode switch PR #7636 emits <meta name="theme-color"> server-side from settings.skinVariants. That covers operators who hard-code a dark toolbar in settings.json, but not the runtime path: pad.ts auto-flips the toolbar to super-dark when enableDarkMode is on, the browser reports prefers-color-scheme: dark, and no localStorage white-mode override is set, plus the user can flip it via #options-darkmode. Both paths run skinVariants.updateSkinVariantsClasses(), which until now never touched the meta — so dark-mode users kept the light #ffffff baseline and saw a white address bar above a dark toolbar (stffen on #7606 after 2.7.3). Push the toolbar-color lookup into updateSkinVariantsClasses so the meta tracks every class change: the auto-switch on init, the user toggle, and the skinVariants builder. Mirrors the CSS-source-order table from src/node/utils/SkinColors.ts (last matching -toolbar token wins). When no meta is present (non-colibris skin, server omits it) the helper is a no-op. Adds Playwright coverage for both paths under colorScheme: 'light' (manual toggle) and 'dark' (auto-switch on dark-OS clients — the case stffen reported). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> fix(theme-color): action Qodo PR review (1) Bug — duplicated toolbar→color table: extract the CSS-source-order mapping and the default-color constant into src/static/js/skin_toolbar_colors, re-imported by both src/node/utils/SkinColors.ts (server, EJS template helper) and src/static/js/skin_variants.ts (client, runtime updates). Lives under static/js so the browser bundle can resolve it; server-side imports of static/js modules already exist (Changeset, AttributeMap, ImportHtml, hooks). One source of truth means a future palette change can no longer silently desync the server-rendered baseline meta from the client updates, which was the exact regression that brought us here. (2) Rule violation — 4-space continuation indentation in the new Playwright spec: re-indent the themeColor helper and the multiline test(...) call to the repo's 2-space rule (.editorconfig). Existing backend coverage (configuredToolbarColor unit tests + the specialpages server-render checks for both pad and timeslider) still passes against the refactored helper, so it's regression-locked end to end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-07 10:43:29 +01:00
John McLear	da87a4fca2	fix(7686): username 'false' / 'malformed color: false' for legacy settings.json (#7688 ) * fix(7686): legacy padOptions.userName/userColor=false breaks pad Settings.json files generated before December 2021 used `false` as the default for these two string options (commit `8c857a85a` switched the template default to `null` and noted "this change has no effect due to a bug in how pad options are processed; that bug will be fixed in a future commit" — the follow-up never landed). pad.ts:getParams() then runs `false.toString()`, the resulting string "false" passes the `!== false` sentinel check at _afterHandshake, and notifyChangeName ships USERINFO_UPDATE with name="false" and colorId="false" (clobbered via clientVars.userColor). The server's hex regex rejects the colour and throws `malformed color: false`; the user sees their name as "false" and a white swatch. Defense in depth: - Server: Settings.ts::reloadSettings() coerces legacy boolean false to null for padOptions.userName / padOptions.userColor and warns the operator, matching the existing disableIPlogging shim pattern. - Client: the pad.ts userName / userColor callbacks reject the literal "false" string so URL params (?userName=false) and any other path that surfaces the sentinel as a string are also no-ops. - Backend regression test mirrors the shim and asserts it normalizes legacy false, leaves explicit values intact, leaves null untouched, does not coerce other padOptions keys, and does not coerce the string "false" (that path is the client guard's responsibility). Closes #7686 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7686): guard padOptions shim against non-object config (Qodo) Qodo flagged that storeSettings() will overwrite settings.padOptions raw with whatever settings.json supplies — including null, primitives, or arrays — which would make the new userName/userColor shim crash on property access. Add a shape guard so the shim is a no-op for malformed padOptions, and extend the regression test to cover null / primitive / array shapes. This doesn't change which configs work (Pad.ts also assumes padOptions is an object and would already crash on a null padOptions when a pad is opened) but it stops the shim from being the loud thing in the stack trace if someone hits it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-07 09:32:17 +01:00
SamTV12345	bfdbd2bb91	chore: removed axios (#7685 ) * chore: removed axios * chore: pnpm	2026-05-06 22:06:04 +02:00
John McLear	487842006c	feat(gdpr): configurable privacy banner (PR4 of #6701 ) (#7549 ) * docs: PR4 GDPR privacy banner design spec * docs: PR4 GDPR privacy banner implementation plan * feat(gdpr): typed privacyBanner setting block + public getter exposure * feat(gdpr): send privacyBanner config to the browser via clientVars * feat(gdpr): privacy banner DOM (hidden by default) * feat(gdpr): render privacy banner on pad load when enabled * style(gdpr): privacy banner layout * test+fix(gdpr): privacy banner Playwright + hidden-attr CSS override * docs(gdpr): privacyBanner configuration section * fix(gdpr): reject unsafe learnMoreUrl schemes Qodo review: showPrivacyBannerIfEnabled assigned config.learnMoreUrl directly to <a href>, so a misconfigured settings.privacyBanner. learnMoreUrl of `javascript:alert(1)` or `data:…<script>…` would run script on click. Validate via URL parsing and allow only http(s) / mailto; everything else yields no link. Playwright regression guards the four cases (javascript, data, https, mailto). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(privacy-banner): drop unneeded !important on [hidden] rule Class+attribute selector already outranks `.privacy-banner { display: flex }` on specificity (0,2,0 vs 0,1,0), so `!important` was redundant. Adds a comment explaining why so a future reader doesn't put it back. Per Sam's review on #7549. * refactor(privacy-banner): render as a persistent gritter, not custom DOM Drops the bespoke #privacy-banner template + ~50 lines of popup.css and delegates to $.gritter.add({sticky: true, position: 'bottom'}). The notice now matches every other gritter on the pad (theme variables, shadow, animation, (X) close), sits in the bottom corner instead of above the editor, and inherits dark-mode handling for free. The two dismissal modes survive intact: - dismissible: gritter closes on (X); before_close persists a flag in localStorage so the notice is suppressed on subsequent loads. - sticky: closes for the current session only; never persists; the next pad load shows it again. learnMoreUrl still goes through the same safeUrl() filter so a javascript:/data:/vbscript: URL can't smuggle a script handler into the anchor (Qodo's review concern remains addressed). Tests: src/tests/frontend-new/specs/privacy_banner.spec.ts now drives the real showPrivacyBannerIfEnabled via a __etherpad_privacyBanner__ test hook and asserts against the rendered gritter, instead of the previous tests that mutated DOM by hand and never exercised the function under test. Coverage adds: enabled=false short-circuit, dismissible-flag-respected on subsequent show, sticky-ignores-flag, sticky-close-does-not-persist, javascript: rejection, data: rejection, and mailto: allow-list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(privacy-banner): noreferrer + validate dismissal (Qodo) Two follow-ups from Qodo's review on #7549: 1. The Learn-more link now sets `rel="noreferrer noopener"` (was just `noopener`). Without `noreferrer` the browser sends the pad URL as a Referer to the operator-configured external policy site, which leaks pad identifiers to a third party. Matches the rel pattern already used by pad_utils.ts. 2. `privacyBanner.dismissal` is now validated in reloadSettings(): an unknown value falls back to 'dismissible' with a `logger.warn`, in the same shape as the existing ipLogging validation a few lines up. The client also guards defensively (treats anything other than the exact string 'sticky' as 'dismissible') so that hot-reload paths that skip the server validator can't silently degrade a typo'd 'sticky' into "no close button persisted, no localStorage suppression". Test added: spec asserts the rel attribute, and a new test exercises the dismissal fallback (sets dismissal:'wat', asserts the gritter is shown, the (X) closes it, and the dismissal flag is persisted — i.e. the unknown value is treated like 'dismissible'). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(privacy-banner): gate test hook on webdriver, align doc with sticky behavior Two follow-ups from Qodo's second review on #7549. Rule violation: __etherpad_privacyBanner__ was published on every pad load even when privacyBanner.enabled was false, so the disabled-by- default feature still added an observable global. Gate the assignment on `navigator.webdriver` — Playwright/ChromeDriver/Selenium set this to true; production browsers do not — so the hook is only present for tests and the disabled path is genuinely zero-side-effect. Bug 3 (sticky still closable): doc/privacy.md previously claimed `dismissal: "sticky"` removes the close button, but the gritter implementation always renders (X). Aligning the doc with reality — sticky now means "shows on every load, but closable for the session" — rather than adding bespoke CSS to a vanilla gritter (matches the "don't style it differently than other gritter messages" preference that drove the gritter migration in `906e145`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(privacy-banner): allow-list keys before sending to clientVars (Qodo) storeSettings() merges nested objects with _.defaults() and preserves unknown nested keys, and TypeScript's Pick<> doesn't strip at runtime. The previous wire path forwarded settings.privacyBanner by reference into both clientVars and getPublicSettings(), so any extra keys an operator typed (or pasted) under privacyBanner — credentials, internal notes, anything — would have shipped to every browser on every pad load. Adds getPublicPrivacyBanner() in Settings.ts that returns a literal with only {enabled, title, body, learnMoreUrl, dismissal}, and uses it from both leak sites (PadMessageHandler.ts clientVars and getPublicSettings()). Single source of truth for the wire shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-03 13:59:38 +08:00
John McLear	49bc33f019	feat(gdpr): HttpOnly author-token cookie (PR3 of #6701 ) (#7548 ) * docs: PR3 GDPR anonymous identity hardening design spec * docs: PR3 GDPR anon identity implementation plan * feat(gdpr): ensureAuthorTokenCookie helper — HttpOnly server-set author token * feat(gdpr): set HttpOnly author-token cookie from the pad routes * feat(gdpr): read author token from cookie first, keep message.token fallback * feat(gdpr): stop generating the author token client-side * test(gdpr): server sets + reuses the HttpOnly author-token cookie * fix+test(gdpr): parse token cookie from handshake Cookie header socket.io handshake doesn't run cookie-parser, so socket.request.cookies is undefined. Parse the Cookie header directly in handleClientReady so the HttpOnly token actually resolves. Playwright spec covers HttpOnly attribute, reload-stability, and context-isolation. * docs(gdpr): token cookie is now HttpOnly + server-set * fix(gdpr): close two HttpOnly token bypasses Qodo review: - Timeslider still ran the pre-PR3 JS-cookie path: it read Cookies.get('${cp}token') (which HttpOnly hides), then generated a fresh plaintext token and overwrote the server's HttpOnly cookie with it, and sent token in every socket message. Strip the token read/ write entirely from timeslider.ts and from the outgoing message shape; the server reads the cookie off the socket.io handshake just like on /p/:pad. - tokenTransfer re-issued the author cookie without HttpOnly, undoing the hardening the first time a user transferred a session. Re-set it as HttpOnly + Secure (on HTTPS) + SameSite=Lax. Also stop trusting the body-supplied token on POST: read it off req.cookies server-side so the client never needs JS access to the token. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-03 05:56:56 +01:00
John McLear	9014d3a7c4	fix(colors): pick WCAG-higher-contrast text for author colors (#7565 ) * feat(colors): clamp author backgrounds to WCAG 2.1 AA on render Fixes #7377. Authors can pick any color via the color picker, so a user who chooses a dark red ends up with black text rendered on a background that fails WCAG 2.1 AA (4.5:1) — unreadable, but there is no way for viewers to remediate since they cannot change another author's color. Screenshot in the issue shows exactly this. This PR lands a viewer-side clamp. For each author background, if neither black nor white text would satisfy the target contrast ratio, the bg is iteratively blended toward white until black text does. The author's stored color is untouched — turning off the new padOptions.enforceReadableAuthorColors flag restores the raw colors immediately. New helpers in src/static/js/colorutils.ts: - relativeLuminance(triple) — WCAG 2.1 relative-luminance formula - contrastRatio(c1, c2) — in [1, 21]; >=4.5 = AA, >=7.0 = AAA - ensureReadableBackground(hex, minContrast = 4.5) — returns a hex that meets minContrast against black text, preserving hue Wire-up: - src/static/js/ace2_inner.ts (setAuthorStyle): pass bgcolor through ensureReadableBackground before picking text color. Gated on padOptions.enforceReadableAuthorColors (default true). Guarded by colorutils.isCssHex so the few non-hex values (CSS vars, etc.) skip the clamp and pass through unchanged. - Settings.ts / settings.json.template / settings.json.docker: new padOptions.enforceReadableAuthorColors flag, default true, with a matching PAD_OPTIONS_ENFORCE_READABLE_AUTHOR_COLORS env var in the docker template. - doc/docker.md: env-var row. - src/tests/backend/specs/colorutils.ts: new unit coverage for the three new helpers, including the exact #cc0000 failure case from the issue screenshot. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(7377): simplify — just pick higher-contrast text, drop bg clamp First iteration added an iterative bg-lightening helper (ensureReadableBackground) gated by a new padOptions flag. CI caught the correct simpler framing: because WCAG contrast is symmetric in [1, 21], at least one of black/white always clears AA (4.5:1) for any sRGB colour. The real bug was that the pre-fix textColorFromBackgroundColor used a plain-luminosity cutoff (< 0.5 → white), which produced sub-AA combinations like white-on-red (#ff0000) at 4.0:1. Reduce the PR to the minimal surface: - colorutils.textColorFromBackgroundColor now picks whichever of black/white has the higher WCAG contrast ratio against the bg. - colorutils.relativeLuminance and colorutils.contrastRatio are kept as reusable building blocks; ensureReadableBackground is dropped (no caller needed it once text selection was fixed). - ace2_inner.ts setAuthorStyle no longer needs the opt-in flag or the isCssHex guard — the helper handles every input its caller already passes. - padOptions.enforceReadableAuthorColors setting reverted along with settings.json.template, settings.json.docker, and doc/docker.md. - Tests replaced: instead of asserting the bg gets lightened, assert that the chosen text colour clears AA for every primary. Covers the exact #ff0000 failure case from the issue screenshot. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(7377): assert relative-contrast invariant, not absolute AA Pure primaries like #ff0000 cannot clear WCAG AA (4.5:1) against either #222 or #fff — the best either can do is ~4.0:1. No text-colour choice alone fixes that; bg clamping would be a separate concern. The test should therefore verify the real invariant: the chosen text colour must produce the higher contrast of the two options, regardless of whether that contrast clears any absolute threshold. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7377): compare against rendered #222/#fff, not pure black/white First cut of textColorFromBackgroundColor computed contrast against pure black (L=0) and pure white (L=1), then returned the concrete #222/#fff the pad actually renders with. For some mid-saturation backgrounds the two comparisons disagreed — e.g. #ff0000: vs pure black = 5.25 → pick black → render #222 → actual 3.98 vs pure white = 4.00 → would-render #fff → actual 4.00 The helper picked the wrong option because it compared against the wrong target. Compare against the actual rendered colours so the returned text colour is genuinely the higher-contrast choice. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(7377): pick unambiguous colibris test bgs #ff0000 lives right at the boundary for the two text choices (4.00 vs 3.98), so the test for colibris-skin mapping was entangled with the border-case selector pick. Use #ffeedd (clearly light → dark text wins) and #111111 (clearly dark → light text wins) so the test isolates the skin mapping from the tie-breaking logic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(7377): use rendered text colour + clamp bg to actually meet AA Local repro of the issue exposed two real bugs in the previous fix: 1. textColorFromBackgroundColor compared bg against a hardcoded #222 — but in the colibris skin --super-dark-color resolves to #485365. For the issue's exact case (#9AB3FA author bg) the selector returned var(--super-dark-color) thinking it was getting a 7.7:1 ratio, while the browser actually rendered 3.78:1 — identical to what the issue screenshot reported. This PR's previous behaviour on the issue's inputs was unchanged from the pre-fix. 2. For mid-saturation pastels (#9AB3FA) and pure primaries (#ff0000) neither rendered dark nor white text can clear AA. Text-colour selection alone genuinely cannot fix this band; the ensureReadable bg clamp dropped in `ce0c5c283` was load-bearing. Changes: - colorutils.ts: per-skin SKIN_TEXT_COLORS table with darkRef/lightRef matching what the browser actually paints (colibris #485365, default #222). Re-introduces ensureReadableBackground, but skin-aware and symmetric — blends bg toward white or black depending on which text colour wins, so it works for both light and dark backgrounds. - ace2_inner.ts: setAuthorStyle runs the bg through the clamp before picking text colour. Gated on padOptions.enforceReadableAuthorColors (default true). - Settings.ts / settings.json.template / settings.json.docker / doc/docker.md: padOption + PAD_OPTIONS_ENFORCE_READABLE_AUTHOR_COLORS env var. - tests: failing-then-green coverage for the issue's exact case (#9AB3FA + colibris), the previously-impossible #ff0000, the no-mutation case, non-hex pass-through, and a sweep over primaries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(7377): add e2e DOM-contrast spec + extra unit cases The previous coverage was unit-only, which is what let the original wrong- reference-colour bug ship — the algorithm tests were green but nothing exercised what the browser actually paints. New coverage: Playwright (src/tests/frontend-new/specs/wcag_author_color.spec.ts): - Sets the user's colour to the issue's exact #9AB3FA, types text, reads the rendered author span's computed bg + colour from the inner frame, and asserts the WCAG ratio between the two is >= 4.5. Repeated for #ff0000 (the other historically-failing case). - Asserts #ffeedd (already AA-friendly) is rendered unchanged — guards against the clamp mutating colours that don't need it. Backend additions (src/tests/backend/specs/colorutils.ts): - Symmetric-clamp test: dark mid-saturation bg where light text wins, the clamp must darken (not lighten). Direction check via relativeLuminance. - minContrast parameter: AAA (7.0) must produce more clamping than AA. - Output shape: result must be a parseable hex string (round-trip safe). - Short-hex (#abc) input is accepted and normalised. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-03 12:56:30 +08:00
John McLear	25c43140bc	feat(userlist): click a user to open chat with @<name> prefilled (#7660 ) * feat(userlist): click a user to open chat with @<name> prefilled Newcomers to a multi-user pad regularly fail to discover the chat panel and the @-mention convention. Make the user list itself the discovery affordance: clicking another user's row opens chat (if hidden) and prefills the input with "@<their_name> ", ready to send. The skin gets a small visual cue — pointer cursor on .usertdname and an underline on hover — so the affordance is visible without requiring a redesign. The color swatch keeps its own click semantics (color picker), so the swatch cell is excluded from the new handler. To let bot/AI plugins substitute their trigger string for an otherwise-useless @-mention of the bot's display name (e.g. "@AI Assistant" → "@ai"), this adds a new client-side hook, chatPrefillFromUser, that takes {authorId, name, prefill} and lets the first plugin to return a non-empty string override the default prefill. Documented in doc/api/hooks_client-side.md alongside chatSendMessage. Plugin errors in the hook are caught — a misbehaving plugin can't break the click. If chat is hidden by pad settings, chat.show() is a no-op and the click effectively does nothing, which matches the existing behavior of "no chat means no chat-related affordances". The new prefill never clobbers a real partial message in the input; if the user was mid-typing something, the @-mention is appended rather than replacing. * fix(userlist): don't steal rename focus + add Playwright coverage Two follow-ups on review of the click-to-chat handler: 1. Bug (Qodo, correctness): clicking the rename <input> on an unnamed user's row triggered the new row handler, which then focused #chatinput and made it impossible to name unnamed users from the user list. Add an early-return that skips form controls inside the row (input/textarea/select/button/a/[contenteditable=true]). The swatch was already excluded; this widens the same idea to anything that's interactive on its own merits. 2. Test coverage: add a frontend Playwright spec (userlist_click_to_chat.spec.ts) covering the supported flows and the new regression: - clicking another named user opens chat and prefills "@<name> " - clicking the swatch opens color picker, not chat - clicking the rename <input> on an unnamed user keeps focus on the input (regression test for the bug above) - partial chat message is preserved when prefilling * test: stabilise the partial-message preservation case The 'partial message in chat input is preserved when prefilling' case was flaking on CI. Three small changes: - Seed the chat input with fill() rather than click() + keyboard.type(). Earlier the test was racing chat.focus()'s own setTimeout(100) — when the keyboard.type started before that timer fired, the typing landed in whatever element had focus at the time, which wasn't always the chat input. fill() bypasses focus state entirely. - Wait for the chat box to be visible before filling, so we don't race the chaticon click handler. - Replace the two sequential expect/wait pairs after the daveRow click with one waitForFunction that asserts both 'hi there' and '@Dave' are in the input together. The prefill is async (setTimeout(50) inside the click handler), so a combined wait is more reliable than checking one piece, then snapshotting and asserting the other. The other three cases in this file passed unchanged on CI; only this fourth one was racy. * fix: don't commit local .claude worktrees / var state These were accidentally added in `ffe947706` by an over-broad git add -A. Both paths are workspace-local and unrelated to this PR.	2026-05-03 05:40:44 +01:00
John McLear	e0a989094d	feat(colors): add padOptions.fadeInactiveAuthorColors with toolbar UI (#7554 ) Adds a new pad option `fadeInactiveAuthorColors` (default `true`) that controls whether each author's caret/background fades toward white as they go inactive. Configurable server-side (`settings.json` / `PAD_OPTIONS_FADE_INACTIVE_AUTHOR_COLORS`), per-pad in the Pad Settings panel, per-user in the My View panel, or via `?fadeInactiveAuthorColors=false`. Disabling the fade is useful on busy pads where every faded author visually counts as a second on-screen color (a 30-author pad becomes a 60-color pad), or when inactivity tracking is undesirable for whatever reason. Closes #7138.	2026-05-03 04:01:13 +08:00
John McLear	5e8704f8d8	feat(gdpr): pad deletion controls (PR1 of #6701 ) (#7546 ) * docs: PR1 GDPR deletion-controls design spec First of five GDPR PRs tracked in #6701. PR1 covers deletion controls: one-time deletion token, allowPadDeletionByAllUsers flag, authorisation matrix for handlePadDelete and the REST deletePad endpoint, a single token-display modal for browser pad creators, and test coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: PR1 GDPR deletion-controls implementation plan 13 TDD-structured tasks covering PadDeletionManager unit tests, socket + REST three-way auth, clientVars wiring, one-time token modal, delete-with-token UI, Playwright coverage, and PR handoff. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(gdpr): scaffolding for pad deletion tokens PadDeletionManager stores a sha256-hashed per-pad deletion token and verifies it with timing-safe comparison. createPad / createGroupPad return the plaintext token once on first creation, and Pad.remove() cleans it up. Gated behind the new allowPadDeletionByAllUsers flag which defaults to false to preserve existing behaviour. Part of #6701 (GDPR PR1). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix+test(gdpr): lazy DB access in PadDeletionManager + unit tests Capturing DB.db at module-load time was null until DB.init() ran, which broke importing the module outside a live server (including from the test runner). Switch to DB.db.* at call time and add unit tests exercising create/verify/remove plus timing-safe comparison. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(gdpr): three-way auth for socket PAD_DELETE Creator cookie → valid deletion token → allowPadDeletionByAllUsers flag. Anyone else still gets the existing refusal shout. * feat(gdpr): optional deletionToken on programmatic deletePad * feat(gdpr): advertise optional deletionToken on REST deletePad * test(gdpr): cover deletePad authorisation matrix via REST * feat(gdpr): surface padDeletionToken in clientVars for creators only Revision-0 author on their first CLIENT_READY visit receives the plaintext token; all subsequent CLIENT_READYs receive null because createDeletionTokenIfAbsent is idempotent. Readonly sessions and any other user never see the token. * i18n(gdpr): strings for deletion-token modal and delete-with-token flow * feat(gdpr): token modal + delete-with-token disclosure markup * feat(gdpr): show deletion token once, allow delete via recovery token * style(gdpr): modal + delete-with-token layout * test(gdpr): Playwright coverage for deletion-token modal + delete-with-token * fix(test): auto-dismiss deletion-token modal in goToNewPad helper The token modal introduced in PR1 blocks clicks for every Playwright test that creates a new pad via the shared helper. Add a one-line dismissal so unrelated tests keep passing, and have the deletion-token spec navigate inline via newPadKeepingModal() when it needs the modal open to capture the token. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): dismiss deletion-token modal without focus transfer Clicking the ack button transferred focus out of the pad iframe, which made subsequent keyboard-driven tests (Tab / Enter) silently miss the editor. Swap the click for a page.evaluate() that hides the modal and nulls clientVars.padDeletionToken directly, leaving focus where it was. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(gdpr): PadDeletionManager race + document createPad/deletePad Qodo review: - createDeletionTokenIfAbsent() was a non-atomic read-then-write. Two concurrent callers for the same pad could both return different plaintext tokens while only the later hash was stored, leaving the first caller with an unusable recovery token. Serialise per-pad via a Promise chain and add a regression test that fires 8 concurrent calls and asserts exactly one plaintext is emitted and validates. - doc/api/http_api.md now documents createPad returning deletionToken and deletePad accepting the optional deletionToken parameter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(gdpr): always render delete-with-token in settings popup The rebase onto develop placed the delete-pad-with-token details inside the pad-settings-section conditional, which is only rendered when enablePadWideSettings is true AND the section is toggled visible. Second-device recovery (typing the captured token on a fresh browser) must work without pad-wide settings enabled, so move the details out to sit alongside the existing pad_deletion_token.spec.ts expectations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(gdpr): require valid token when supplied, gate on auth, harden a11y/i18n - PadMessageHandler: a supplied deletion token must validate; do not fall back to the creator-cookie path when the token is wrong (was deleting the pad anyway when the creator pasted a wrong token into the field). - Skip token issuance + UI when requireAuthentication is on (creator identity is stable, recovery token is redundant noise). - Server emits messageKey instead of hardcoded English; both shout handlers (inline alert and global gritter) localize via html10n. - Suppress the global "Admin message" gritter for pad.deletionToken.* shouts to avoid the "Admin message: undefined" duplicate. - Token-modal a11y: role=dialog, aria-modal, aria-labelledby/describedby, visually-hidden label on the token input, aria-live on Copy, focus to the token input on open and restore on dismiss. - Style the "Delete Pad with Token" disclosure to match the Delete pad button; align the Copy/value row; pad the disclosure label. Tests: Playwright now covers the creator-with-wrong-token path, asserts no "Admin message" / "undefined" gritter on denial; backend API test covers requireAuthentication suppressing the token. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-01 13:50:04 +01:00
John McLear	6195289198	feat(gdpr): IP/privacy audit (PR2 of #6701 ) (#7547 ) * docs: PR2 GDPR IP/privacy audit design spec Second of five GDPR PRs (#6701). Audit identifies four log-sites that leak IPs despite disableIPlogging=true, proposes a tri-state ipLogging setting with a back-compat shim, and specifies a doc/privacy.md that documents Etherpad's actual IP handling. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: PR2 GDPR IP/privacy audit implementation plan 7 TDD-structured tasks: anonymizeIp helper + unit tests, tri-state ipLogging setting with disableIPlogging deprecation shim, wiring through 5 leaking log sites, clientVars.clientIp removal, access-log integration test, doc/privacy.md, and PR handoff. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(gdpr): anonymizeIp helper with v4/v6/v4-mapped truncation * feat(gdpr): tri-state ipLogging setting + disableIPlogging shim * fix(gdpr): route every IP log site through anonymizeIp Closes four leaks where disableIPlogging was silently ignored (rate-limit warn, both auth-log calls in webaccess, import/export rate-limit warn) and normalises the four that did honour the flag onto the new ipLogging tri-state via the shared helper. * chore(gdpr): drop dead clientVars.clientIp placeholder Server side: remove the literal '127.0.0.1' assignments from both clientVars and collab_client_vars. Type side: drop clientIp from ClientVarPayload and ServerVar. pad.getClientIp now returns the same '127.0.0.1' literal as a plugin-compat shim (pad_utils.uniqueId still uses it as a prefix). * test(gdpr): ipLogging modes + disableIPlogging shim * docs(gdpr): operator-facing privacy and IP handling statement * fix(gdpr): validate ipLogging at load + regression test for log sites Qodo review: - settings.ipLogging is loaded as a trusted union but nothing enforced the shape. An unknown value (e.g. a typo or null) silently fell through to anonymizeIp's "truncated" branch and emitted partially redacted IPs. Fall back to "anonymous" with a WARN at load time. - New regression test scans the four known log-sites for raw req.ip / socket.request.ip / request.ip inside logger calls that don't wrap through anonymizeIp / logIp, so a future edit that re-introduces a raw IP fails CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-01 13:47:40 +01:00
John McLear	e39dbde887	feat(updater): tier 1 — notify admin and pad users of available updates (#7601 ) * docs(updater): add four-tier auto-update design spec Four-tier opt-in self-update subsystem (off / notify / manual / auto / autonomous). GitHub Releases as source of truth; install-method auto-detection with admin override; in-process execution with supervisor restart; 60s drain + announce; auto-rollback on health-check failure with crash-loop guard. Pad-side severe/ vulnerable badge that does not leak the running version. Top-level adminEmail with escalating cadence (weekly while vulnerable, monthly while severe). Refs: docs/superpowers/specs/2026-04-25-auto-update-design.md * docs(updater): add PR 1 (Tier 1 notify) implementation plan Bite-sized TDD task breakdown for shipping Tier 1 notify only: - VersionChecker, InstallMethodDetector, UpdatePolicy, Notifier, state modules - /admin/update/status (admin-auth) and /api/version-status (public, no version leak) - Admin UI banner + read-only update page + nav link - Pad-side severe/vulnerable footer badge - Settings: updates.* block + top-level adminEmail - Tests: vitest unit + mocha integration + Playwright admin/pad - CHANGELOG + doc/admin/updates.md PRs 2-4 (manual/auto/autonomous) get their own plans after PR 1 lands. * feat(updater): add shared types for auto-update subsystem * feat(updater): clarify OutdatedLevel and EMPTY_STATE doc, drop path header Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(updater): add semver helpers and vulnerable-below parser Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(updater): tighten semver regex to reject four-part versions * feat(updater): add state persistence with schema validation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(updater): reject null email and array latest in state validation typeof null === 'object' meant {email:null} passed the old isValid check, which would crash downstream Notifier code reading email.severeAt. Likewise, an array would pass the typeof latest === 'object' branch. Introduce isPlainObject helper (null-safe, Array.isArray guard) and use it for both fields. Adds two regression tests covering the exact broken inputs. * feat(updater): add install-method detector with override * feat(updater): add policy evaluator * feat(updater): add GitHub Releases checker with ETag support Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(updater): validate release fields and preserve ETag on prerelease Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(updater): add email cadence decider Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(updater): tagChanged email fires regardless of cadence; drop unused field * feat(settings): add updates.* and adminEmail settings Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(updater): wire boot hook and periodic checker Register expressCreateServer/shutdown hooks in ep.json and implement the boot-wiring module that detects install method, starts the polling interval and runs the notifier dedupe pass each tick. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(updater): add /admin/update/status and /api/version-status endpoints Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * i18n(updater): add english strings for update banner, page, and pad badge * feat(updater): add pad footer badge for severe/vulnerable status * feat(admin-ui): add update banner, page, and nav link Add UpdateStatusPayload to the zustand store, a persistent UpdateBanner rendered in the App layout, a /update page showing version details and changelog, and a Bell nav link — all wired to the /admin/update/status endpoint added in Task 10. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(updater): add Playwright specs for admin banner/page and pad badge * docs(updater): document tier 1 settings, badge, email cadence * refactor(updater): dedupe helpers, fix misleading log, add banner styling - Export stateFilePath from index.ts and import it in updateStatus.ts (removes local duplicate) - Import getEpVersion from Settings.ts in both index.ts and updateStatus.ts (removes two local definitions) - Fix misleading 'backing off' log message — no backoff is implemented, just retries at next interval - Remove EMPTY_STATE_FOR_TESTS re-export from state.ts; state.test.ts now imports EMPTY_STATE directly from types.ts - Add .update-banner and .update-page CSS rules to admin/src/index.css Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(updater): address review feedback — async wrap, tier=off skip, poll race, opt-in admin gate - Wrap /api/version-status and /admin/update/status with a small async helper so a rejected promise becomes next(err) instead of an unhandled rejection. - Short-circuit route registration when updates.tier === 'off' so the heavier opt-out also removes the HTTP surface (matches pre-PR behavior for that case). - Add an in-flight guard around performCheck() so overlapping interval ticks can't race on update-state.json writes or duplicate email decisions; track the initial setTimeout handle and clear it in shutdown(). - Add updates.requireAdminForStatus (default false) so admins can lock /admin/update/status to authenticated admin sessions without disabling the updater. Default false preserves current behavior (the running version is already exposed publicly via /health). Backend specs cover unauth → 401, non-admin → 403, admin → 200. - Bump admin troubleshooting menu count test 5 → 6 to account for the new Update nav link. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(updater): address Qodo round-2 review feedback Round 2 of Qodo review on #7601. Addressing the action-required items: #1 Badge bypassed pad baseURL — derive basePath the same way padBootstrap.js does (`new URL('..', window.location.href).pathname`) and prefix the fetch with it. Subpath deployments now reach /<prefix>/api/version-status instead of 404ing. #2 Updater poller could get stuck — `getCurrentState()` is now inside the try/finally so a one-time loadState() rejection can't leave `checkInFlight=true` and permanently silence polling. #3 Updates off hung admin page — UpdatePage now self-fetches and renders explicit `disabled` (404), `unauthorized` (401/403), and `error` states instead of staying on "Loading...". Banner-driven prefetch is still honoured if it landed first. #11 NaN polling interval — coerce `checkIntervalHours` to a number, clamp to [1h, 168h], log a warning and fall back to 6h on non-finite input. Math.max(1, NaN) === NaN previously meant a malformed settings.json could turn the poller into a tight loop. #13 State validation accepted broken subfields — `isValid()` now inspects `latest.{version,tag,body,publishedAt,htmlUrl,prerelease}`, `vulnerableBelow[].{announcedBy,threshold}`, and `email.{severeAt,vulnerableAt,vulnerableNewReleaseTag}`. A hand-edited file with a number where a string is expected is now treated as corrupt and reset to EMPTY_STATE rather than crashing later in semver parsing or email rendering. #14 Badge cache stampede — wrap `computeOutdated()` in a single-flight promise so concurrent requests at cache expiry await one shared computation instead of fanning out into N redundant disk reads. Plus six new state.test.ts cases covering each new validation guard. Pushing back on the remaining items: #4 `updates.tier` defaults to `notify` — intentional. The whole point of tier 1 is to surface the "you are behind" signal to admins by default. Opt-in defeats the purpose; the existing failure mode (admin never hears about a security-relevant release) is exactly what this PR is fixing. #5/#8 Admin status endpoint admin-auth — `currentVersion` is already public via `/health`, so wrapping the route in admin-auth doesn't reduce the disclosure surface meaningfully. Operators who want it gated set `updates.requireAdminForStatus=true` (already wired and covered by the comment on the route handler). #10 Plain `https://` URLs in planning doc — planning markdown is viewed in editors and on GitHub where protocol-relative URLs would either render literally or break entirely. Keeping `https://`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-01 20:02:12 +08:00
John McLear	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>	2026-04-30 14:56:02 +08:00
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>	2026-04-29 09:25:53 +01:00
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>	2026-04-29 09:21:05 +01:00
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 `5eee7895a`) had no test, which is how the bug shipped originally — plugins.ts spawned `pnpm --version` at startup, the rejection was never caught, and the .deb crashed mid-boot. Add a backend spec that exercises: - ENOENT for a missing binary -> rejects (regression test) - successful command -> resolves stdout - non-zero exit -> rejects with code backend-tests.yml's recursive mocha glob picks up the new spec automatically; no workflow change needed there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(packaging-ci): use NodeSource LTS for the smoke test (was Ubuntu's node 18) ubuntu-latest's default apt nodejs is 18.19.1, but our package requires nodejs (>= 20). The smoke test was doing `apt-get install nodejs` followed by `dpkg -i ... \|\| apt-get install -f`, which on a node-18 host fails the dep check, then `-f` "fixes" by REMOVING the etherpad package — and the next assertion (test -x /usr/bin/etherpad) crashes. Match what packaging/test-local.sh and the README recommend: install node from NodeSource (current LTS) before installing the .deb. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(packaging-ci): sudo-prefix smoke assertions that read /etc/etherpad postinstall sets /etc/etherpad to 0750 root:etherpad (DB creds live here) and /var/lib/etherpad similarly. The GH Actions runner user isn't in the etherpad group, so 'test -f /etc/etherpad/settings.json' hits EACCES. Add sudo to each check that crosses one of those dirs. (Wrapping the whole block in `sudo bash <<EOF` would have been cleaner but YAML literal-block + heredoc terminator don't play well together at this indent.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(packaging): close chown -R symlink-deref escalation; Pre-Depends adduser postinstall: Use `chown -hR` instead of `chown -R` on /var/lib/etherpad/var and /var/lib/etherpad/plugin_packages. Both directories are writable by the unprivileged etherpad service user, so a symlink planted there could redirect root's chown onto arbitrary system files (e.g. /etc/shadow) on the next `apt upgrade`. -hR makes chown act on the symlink itself rather than its target — standard mitigation for this TOCTOU-style local privilege escalation. nfpm: Move adduser from Depends to Pre-Depends. preinst creates the etherpad user before unpacking; with plain `dpkg -i` (no apt) the Depends list isn't installed beforehand, so a minimal system without adduser would fail preinst before unpack and apt-get -f couldn't recover. Pre-Depends guarantees adduser is configured first. Both flagged in Qodo's persistent review of `3daf300f0`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(packaging): predepends lives at top-level deb:, not under overrides nfpm's Overridables schema doesn't include predepends; it's a deb-only top-level field. Previous commit nested it under overrides.deb, which caused nfpm to reject the entire manifest with "field predepends not found in type nfpm.Overridables" and broke both arch builds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(packaging): four Qodo follow-ups (CI ordering, secure node install, disable on remove, writable settings) deb-package.yml: - Move 'Resolve version' (which calls `node -p`) to AFTER setup-node so it doesn't depend on the runner image preinstalling node. - Replace `curl ... \| sudo bash` NodeSource installer with the explicit gpg-key + sources.list approach. Same outcome (NodeSource LTS apt repo), but no execution of network-fetched code as root. Reduces blast radius if NodeSource's setup endpoint is ever compromised — we only trust the signed apt repo metadata. postinstall.sh: - /etc/etherpad/settings.json now etherpad:etherpad mode 0660 (was root:etherpad 0640). The admin /admin/settings UI persists changes by writing back to settings.settingsFilename; with the previous perms the etherpad user could read but not write, so saving via the admin UI failed silently. Group-only access preserved (DB creds still unreadable by other users). postremove.sh: - On `dpkg --remove`, run `systemctl disable etherpad.service` before `daemon-reload` so the wants/ symlink doesn't dangle after dpkg deletes the unit file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(packaging): narrow workflow token scope; pin local nfpm to NFPM_VERSION deb-package.yml: Workflow-level permissions was `contents: write` so the build job got write access on every PR run, even though only the release job needs it (to attach release assets). Narrow the workflow default to `contents: read` and let the release job opt back in to write — it already declares its own job-level `contents: write` block, so this is just removing an over-broad default. test-local.sh: The script defined NFPM_VERSION but then unconditionally ran `goreleaser/nfpm:latest`, so local builds could diverge from CI's pinned v2.43.0. Use the variable in the docker tag (stripping the leading "v" to match the image's tag scheme). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-04-27 10:33:30 +01:00
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 `a55436ca0` were guarding against an overlap with a "Log out" button — but no Log out button exists in vanilla etherpad-lite (the original report came from a setup with a plugin that adds one). Without that button visible, the cap just makes the default username field unnecessarily narrow. Restore #myusernameform to just `margin-left: 10px` and drop the forced width on the input. If the overlap reappears in a real plugin setup it should be re-fixed there (or with a more targeted rule that only kicks in when a logout button is actually present). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat): keep titlesticky at top of title bar (#7590 review) The previous pass bottom-aligned both corner controls via align-items: flex-end on #titlebar. That correctly placed the close button (#titlecross) on the title's baseline, but it also dragged the much smaller "stick to screen" button (#titlesticky) down to the same baseline — visibly far below where it sat in the original layout. Switch to per-control align-self so each lands where it should: - #titlesticky → align-self: flex-start (top, where it always was) - #titlecross → align-self: flex-end (bottom, on the title's baseline) - #titlelabel → align-self: center (don't stretch the heading) Drop align-items from #titlebar so the defaults don't override these. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * revert(chat): restore original #titlebar layout (#7590 review) Both attempted CSS layouts for the title bar (full flex with align-items: flex-end, then per-control align-self) ended up looking worse than the original in review. Drop all the #titlebar / #titlelabel / #titlecross / #titlesticky changes from `905294d5b` and `f37da9a62` and restore the pre-existing float-based layout. The chat panel ships with its original visuals; we'll revisit #7590 separately if needed. Keeps the chat-icon click fix from `905294d5b` (#chaticon .buttonicon flex/pointer-events reset) and the focus-visible additions for the title-bar buttons. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat): clear inline display:none in chat.show() When the user disables chat in settings, applyShowChat(false) calls \`$('#chatbox').hide()\` which sets the chatbox's inline display to \`none\`. Re-enabling chat doesn't undo that — it only re-shows the icon. Then clicking the icon runs chat.show(), which adds the \`.visible\` class but only flips visibility, not display, so the chatbox stays hidden by the lingering inline style and the chat appears not to open. Clear the inline display in chat.show() before adding the .visible class so the box becomes visible regardless of how it got hidden. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(colibris): align username gap; grey unchecked-disabled toggles users.css: change #myusernameform margin-left from 35px to 10px to match the base popup_users.css. The 35px value was chosen for the sticky chatAndUsers layout, but for the standalone Users popup it opens an unnecessarily wide gap between the colour swatch and the username field. (#7593 review) form.css: drop the \`:checked\` qualifier from the disabled toggle visual rule so unchecked-but-disabled toggles also dim. Without this, "Chat always on screen" / "Show Chat and Users" stayed fully bright when "Disable chat" was ticked even though the underlying inputs were disabled. Fixes #7592 in the colibris skin. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat): simple flex titlebar — CHAT _ [] Single flex row, vertically centred via align-items: center. Title takes the remaining width with flex: 1; the two corner controls fall in at the right edge in source order (titlecross then titlesticky), giving the intended visual: minus on the left, sticky on the right. Drops `float: right` from the controls, `display: inline` from the heading, and the prior `padding-top: 2px` hack on titlesticky (flex alignment handles the vertical position now). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat): titlebar uses underscore for minimize; symmetric padding - Replace \`−\` with \`_\` in #titlecross. The minus glyph sits at the centre of its em-box and read as a hyphen mid-row when the row was vertically centred; \`_\` sits at the bottom of its em-box and reads as a proper minimize indicator. - Even out #titlebar horizontal padding to 9px and drop the asymmetric \`margin-left: 4px\` on #titlelabel so CHAT on the left and the sticky button on the right are the same distance from the bar's edges. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat): lift #titlecross underscore 5px The \`_\` glyph renders at the bottom of its em-box, so even with the title bar's flex \`align-items: center\` it sits noticeably below the CHAT baseline. Lift it with \`transform: translateY(-5px)\` (doesn't affect flex layout calculations) so the underscore reads at roughly the same vertical line as the title. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(chat): cover #7590 / #7592 / #7593 fixes Adds Playwright frontend specs for the changes in this PR: chat.spec.ts - chat icon click reveals chatbox after disable→enable cycle (regression: chat.show() must clear inline display:none) - title bar lays out as a centred flex row with underscore minimize (covers display, align-items, label flex:1, no float, translateY lift, and visual padding symmetry via rendered geometry) - chat icon click reliably opens the chat box (#chaticon .buttonicon pointer/flex reset) pad_settings.spec.ts - disabling chat disables and visually greys the dependent chat toggles (#7592 — checks input :disabled state and label opacity) change_user_name.spec.ts - #myusernameform has 10px left margin and is not width-capped (#7593 review — colibris margin alignment, no input width cap) Padding symmetry asserted via rendered rect deltas rather than the CSS literal, since colibris ships its own #titlebar padding override. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-04-26 09:56:38 +01:00
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>	2026-04-26 10:26:54 +02:00
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>	2026-04-26 10:19:23 +02:00
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.	2026-04-26 08:20:40 +01:00
John McLear	7b9a5eb01a	fix(a11y): dialog semantics, focus management, icon labels, html lang (#7584 ) * fix(a11y): negotiate lang/dir per request and set on <html> Server-renders the html element with `lang` and `dir` matching the client's Accept-Language header (negotiated against availableLangs from i18n hooks). Falls back to `en`/`ltr` if no match. This gives screen readers a correct document language during the brief window before client-side html10n refines it (l10n.ts already sets both attributes after locale data loads). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): dialog semantics on popups; fix aria-role typo on userlist Adds role=dialog, aria-modal=true, and either aria-labelledby (when an h1 is present) or aria-label (for popups without an h1) to: - #settings, #import_export, #embed, #skin-variants (labelledby) - #connectivity, #users, #mycolorpicker (aria-label) Fixes the invalid aria-role="document" attribute on #otherusers; it's now role=region with aria-live=polite so screen readers announce collaborator joins/leaves. Container aria-label values are English-only for now — Etherpad's html10n implementation only supports localizing specific attributes (title, alt, placeholder, etc), not aria-label on container nodes. Localization can follow once html10n grows that affordance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): focus management and Escape-to-close for popups Three additions to toggleDropDown / _bodyKeyEvent: - Remember the trigger element (document.activeElement) when opening a popup, so we can restore focus when it closes. - On open, focus the first focusable element inside the popup so keyboard users land inside the dialog instead of staying on the trigger button. - Escape pressed while focus is inside a popup closes it, then the restore-focus path runs and the trigger button is refocused. Replaces the previous behavior where Escape from inside a popup did nothing; users had to click outside to dismiss. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): make chaticon and chat header controls real buttons - #chaticon: <div onclick> → <button type=button> with aria-label - #titlecross / #titlesticky: <a onClick> → <button type=button> with aria-label (Close chat / Pin chat to screen) - Decorative chat-bubble glyph gets aria-hidden=true so it isn't read alongside the button label - #chatcounter labelled "Unread messages" - Inline onclick attributes moved to chat.init() handlers - CSS reset on the new buttons (transparent bg, no border, inherit font/color) so they match the prior visual design - :focus-visible outlines for keyboard users Existing test selectors (#chaticon, #titlecross, #titlesticky) are unchanged and continue to work — they never relied on element type. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): accessible names for icon-only toolbar/export controls - Export links (#exportetherpada, #exporthtmla, #exportplaina, #exportworda, #exportpdfa, #exportopena): added aria-label so the link is announced as e.g. "Export as PDF". The inner icon span gets aria-hidden=true so screen readers don't read both the icon text and the link label. - Show-more toolbar toggle (.show-more-icon-btn): converted from <span> to <button type=button> with aria-label and aria-expanded. The click handler now toggles aria-expanded alongside the full-icons class so assistive tech reflects the open/closed state. - Theme switcher knob: aria-label changed from "theme-switcher-knob" (a class-style identifier, not human text) to "Toggle theme". Aria-label values are English-only for now. Etherpad's html10n implementation only localizes a fixed attribute list (title, alt, placeholder, value, innerHTML, textContent); aria-label is not included, so a clean l10n path requires a follow-up to either extend html10n or set aria-label client-side after locale loads. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(a11y): cover dialog semantics, html lang, icon button labels New Playwright spec verifies the a11y guarantees added by this branch: - <html> has a non-empty lang attribute - settings/import_export/embed/users popups expose role=dialog, aria-modal=true, and either aria-labelledby (when an h1 exists) or aria-label (when none does) - Escape from inside the settings popup closes it AND restores focus to the trigger button - Export links each carry a descriptive aria-label - #chaticon is a real <button> with aria-label - #titlecross / #titlesticky are real <button>s with aria-label - #otherusers uses role=region + aria-live=polite + aria-label (and the previous aria-role typo is gone) - .show-more-icon-btn is a <button> with aria-label and aria-expanded Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(a11y): address Qodo review feedback from PR #7584 1. Users Escape close broken - toggleDropDown('none') intentionally skips the users module so switching between other popups doesn't hide the user list. That meant Escape couldn't dismiss the Users popup either. The Escape branch now checks for #users as the focused popup and closes it explicitly (respecting stickyUsers) before falling through to the normal close-all path. 2. Embed focus overridden - the rAF auto-focus in toggleDropDown grabbed the first focusable descendant, which stole focus from command handlers that target a specific control (notably the Embed command's #linkinput). rAF now bails out if focus is already inside the newly-opened popup. 3. Button click blurs :focus before toggleDropDown captures trigger - discovered while investigating the Firefox Playwright failure for "settings popup Escape restores focus". Button.bind() calls $(':focus').trigger('blur') before invoking the callback, so by the time toggleDropDown() captured document.activeElement as the restore target it was already <body>. The click handler now stashes padeditbar._lastTrigger to the clicked <button> before blur runs; toggleDropDown only falls back to activeElement when the pre-stash didn't happen (keyboard shortcut path). 4. html10n overwrites aria-label - html10n unconditionally set aria-label to the translated string, clobbering explicit aria-label on elements that also carry data-l10n-id. setAttribute now only fires when the element has no aria-label; explicit author labels win, unlabelled translated elements still get a name. 5. Button visual reset - the show-more-icon-btn and #chaticon conversions inherited UA default button border/background/padding, shifting icon glyphs visibly off-centre. Added appearance / background / border / padding resets. 6. Export links test assumes soffice is installed - #exportworda, #exportpdfa, #exportopena are removed client-side by pad_impexp.ts when clientVars.exportAvailable === 'no'. The test now skips links absent at runtime. Verified locally: all 10 a11y_dialogs specs pass on both Chromium and Firefox; backend suite remains 799/799 passing; ts-check clean. * fix(a11y): close popups with no focusable content; unbreak chat-icon layout Round 2 of #7584 review follow-ups. 1. Users popup Escape still didn't close the dialog (user-confirmed). Root cause: _bodyKeyEvent is bound to the OUTER document's body. When #users opens, the command handler tries to focus #myusernameedit but that input is `disabled`, so focus stays in the ace editor iframe. Keydown from inside the iframe does not bubble to the outer document, so Esc never reaches _bodyKeyEvent. Fix: in the open-popup rAF, if no command handler placed focus inside the dialog, focus the popup div itself (with tabindex=-1). That keeps subsequent keydown events on the outer document so Esc can dismiss the popup. Also broadened the Esc branch to fire whenever any popup is `.popup-show`, regardless of where :focus lives — some popups legitimately have no focusable content at open. Added a regression test that opens #users and asserts Esc closes it. Passes on both Chromium and Firefox. 2. Chat icon (#chaticon) visual still wrong after the first CSS fix. - My previous `border: 0` reset was overriding the intended `border: 1px solid #ccc; border-bottom: none` from the earlier rule. Removed `border: 0`; the earlier explicit border suffices to suppress UA defaults. - The `<span class="buttonicon">` inside `#chaticon` was picking up the global `.buttonicon { display: flex; }` rule meant for toolbar button instances, which broke the inline layout of the label + glyph + counter row. Added a scoped `#chaticon .buttonicon { display: inline; }` override. All 11 a11y_dialogs specs pass on Chromium and Firefox. Backend suite and ts-check remain clean. * fix(a11y): only stash _lastTrigger for dropdown-opening buttons Round 3 follow-up. The previous Button.bind() change stashed every clicked toolbar button as padeditbar._lastTrigger before blurring :focus. That was necessary for popup-opening buttons (settings, import_export, etc.) so Escape could return focus to them — but it also fired for non-popup toolbar buttons (list toggles, bold/italic, indent/outdent, clearauthorship). For those, the stash held a stale reference that interfered with subsequent editor interactions and regressed Playwright tests: ordered_list, unordered_list, undo_clear_authorship. Fix: only stash when the clicked command is a registered dropdown (settings, import_export, embed, showusers, savedrevision, connectivity). Other commands return focus to the ace editor as before and leave _lastTrigger alone. Verified locally on Chromium: - ordered_list.spec.ts: 6/6 pass (was 4/6) - unordered_list.spec.ts: 6/6 pass (was 4/6) - undo_clear_authorship.spec.ts: 2/2 pass (was 0/2) - a11y_dialogs.spec.ts: 11/11 pass (unchanged) * fix(a11y): address Qodo review round 4 for PR #7584 #1 Stale aria-label after relocalize html10n.translateNode() refused to overwrite any existing aria-label, which also skipped updates on language change (pad.applyLanguage() re-runs localize). Use a `data-l10n-aria-label="true"` marker: set aria-label + marker when html10n populates it, overwrite only if the marker is present. Explicit template-supplied aria-labels stay as-is; html10n-generated ones refresh on relocalize. #2 Escape won't close colorpicker _bodyKeyEvent caught Escape on any `.popup.popup-show` but only closed dropdown popups via toggleDropDown('none'). Popups opened outside the editbar framework (#mycolorpicker, toggled directly by pad_userlist.ts) stayed open while preventDefault() swallowed the key. Now the Escape branch manually closes any popup that toggleDropDown('none') cannot reach (non-dropdown ids, plus #users unless pinned) and leaves registered dropdowns for toggleDropDown to close so its focus-restore sees the transition. #3 Stale focus restoration toggleDropDown('none') restored focus to _lastTrigger even when no popup was open on entry, which meant background callers (connectivity setup, periodic state handling) could yank focus out of the editor to a stale toolbar button. Gated the restore on `wasAnyOpen === true` so it only fires when there was a popup to close. #11 English aria-label overrides i18n (export links, chat icon) Removed the hard-coded English aria-label from export anchors and removed aria-hidden from their inner localized spans. Screen readers now get the localized child text as the accessible name (Etherpad, HTML, PDF, etc.), matching the visible UI language. Removed the English aria-label from #chaticon and #titlesticky as well — both have data-l10n-id, so html10n populates a localized aria-label via the marker mechanism in #1. #titlecross keeps its static aria-label because it has no data-l10n-id yet. #4 4-space indent in a11y spec Two tests had continuation lines at 4-space indent violating the repo's 2-space rule. Folded the signatures onto one line. Updated a11y_dialogs.spec.ts to assert accessible-name presence rather than hard-coded English for elements whose names now come from the localized text. Still asserts static English for #titlecross (not localized yet). Verified locally (dev server restarted for each round): - a11y_dialogs.spec.ts: 11/11 on Chromium, 11/11 on Firefox - ordered_list + unordered_list + undo_clear_authorship: 13/13 on Chromium - Full backend suite: 799 passing, 0 failing - tsc --noEmit clean in our code #9 Popup behavior documentation: deferred to a follow-up doc PR so this PR stays focused on the a11y code changes. The new keyboard behavior (Escape-to-close, focus-restore-to-trigger) is small enough to summarize in a short doc/ addition. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-04-24 03:04:18 +01:00
John McLear	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>	2026-04-23 08:57:30 +01:00
John McLear	e58dfa4752	feat: add timeslider line numbers (#7542 ) * feat: add timeslider line numbers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * perf: coalesce timeslider line-number updates Addresses Qodo review: updateLineNumbers() was called synchronously from applyChangeset() on every changeset, forcing full-document layout reads/writes during timeslider scrubbing/playback. scheduleLineNumberUpdate() also queued a fresh double-rAF pair for every resize tick. Add a pending flag so only one rAF pair is in flight, and route applyChangeset() through the scheduler. 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>	2026-04-19 11:18:54 +01:00
John McLear	e0ccdb4d9f	Add creator-owned pad settings defaults (#7545 ) * Add creator-owned pad settings defaults Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Refine pad settings layout Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix settings popup heading and width Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Explain enforced user settings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Cover creator override flow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Let creators bypass enforced settings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address pad settings follow-ups Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-04-19 11:13:44 +01:00

1 2 3 4 5 ...

2048 commits