fix(pad): show saved-revision markers in in-pad history mode (#7946)

src/node/db/Pad.ts

@@ -868,6 +868,8 @@ class Pad {
     await this.saveToDatabase();
   }
 
+  // Returns the newly created saved revision, or undefined if this revision
+  // was already saved (so callers can broadcast only genuine additions).
   async addSavedRevision(revNum: string, savedById: string, label: string) {
     // if this revision is already saved, return silently
     for (const i in this.savedRevisions) {
@@ -887,6 +889,7 @@ class Pad {
     // save this new saved revision
     this.savedRevisions.push(savedRevision);
     await this.saveToDatabase();
+    return savedRevision;
   }
 
   getSavedRevisions() {

src/node/handler/PadMessageHandler.ts

@@ -622,7 +622,18 @@ exports.handleMessage = async (socket:any, message: ClientVarMessage) => {
 const handleSaveRevisionMessage = async (socket:any, message: ClientSaveRevisionMessage) => {
   const {padId, author: authorId} = sessioninfos[socket.id];
   const pad = await padManager.getPad(padId, null, authorId);
-  await pad.addSavedRevision(pad.head, authorId);
+  const savedRevision = await pad.addSavedRevision(pad.head, authorId);
+  // Notify every client in the pad room — including any open timeslider —
+  // so saved-revision markers appear live instead of only on the next
+  // timeslider load (#7946). The client's NEW_SAVEDREV handler existed but
+  // was never reached because this broadcast was missing; live editors that
+  // don't handle the type ignore it. Skip the emit for duplicate saves.
+  if (savedRevision) {
+    socketio.sockets.in(padId).emit('message', {
+      type: 'COLLABROOM',
+      data: {type: 'NEW_SAVEDREV', savedRev: savedRevision},
+    });
+  }
 };
 
 /**

src/static/css/pad.css

@@ -214,12 +214,52 @@ body.history-mode #history-controls { display: flex; }
 .history-controls button.buttonicon.buttonicon-play.pause::before {
   content: "\e829";
 }
-.history-slider-input {
+.history-slider-wrap {
+  position: relative;
   flex: 1 1 auto;
   min-width: 80px;
   margin: 0 6px;
+  display: flex;
+  align-items: center;
+}
+.history-slider-input {
+  flex: 1 1 auto;
+  min-width: 0;
+  width: 100%;
   cursor: pointer;
 }
+/* Saved-revision markers overlaid on the slider track (issue #7946). Inset
+ * left/right by ~half the native range thumb so a marker lines up with the
+ * thumb centre at the track extremes. pointer-events:none on the layer keeps
+ * slider dragging unobstructed; the stars themselves re-enable clicks to seek. */
+.history-slider-stars {
+  position: absolute;
+  left: 8px;
+  right: 8px;
+  top: 0;
+  bottom: 0;
+  pointer-events: none;
+}
+.history-slider-stars .history-star {
+  position: absolute;
+  top: 50%;
+  transform: translate(-50%, -50%);
+  width: 16px;
+  height: 16px;
+  padding: 0;
+  margin: 0;
+  border: 0;
+  background: none;
+  line-height: 1;
+  cursor: pointer;
+  pointer-events: auto;
+}
+.history-slider-stars .history-star::before {
+  font-family: fontawesome-etherpad;
+  content: "\e856";
+  color: #da9700;
+  font-size: 14px;
+}
 .history-timer {
   flex: 0 0 auto;
   font-size: 12px;
@@ -282,7 +322,7 @@ body.history-mode #history-controls { display: flex; }
 @media (max-width: 800px) {
   .history-controls { padding: 0 6px; gap: 4px; min-height: 36px; }
   .history-controls button.buttonicon { padding: 4px 6px; min-width: 32px; }
-  .history-slider-input { min-width: 60px; margin: 0 2px; }
+  .history-slider-wrap { min-width: 60px; margin: 0 2px; }
 }
 @media (max-width: 480px) {
   .history-controls #history-leftstep,

src/static/js/pad_mode.ts

@@ -55,6 +55,10 @@ class PadModeController {
   private padId: string;
   private innerHashChangeHandler: (() => void) | null = null;
   private revObserver: MutationObserver | null = null;
+  // Watches the embedded slider's #ui-slider-bar so saved revisions added live
+  // (NEW_SAVEDREV from a collaborator while we're in history mode) get mirrored
+  // onto the outer slider — clientVars only carries the entry-time snapshot.
+  private savedRevObserver: MutationObserver | null = null;
   private syncingHash = false;
 
   // History-mode bridges — populated on enter, torn down on exit.
@@ -194,6 +198,11 @@ class PadModeController {
     }
     this.revLabel.textContent = '';
     this.dateLabel.textContent = '';
+    const stars = document.getElementById('history-slider-stars');
+    if (stars) {
+      stars.replaceChildren();
+      stars.dataset.sig = '';
+    }
   }
 
   // Restore everything entry-time we stashed: chat message visibility, the
@@ -248,6 +257,10 @@ class PadModeController {
       this.revObserver.disconnect();
       this.revObserver = null;
     }
+    if (this.savedRevObserver) {
+      this.savedRevObserver.disconnect();
+      this.savedRevObserver = null;
+    }
     if (this.iframe) {
       try {
         if (this.innerHashChangeHandler && this.iframe.contentWindow) {
@@ -374,6 +387,10 @@ class PadModeController {
         playBtn.classList.toggle('pause', playing);
         playBtn.setAttribute('aria-pressed', playing ? 'true' : 'false');
       }
+      // Saved-revision markers depend on the slider max, which is only known
+      // once the inner slider has reported its length — render them here so we
+      // pick up the correct positions on first sync and on any max change.
+      this.renderSavedRevisionStars(innerWin);
     };
     // The hook registered earlier in attachInnerBridges already calls
     // onRevChange — piggyback on it for slider input/timer updates by
@@ -386,10 +403,87 @@ class PadModeController {
       }
       BS.onSlider(sync);
       sync(BS.getSliderPosition?.() ?? 0);
+      // Now that the inner slider exists, watch it for live NEW_SAVEDREV stars.
+      this.observeInnerSavedRevisions(innerWin);
     };
     registerSync();
   }
 
+  // Mirror the embedded timeslider's saved revisions onto the outer slider as
+  // clickable star markers (issue #7946). The inner slider draws its own stars
+  // on #ui-slider-bar, but that DOM is hidden in embed mode, so users only see
+  // the outer #history-slider-input — which had no markers.
+  //
+  // The inner #ui-slider-bar .star elements are the live source of truth: the
+  // timeslider keeps them current as NEW_SAVEDREV messages arrive (each carries
+  // a `pos` attribute = revNum), whereas clientVars.savedRevisions is only the
+  // entry-time snapshot. We read positions from those stars and pull labels
+  // from the snapshot where available. A signature guard keeps this cheap when
+  // sync() fires on every scrub; positions are percentage-based so they reflow
+  // on resize for free.
+  private renderSavedRevisionStars(innerWin: Window): void {
+    const inner: any = innerWin as any;
+    const layer = document.getElementById('history-slider-stars');
+    const sliderInput = document.getElementById('history-slider-input') as HTMLInputElement | null;
+    if (!layer || !sliderInput || !innerWin.document) return;
+
+    const max = Number(sliderInput.max) || 0;
+    const revNums = Array.from(innerWin.document.querySelectorAll('#ui-slider-bar .star'))
+        .map((el) => Number(el.getAttribute('pos')))
+        // max === 0 is a valid single-revision pad: only rev 0 belongs there.
+        .filter((n) => Number.isFinite(n) && n >= 0 && (max === 0 ? n === 0 : n <= max));
+
+    if (revNums.length === 0 || max < 0) {
+      if (layer.childElementCount) layer.replaceChildren();
+      layer.dataset.sig = '';
+      return;
+    }
+
+    // Labels live in the clientVars snapshot, keyed by revNum.
+    const labels = new Map<number, string>();
+    const snapshot = inner.clientVars?.savedRevisions;
+    if (Array.isArray(snapshot)) {
+      for (const r of snapshot) {
+        const n = Number(r && r.revNum);
+        if (Number.isFinite(n) && r && typeof r.label === 'string' && r.label) labels.set(n, r.label);
+      }
+    }
+
+    const sig = `${max}:${[...revNums].sort((a, b) => a - b).join(',')}`;
+    if (layer.dataset.sig === sig) return;
+    layer.dataset.sig = sig;
+    layer.replaceChildren();
+
+    for (const revNum of revNums) {
+      const frac = max === 0 ? 0 : revNum / max;
+      // A purely visual marker (the layer is aria-hidden): keyboard/screen
+      // reader users already reach any revision via the slider and step
+      // buttons, so we mirror the legacy timeslider's mouse-only stars rather
+      // than inject extra tab stops. The hover title aids mouse users; the
+      // click is a convenience to jump straight to the saved point.
+      const star = document.createElement('span');
+      star.className = 'history-star';
+      star.style.left = `${(frac * 100).toFixed(4)}%`;
+      star.title = labels.get(revNum) || `Revision ${revNum}`;
+      star.addEventListener('click', () => {
+        try { inner.BroadcastSlider?.setSliderPosition?.(revNum); } catch (_e) { /* inner gone */ }
+      });
+      layer.appendChild(star);
+    }
+  }
+
+  // Re-render the outer markers whenever the embedded slider adds a star
+  // (NEW_SAVEDREV). Observing the inner #ui-slider-bar covers saved revisions
+  // created live while history mode is open, which sync()'s scrub-driven
+  // callback would otherwise miss until the next slider move.
+  private observeInnerSavedRevisions(innerWin: Window): void {
+    if (this.savedRevObserver) return;
+    const bar = innerWin.document && innerWin.document.getElementById('ui-slider-bar');
+    if (!bar) return;
+    this.savedRevObserver = new MutationObserver(() => { this.renderSavedRevisionStars(innerWin); });
+    this.savedRevObserver.observe(bar, {childList: true});
+  }
+
   // Capture the live state we'll restore on exit: live chat message
   // visibility (just the timestamps — actual messages stay), live users
   // panel HTML, and current Export hrefs.

src/templates/pad.html

@@ -159,8 +159,15 @@
         </button>
         <button type="button" id="history-rightstep" class="buttonicon buttonicon-step-forward">
         </button>
-        <input type="range" id="history-slider-input" class="history-slider-input"
-               min="0" max="0" value="0" step="1">
+        <!-- Slider + saved-revision markers. The stars overlay sits above the
+             range track; pad_mode.ts fills it from the embedded timeslider's
+             savedRevisions so explicit "Save Revision" points stay visible in
+             in-pad history mode (issue #7946). -->
+        <span class="history-slider-wrap">
+          <input type="range" id="history-slider-input" class="history-slider-input"
+                 min="0" max="0" value="0" step="1">
+          <span id="history-slider-stars" class="history-slider-stars" aria-hidden="true"></span>
+        </span>
         <span id="history-timer" class="history-timer hide-for-mobile" aria-live="polite"></span>
         <!-- Follow toggle: an icon-only button. The eye is always rendered;
              the diagonal slash overlay is shown only when aria-pressed is

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

@@ -0,0 +1,97 @@
+import {expect, Page, test} from "@playwright/test";
+import {clearPadContent, goToNewPad, goToPad, writeToPad} from "../helper/padHelper";
+
+// Regression coverage for #7946: after #7659 moved the timeslider into the pad
+// as an embedded iframe, the user-facing control became the outer
+// #history-slider-input range input. Saved revisions ("Save Revision" button)
+// are still drawn as stars inside the now-hidden iframe slider, so they
+// stopped appearing for users. pad_mode.ts must bridge the saved revisions out
+// onto the outer slider as markers.
+test.describe('timeslider saved-revision markers', function () {
+  test.describe.configure({mode: 'serial'});
+
+  test.beforeEach(async ({context}) => {
+    await context.clearCookies();
+  });
+
+  const enterHistoryMode = async (page: Page) => {
+    await page.click('.buttonicon-history');
+    await page.waitForSelector('#history-controls:not([hidden])', {state: 'visible'});
+    await page.waitForSelector('#history-frame');
+  };
+
+  test('a saved revision shows a marker on the outer history slider', async function ({page}) {
+    await goToNewPad(page);
+    await clearPadContent(page);
+
+    // Build a few revisions, save one partway through, then add more so the
+    // saved marker lands in the middle of the slider (not under the thumb).
+    await writeToPad(page, 'One ');
+    await page.waitForTimeout(400);
+    await writeToPad(page, 'Two ');
+    await page.waitForTimeout(600);
+
+    // Save a revision at the current head.
+    await page.click('.buttonicon-savedRevision');
+    // The save confirmation gritter carries class `saved-revision`.
+    await page.waitForSelector('.saved-revision', {state: 'visible'});
+
+    // Add more edits so head advances past the saved revision.
+    await writeToPad(page, 'Three ');
+    await page.waitForTimeout(400);
+    await writeToPad(page, 'Four ');
+    await page.waitForTimeout(800);
+
+    await enterHistoryMode(page);
+
+    // The outer slider must show at least one visible saved-revision marker.
+    const marker = page.locator('.history-star');
+    await expect(marker.first()).toBeVisible({timeout: 15000});
+    expect(await marker.count()).toBeGreaterThanOrEqual(1);
+
+    // The marker must be positioned within the slider track, not collapsed to
+    // the origin. Assert on the inline left percentage directly (markers are
+    // always positioned via style.left = "N%"); a fallback to a layout-derived
+    // coordinate would let a degenerate left:0% render still pass.
+    const leftPct = await marker.first().evaluate(
+        (el) => parseFloat((el as HTMLElement).style.left));
+    expect(leftPct).toBeGreaterThan(0);
+    expect(leftPct).toBeLessThan(100);
+  });
+
+  test('a revision saved live appears on an already-open history slider', async function ({browser}) {
+    // Reviewer (history viewer).
+    const ctxA = await browser.newContext();
+    const pageA = await ctxA.newPage();
+    const padId = await goToNewPad(pageA);
+    await clearPadContent(pageA);
+    await writeToPad(pageA, 'Alpha Beta ');
+    await pageA.waitForTimeout(600);
+    await pageA.click('.buttonicon-savedRevision');
+    await pageA.waitForSelector('.saved-revision', {state: 'visible'});
+    await writeToPad(pageA, 'Gamma Delta ');
+    await pageA.waitForTimeout(800);
+    await enterHistoryMode(pageA);
+    await expect(pageA.locator('.history-star').first()).toBeVisible({timeout: 15000});
+    const before = await pageA.locator('.history-star').count();
+
+    // A second collaborator keeps editing the live pad and saves a revision
+    // while pageA is sitting in history mode. The server must broadcast
+    // NEW_SAVEDREV so pageA's open timeslider gains a marker without reloading.
+    const ctxB = await browser.newContext();
+    const pageB = await ctxB.newPage();
+    await goToPad(pageB, padId);
+    await writeToPad(pageB, 'Epsilon Zeta Eta ');
+    await pageB.waitForTimeout(800);
+    await pageB.click('.buttonicon-savedRevision');
+    await pageB.waitForSelector('.saved-revision', {state: 'visible'});
+
+    await expect.poll(
+        async () => await pageA.locator('.history-star').count(),
+        {timeout: 20000})
+        .toBeGreaterThan(before);
+
+    await ctxA.close();
+    await ctxB.close();
+  });
+});