feat(pad): scrub history in-place on the pad URL (#7659) by JohnMcLear · Pull Request #7710 · ether/etherpad

JohnMcLear · 2026-05-09T14:45:24Z

Closes #7659.

Click 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.

Summary

New PadModeController (src/static/js/pad_mode.ts) owns
enter/exit, the URL hash, browser back/forward, Esc to exit, and a
mutation-observer bridge that copies the inner timeslider's revision
label/date into the outer banner.
pad.html grows a banner element and an iframe mount slot. The live
ACE iframe stays mounted (just hidden), so the live socket survives
the round-trip and the user snaps back to the current state without a
reconnect on exit.
/p/:pad/timeslider 302-redirects to the pad page for direct visits
(legacy bookmarks; the browser preserves any #NN hash through the
redirect, and a client-side shim translates it to #rev/NN). Iframe
consumers pass ?embed=1 and still get the timeslider HTML; the
embedded variant hides the redundant title and return-to-pad button
via CSS, but the slider, settings, and export controls stay reachable.

Visuals

Live	History mode
Pad as today	Banner: Viewing history • Version N • Saved … + Return-to-live button. Outer toolbar dimmed (sepia + saturate); editor area hosts the embedded timeslider iframe.

Test plan

pnpm run ts-check clean for changed files
Backend: tests/backend/specs/timesliderRedirect.ts covers 302
direct visits, pad-name preservation, and ?embed=1 HTML response
Playwright: new padmode.spec.ts (toolbar entry, return-to-live,
browser back, legacy-URL redirect; asserts the rendered localized
banner string, not just element presence)
Playwright: existing timeslider*.spec.ts updated to pass
?embed=1 for direct-iframe access; all 14 timeslider + padmode
tests green
Adjacent UI specs (editbar, embed_value, a11y_dialogs)
re-run with no regressions
Manual browser walkthrough on a live pad: enter via toolbar →
banner appears, sepia tint applied, slider works inside iframe;
Return-to-live cleanly restores live editing without reconnect.

🤖 Generated with Claude Code

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>

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>

qodo-code-review · 2026-05-09T14:45:28Z

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

qodo-free-for-open-source-projects · 2026-05-09T14:45:41Z

Review Summary by Qodo

Implement in-pad history mode with hash-based navigation

✨ Enhancement

Walkthroughs

Description

• Implement in-pad history mode via hash-based navigation (#rev/N)
• Keep pad shell mounted while viewing historical revisions
• Add sticky banner and visual indicators for history viewing
• Redirect legacy /p/:pad/timeslider URLs to pad page with 302

Diagram

flowchart LR
  A["User clicks history button"] --> B["PadModeController.enterHistory"]
  B --> C["Set URL hash to #rev/N"]
  B --> D["Mount timeslider iframe"]
  B --> E["Show history banner"]
  F["Direct /p/:pad/timeslider visit"] --> G["302 redirect to /p/:pad"]
  G --> H["Browser preserves #NN hash"]
  H --> I["Client translates to #rev/NN"]
  I --> B
  J["User clicks Return-to-live"] --> K["PadModeController.exitHistory"]
  K --> L["Unmount iframe"]
  K --> M["Clear hash"]
  K --> N["Hide banner"]

File Changes

1. src/static/js/pad_mode.ts ✨ Enhancement +248/-0

New PadModeController for in-pad history mode

src/static/js/pad_mode.ts

2. src/static/js/pad.ts ✨ Enhancement +2/-0

Initialize PadModeController on pad startup

src/static/js/pad.ts

3. src/static/js/pad_editbar.ts ✨ Enhancement +8/-1

Route history button to PadModeController

src/static/js/pad_editbar.ts

View more (13)

4. src/node/hooks/express/specialpages.ts ✨ Enhancement +16/-1

Add 302 redirect for direct timeslider visits

src/node/hooks/express/specialpages.ts

5. src/templates/pad.html ✨ Enhancement +20/-1

Add history banner and iframe mount slot

src/templates/pad.html

6. src/templates/timeslider.html ✨ Enhancement +1/-1

Add embedded-history-frame CSS class support

src/templates/timeslider.html

7. src/static/css/pad.css ✨ Enhancement +54/-0

Style history banner and iframe container

src/static/css/pad.css

8. src/static/css/timeslider.css ✨ Enhancement +19/-0

Hide redundant elements in embedded mode

src/static/css/timeslider.css

9. src/locales/en.json 📝 Documentation +3/-0

Add localization strings for history mode

src/locales/en.json

10. src/tests/backend/specs/timesliderRedirect.ts 🧪 Tests +45/-0

Test 302 redirect and embed=1 parameter

src/tests/backend/specs/timesliderRedirect.ts

11. src/tests/frontend-new/specs/padmode.spec.ts 🧪 Tests +88/-0

Test in-pad history mode entry and exit

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

12. src/tests/frontend-new/specs/timeslider.spec.ts 🧪 Tests +22/-17

Update to verify hash-based history mode

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

13. src/tests/frontend-new/helper/timeslider.ts 🧪 Tests +5/-1

Add embed=1 parameter to timeslider navigation

src/tests/frontend-new/helper/timeslider.ts

14. src/tests/frontend-new/specs/timeslider_identity_changeset.spec.ts 🧪 Tests +3/-3

Add embed=1 to direct timeslider URLs

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

15. src/tests/frontend-new/specs/timeslider_line_numbers.spec.ts 🧪 Tests +2/-2

Add embed=1 to direct timeslider URLs

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

16. src/tests/frontend-new/specs/timeslider_playback_speed.spec.ts 🧪 Tests +3/-3

Add embed=1 to direct timeslider URLs

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

qodo-free-for-open-source-projects · 2026-05-09T14:45:42Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0)

1. History mode lacks feature flag 📘 Rule violation ☼ Reliability

Description

The new in-pad history mode is initialized and reachable by default without a feature flag. This
violates the requirement that new features be gated and disabled by default to preserve the
pre-change behavior when the flag is off.

Code

src/static/js/pad.ts[675]
+      padMode.init();

Evidence
PR Compliance ID 6 requires new features to be behind a feature flag and disabled by default. The PR
initializes padMode unconditionally on pad load and wires the history toolbar command to enter the
new mode, making the feature active by default.
src/static/js/pad.ts[675-675]
src/static/js/pad_editbar.ts[502-510]
src/static/js/pad_mode.ts[231-246]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
In-pad history mode is a new feature but it is enabled by default (initialized on every pad load and entered via the toolbar). Compliance requires a feature flag with default-off behavior that preserves the pre-change code path.
## Issue Context
The change introduces a new controller (`pad_mode`) and new routing/UX behavior. When the flag is off, clicking the history button should follow the legacy navigation behavior and the pad should not initialize/mount history-mode UI.
## Fix Focus Areas
- src/static/js/pad.ts[45-45]
- src/static/js/pad.ts[675-675]
- src/static/js/pad_editbar.ts[502-510]
- src/static/js/pad_mode.ts[231-246]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Hardcoded https Google Fonts~~ ✓ Resolved 📘 Rule violation ⛨ Security

Description

External font resources are added using hardcoded https:// URLs instead of protocol-independent
// URLs. This can cause mixed-content/compatibility issues in non-HTTPS deployments and violates
the project convention.

Code

src/templates/pad.html[R24-26]

+  <link rel="preconnect" href="https://fonts.googleapis.com">
+  <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
+  <link href="https://fonts.googleapis.com/css2?family=Newsreader:ital,opsz,wght@0,6..72,300..700;1,6..72,300..700&family=Instrument+Serif:ital@0;1&family=Lora:ital,wght@0,400..700;1,400..700&family=Space+Mono:ital,wght@0,400;0,700;1,400;1,700&family=VT323&family=IBM+Plex+Mono:ital,wght@0,400;0,500;0,600;1,400&family=IBM+Plex+Sans:ital,wght@0,400;0,500;0,600;0,700&display=swap" rel="stylesheet">

Evidence

PR Compliance ID 10 requires protocol-independent URLs (//...) where applicable for external
resources. The added Google Fonts preconnect and stylesheet links are hardcoded to https://....

src/templates/pad.html[24-26]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pad.html` adds external Google Fonts resources using `https://` instead of protocol-independent `//` URLs.
## Issue Context
The compliance checklist requires protocol-independent URLs where appropriate to avoid mixed content issues and improve deployment compatibility.
## Fix Focus Areas
- src/templates/pad.html[24-26]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Timeslider URL change undocumented~~ ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The behavior of /p/:pad/timeslider has changed (302 redirect unless ?embed=1), but no
documentation updates are included. This makes existing documentation about timeslider routes
inaccurate and can break adopters who rely on the documented URL.

Code

src/node/hooks/express/specialpages.ts[R402-408]

+      // Direct visits (legacy bookmarks) get redirected back to the pad,
+      // where the in-pad PadModeController handles entering history mode.
+      // The iframe used by history mode requests this URL with ?embed=1
+      // and gets the full timeslider HTML rendered for embedded use.
+      if (!req.query.embed) {
+        return res.redirect(302, `../${encodeURIComponent(req.params.pad)}`);
+      }

Evidence
PR Compliance ID 3 requires updating in-repo docs when public interfaces change. This PR changes the
timeslider route behavior to redirect by default and introduces ?embed=1 for HTML access; existing
docs still describe /p/:padid/timeslider as the direct timeslider page without mentioning the
redirect or embed=1.
src/node/hooks/express/specialpages.ts[402-408]
src/node/hooks/express/specialpages.ts[420-420]
doc/skins.md[5-10]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The timeslider endpoint behavior changed: `/p/:pad/timeslider` now redirects to the pad unless `?embed=1` is provided, but docs still imply the timeslider is directly served at `/p/:padid/timeslider`.
## Issue Context
This is a public, user- and integrator-facing URL change. Documentation should mention the redirect and provide the correct URL for embedding/direct HTML (`?embed=1`) with an example.
## Fix Focus Areas
- doc/skins.md[5-10]
- src/node/hooks/express/specialpages.ts[402-408]
- src/node/hooks/express/specialpages.ts[420-420]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View more (2)

4. ~~Latest hash opens rev0~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

PadModeController.onOuterHashChange() maps #rev/latest to revision 0 via rev < 0 ? 0 : rev,
which forces the embedded timeslider to jump to the first revision instead of the latest. Users
manually navigating to #rev/latest (or any future link that sets it) will see the wrong historical
content.

Code

src/static/js/pad_mode.ts[R214-225]

+  private onOuterHashChange(): void {
+    if (this.syncingHash) return;
+    const rev = parseRevFromHash(window.location.hash);
+    if (rev == null) {
+      if (this.mode === 'history') this.exitHistory();
+      return;
+    }
+    if (this.mode === 'live') {
+      this.enterHistory(rev);
+    } else {
+      this.setInnerRevision(rev < 0 ? 0 : rev);
+    }

Evidence
The outer hash parser explicitly uses -1 to represent latest, but the history-mode hash handler
converts negative revisions to 0 before updating the embedded timeslider’s hash. The embedded
timeslider uses window.location.hash = #${newpos}`` to select an exact revision number, so setting
#0 means “revision 0”.
src/static/js/pad_mode.ts[27-38]
src/static/js/pad_mode.ts[214-226]
src/static/js/pad_mode.ts[196-201]
src/static/js/broadcast_slider.ts[106-126]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`#rev/latest` is parsed as `-1` but then converted to `0` when syncing the outer hash into the embedded timeslider, which incorrectly jumps to revision 0.
### Issue Context
The embedded timeslider interprets `window.location.hash = "#N"` as the desired revision number.
### Fix Focus Areas
- src/static/js/pad_mode.ts[214-226]
- src/static/js/pad_mode.ts[196-201]
### Implementation notes
- When `rev < 0`, clear the inner hash (`win.location.hash = ""`) instead of setting `#0`, or compute the latest revision number from the embedded frame (if available) and set that.
- Ensure the outer URL remains canonical (`#rev/latest` is fine), but don’t mis-target the embedded slider.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~External Google Fonts added~~ ✓ Resolved 🐞 Bug ⛨ Security

Description

pad.html now unconditionally loads multiple Google Fonts via
fonts.googleapis.com/fonts.gstatic.com, adding third‑party network requests on every pad view.
This can break offline/self-hosted deployments and leaks user IP/metadata to a third party by
default.

Code

src/templates/pad.html[R17-26]

+<html lang="<%=renderLang%>" dir="<%=renderDir%>" translate="no" data-theme="editorial" class="pad <%=pluginUtils.clientPluginNames().join(' '); %> <%=settings.skinVariants%>">
<head>
<% e.begin_block("htmlHead"); %>
<% e.end_block(); %>
<title><%=settings.title%></title>
<%- typeof socialMetaHtml !== 'undefined' ? socialMetaHtml : '' %>
<link rel="manifest" href="../../manifest.json" />
+  <link rel="preconnect" href="https://fonts.googleapis.com">
+  <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
+  <link href="https://fonts.googleapis.com/css2?family=Newsreader:ital,opsz,wght@0,6..72,300..700;1,6..72,300..700&family=Instrument+Serif:ital@0;1&family=Lora:ital,wght@0,400..700;1,400..700&family=Space+Mono:ital,wght@0,400;0,700;1,400;1,700&family=VT323&family=IBM+Plex+Mono:ital,wght@0,400;0,500;0,600;1,400&family=IBM+Plex+Sans:ital,wght@0,400;0,500;0,600;0,700&display=swap" rel="stylesheet">

Evidence

The pad shell template includes new ` and a stylesheet ` to Google Fonts, which the browser will
fetch on every pad page load unless blocked.

src/templates/pad.html[16-27]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The core pad HTML template now hard-depends on Google Fonts, introducing third-party requests and potential breakage for offline/CSP-restricted installations.
### Issue Context
Etherpad is commonly self-hosted; loading external fonts by default is a privacy and reliability regression.
### Fix Focus Areas
- src/templates/pad.html[16-27]
### Implementation notes
- Remove the Google Fonts `<link>` tags entirely, or gate them behind an explicit setting/skin.
- If these fonts are required for a theme, bundle/self-host them under `src/static/` and reference local URLs instead.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. ~~Redirect embed bypass too loose~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

The server treats any truthy ?embed= value as an iframe request and skips the redirect, so direct
visits can still access /p/:pad/timeslider by adding ?embed=0 or similar. Additionally, the
redirect target is built as a relative path containing the padId, which can normalize unexpectedly
for edge pad IDs like ...

Code

src/node/hooks/express/specialpages.ts[R230-237]

setRouteHandler("/p/:pad/timeslider", (req: any, res: any, next: Function) => {
+        // Direct visits (legacy bookmarks) get redirected back to the pad,
+        // where the in-pad PadModeController handles entering history mode.
+        // The iframe used by history mode requests this URL with ?embed=1
+        // and gets the full timeslider HTML rendered for embedded use.
+        if (!req.query.embed) {
+          return res.redirect(302, `../${encodeURIComponent(req.params.pad)}`);
+        }

Evidence
The redirect decision is if (!req.query.embed) (so any non-empty embed value bypasses). The
redirect location is ../${encodeURIComponent(req.params.pad)}; pad IDs allow .. by validation,
and encodeURIComponent('..') stays .., so the relative redirect can normalize to parent paths.
src/node/hooks/express/specialpages.ts[230-237]
src/node/hooks/express/specialpages.ts[401-408]
src/node/db/PadManager.ts[195-196]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`/p/:pad/timeslider` bypasses the redirect for any truthy `embed` query value and uses a relative redirect target that can normalize oddly for edge pad IDs.
### Issue Context
The embed path is intended only for the in-pad iframe (`?embed=1`).
### Fix Focus Areas
- src/node/hooks/express/specialpages.ts[230-237]
- src/node/hooks/express/specialpages.ts[401-408]
### Implementation notes
- Change the condition to something strict like `if (req.query.embed !== '1') redirect...`.
- Prefer a simple redirect target of `..` (from `/p/:pad/timeslider` this resolves to `/p/:pad`) or an absolute `/p/${encodeURIComponent(req.params.pad)}` to avoid composing `../${pad}`.
- Keep behavior consistent in both dev live-reload and production handlers.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-free-for-open-source-projects · 2026-05-09T14:53:03Z

      $('#colorpicker').farbtastic({callback: '#mycolorpickerpreview', width: 220});
      $('#readonlyinput').on('click', () => { padeditbar.setEmbedLinks(); });
      padcookie.init();
+      padMode.init();


1. History mode lacks feature flag 📘 Rule violation ☼ Reliability

The new in-pad history mode is initialized and reachable by default without a feature flag. This violates the requirement that new features be gated and disabled by default to preserve the pre-change behavior when the flag is off.

Agent Prompt

## Issue description In-pad history mode is a new feature but it is enabled by default (initialized on every pad load and entered via the toolbar). Compliance requires a feature flag with default-off behavior that preserves the pre-change code path. ## Issue Context The change introduces a new controller (`pad_mode`) and new routing/UX behavior. When the flag is off, clicking the history button should follow the legacy navigation behavior and the pad should not initialize/mount history-mode UI. ## Fix Focus Areas - src/static/js/pad.ts[45-45] - src/static/js/pad.ts[675-675] - src/static/js/pad_editbar.ts[502-510] - src/static/js/pad_mode.ts[231-246]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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>

JohnMcLear · 2026-05-09T15:00:20Z

Pushed f276bb8 addressing the Qodo review:

Fixed

?embed= truthy bypass — now strict req.query.embed !== '1'.
#rev/latest jumping to revision 0 — outer hash parser yields -1, but the iframe sync handler now resolves "latest" to BroadcastSlider.getSliderLength() instead of clamping to 0.
Doc update — doc/skins.md + .adoc now describe the redirect and the ?embed=1 URL.
Backed out a stray data-theme="editorial" and Google Fonts <link> tags that leaked into the branch from an unrelated working-tree change. Those were not part of this feature.
(CI) Three pre-existing backend tests (socialMeta, specialpages) hit /p/:pad/timeslider directly; updated them to pass ?embed=1 like the others.

Dismissed

Feature flag — Etherpad doesn't gate UX changes behind feature flags (and the existing toolbar button is already the only way users reach this code path; legacy URLs still redirect cleanly). Happy to add one if a maintainer wants it, but matching repo convention here.

…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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

JohnMcLear and others added 2 commits May 9, 2026 15:17

qodo-code-review Bot deleted a comment from qodo-free-for-open-source-projects Bot May 9, 2026

qodo-free-for-open-source-projects Bot reviewed May 9, 2026

View reviewed changes

qodo-code-review Bot deleted a comment from qodo-free-for-open-source-projects Bot May 9, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(pad): scrub history in-place on the pad URL (#7659)#7710

feat(pad): scrub history in-place on the pad URL (#7659)#7710
JohnMcLear wants to merge 13 commits intodevelopfrom
feat/7659-timeslider-in-pad

JohnMcLear commented May 9, 2026

Uh oh!

qodo-code-review Bot commented May 9, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented May 9, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented May 9, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot May 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JohnMcLear commented May 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

JohnMcLear commented May 9, 2026

Summary

Visuals

Test plan

Uh oh!

qodo-code-review Bot commented May 9, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented May 9, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-free-for-open-source-projects Bot commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JohnMcLear commented May 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

qodo-free-for-open-source-projects Bot commented May 9, 2026 •

edited

Loading