oficina / slides Public

Presenter view and remote: stop reloading in a loop

present.js reads the slide a page is showing from data-slide on its
root. present_notes.html and remote.html never set it, so the script
took both pages to be on slide 0, the stream's first event (slide 1)
"moved" them, and they reloaded — on every load, ~50 times a second
(polish audit B3).

- Both roots now carry data-slide (presented.Now, the same 1-based
  number the stream sends).
- The two reload-to-follow pages go through follow(): reload only when
  the event's slide or blackout differs from what was rendered, and at
  most once per state, remembered in sessionStorage across exactly one
  reload. A root without data-slide, or a session pointing past a deck
  that got shorter (the server clamps), now costs one reload, not a loop.
- Blackout changes now reload those two pages too: both render the
  blackout pill and the button label server-side.

Tests: TestEveryPresentingPageSaysWhichSlideItShows (plain suite) and
TestBrowserPresentingScreensHoldStill (-tags browser: counts top-level
navigations; 102/75 per 3 s before, 0 after; exactly one per slide move).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 3 weeks ago 5856dd54f9b4871c5563687b54c030a3329d51cc parent 32b52c2

6 files changed, +194 −9

internal/slides/presentation.go +12 −2
internal/slides/static/present.js +42 −7
internal/slides/templates/present_notes.html +1 −0
internal/slides/templates/remote.html +1 −0
internal/slidestest/browser_present_test.go +108 −0
internal/slidestest/present_test.go +30 −0

diff --git a/internal/slides/presentation.go b/internal/slides/presentation.go

index 8cf083a..4e99a54 100644

--- a/internal/slides/presentation.go

+++ b/internal/slides/presentation.go

@@ -472,6 +472,7 @@ type presenterPage struct {

Session string

Token string

+ Now int

Counter string

CurrentLabel string

Current template.HTML

@@ -521,6 +522,7 @@ type remotePage struct {

DeckTitle string

Session string

+ Now int

Counter string

CurrentLabel string

Current template.HTML

@@ -792,7 +794,7 @@ func (a *app) presenterNotes(w http.ResponseWriter, r *http.Request) {

ToolsLabel: rastrillo.T(r, "slides.viewer.tools"),

}

shown := presentedFor(r, view, p)

- page.Counter, page.Current = shown.Counter, shown.Current

+ page.Now, page.Counter, page.Current = shown.Now, shown.Counter, shown.Current

page.Next, page.HasNext, page.Notes = shown.Next, shown.HasNext, shown.Notes

if holder {

@@ -845,6 +847,13 @@ func clockWords(t time.Time) string {

// a hotel wifi. The projector at /p/{session} still gets the real

// rendering, because that one is the slide.

type presented struct {

+ // Now is the 1-based slide these were rendered for — the number the

+ // stream's events carry — and it goes on the page's script root as

+ // data-slide, so static/present.js knows what it is already showing.

+ // Without it the script took the page to be on slide 0, the first

+ // event "moved" it, and the page reloaded itself on every load

+ // (polish audit B3).

+ Now int

Counter string

Current template.HTML

Next template.HTML

@@ -863,6 +872,7 @@ func presentedFor(r *http.Request, view *deck.Deck, p Presentation) presented {

}

index := clampSlide(p.Slide, count)

out := presented{

+ Now: index + 1,

Counter: rastrillo.Tf(r, "slides.present.progress", "n", index+1, "total", count),

Current: slideThumbImg(r, view, p, index),

}

@@ -965,7 +975,7 @@ func (a *app) remote(w http.ResponseWriter, r *http.Request) {

ToolsLabel: rastrillo.T(r, "slides.viewer.tools"),

}

shown := presentedFor(r, view, p)

- page.Counter, page.Current = shown.Counter, shown.Current

+ page.Now, page.Counter, page.Current = shown.Now, shown.Counter, shown.Current

page.Next, page.HasNext, page.Notes = shown.Next, shown.HasNext, shown.Notes

render(w, "remote", page)

}

diff --git a/internal/slides/static/present.js b/internal/slides/static/present.js

index 8d46682..5cc4509 100644

--- a/internal/slides/static/present.js

+++ b/internal/slides/static/present.js

@@ -105,13 +105,6 @@

// event this page understands and is left alone rather than fetched.

if (!(slide > 0) || slide === requested) return;

requested = slide;

- if (!stage || !fragmentHref) {

- // The presenter view and the remote show two slides and the

- // notes; re-rendering all that in here would be a second

- // renderer to keep in step with the Go one.

- window.location.reload();

- return;

- }

var mine = ++newest;

var back = function () {

// Only this request's own failure rolls the record back, and

@@ -131,12 +124,54 @@

.catch(back);

}

+ // The pages with no stage to swap — the presenter view and the

+ // remote — follow by reloading: they show two slides, the notes and

+ // the blackout state, and re-rendering all that in here would be a

+ // second renderer to keep in step with the Go one. Their root says

+ // what they were rendered with (data-slide, data-blackout), and they

+ // reload only when an event says something else.

+ //

+ // And at most once for any one state. A page that has just reloaded

+ // for slide 3 and still says something else — a root missing its

+ // data-slide, or a session pointing past a deck that has since got

+ // shorter, which the server clamps — would otherwise reload again on

+ // the stream's first event, and again after that, forever: the

+ // presenter view once did exactly that about fifty times a second

+ // (polish audit B3). The record is taken off at load, so it only

+ // ever guards the load straight after the reload that wrote it.

+ var renderedBlackout = root.getAttribute('data-blackout') === 'true';

+ var reloadKey = 'slides-present-reload:' + window.location.pathname;

+ var reloadedFor = null;

+ try {

+ reloadedFor = window.sessionStorage.getItem(reloadKey);

+ window.sessionStorage.removeItem(reloadKey);

+ } catch (err) { /* no storage: see follow */ }

+ function follow(state) {

+ var slide = Number(state.slide);

+ if (!(slide > 0)) return;

+ var blackout = !!state.blackout;

+ if (slide === displayed && blackout === renderedBlackout) return;

+ var want = String(slide) + (blackout ? ':blackout' : '');

+ if (want === reloadedFor) return;

+ try {

+ window.sessionStorage.setItem(reloadKey, want);

+ } catch (err) {

+ // Nowhere to remember that this reload happened, so only reload

+ // a page that at least knows which slide it is on.

+ if (!displayed) return;

+ }

+ reloadedFor = want;

+ window.location.reload();

+ }

// apply is the whole of what an event does. The state is complete —

// never a delta — so applying one is exactly right rather than

// approximately, and a page that missed some is right again on the

// next one.

function apply(state) {

if (state.ended) { window.location.reload(); return; }

+ if (!stage || !fragmentHref) { follow(state); return; }

root.setAttribute('data-blackout', state.blackout ? 'true' : 'false');

if (note) note.hidden = !state.blackout;

show(Number(state.slide));

diff --git a/internal/slides/templates/present_notes.html b/internal/slides/templates/present_notes.html

index 221dd2b..d8d9469 100644

--- a/internal/slides/templates/present_notes.html

+++ b/internal/slides/templates/present_notes.html

@@ -110,6 +110,7 @@

A presenter view opened without the token watches and cannot

drive, which is exactly what the buttons above do too. */}}

<div slides-preview data-session="{{.Session}}"

+ data-slide="{{.Now}}"

data-blackout="{{if .Blackout}}true{{else}}false{{end}}"

{{- if .Token}} data-remote-token="{{.Token}}"{{end}}>

<div>

diff --git a/internal/slides/templates/remote.html b/internal/slides/templates/remote.html

index db74c99..8e984aa 100644

--- a/internal/slides/templates/remote.html

+++ b/internal/slides/templates/remote.html

@@ -61,6 +61,7 @@

<div slides-remote data-session="{{.Session}}" data-remote-token="{{.Controls.Token}}"

+ data-slide="{{.Now}}"

data-blackout="{{if .Blackout}}true{{else}}false{{end}}">

<div>

diff --git a/internal/slidestest/browser_present_test.go b/internal/slidestest/browser_present_test.go

new file mode 100644

index 0000000..13ed87b

--- /dev/null

+++ b/internal/slidestest/browser_present_test.go

@@ -0,0 +1,108 @@

+//go:build browser

+// The presenting screens with their script on. The Go tests read the

+// markup; only a browser runs static/present.js against it, and the

+// failure this file was written for (polish audit B3, 2026-09-23) was

+// the script and the markup disagreeing: the presenter view and the

+// remote carried no data-slide, so the page believed it was on slide

+// 0, the stream's first event said slide 1, and the page reloaded —

+// and did the same on every load after, about fifty times a second.

+package slidestest

+import (

+ "regexp"

+ "sync/atomic"

+ "testing"

+ "time"

+ "github.com/chromedp/cdproto/page"

+ "github.com/chromedp/chromedp"

+ "github.com/chromedp/chromedp/kb"

+var notesLocation = regexp.MustCompile(`^(/p/[23456789abcdefghjkmnpqrstuvwxyz]{10})/notes\?t=([A-Za-z0-9_-]{43})$`)

+// TestBrowserPresentingScreensHoldStill: the presenter view and the

+// phone remote each load once and then stay put while nothing

+// changes, and each reloads exactly once when the slide does move —

+// which is how those two pages follow a talk (present.js reloads them

+// rather than re-rendering two slides and the notes itself).

+func TestBrowserPresentingScreensHoldStill(t *testing.T) {

+ rig, mailer := newRig(t)

+ browserSignIn(t, rig, mailer)

+ // Every top-level navigation the tab makes, reloads included.

+ var loads atomic.Int64

+ chromedp.ListenTarget(rig.Context(), func(ev any) {

+ if e, ok := ev.(*page.EventFrameNavigated); ok && e.Frame.ParentID == "" {

+ loads.Add(1)

+ }

+ })

+ var deck string

+ rig.Run(chromedp.Evaluate(`(document.querySelector('a[href^="/d/"]')?.getAttribute('href') ?? '')`, &deck))

+ if !deckPath.MatchString(deck) {

+ t.Fatalf("the deck list links no deck: %q", deck)

+ }

+ var path string

+ rig.Run(

+ chromedp.Navigate(rig.Origin+deck),

+ chromedp.Click(`form[action$="/present"] button`, chromedp.ByQuery),

+ waitFor(`location.pathname.endsWith('/notes')`),

+ chromedp.Evaluate(`location.pathname + location.search`, &path),

+ )

+ m := notesLocation.FindStringSubmatch(path)

+ if m == nil {

+ t.Fatalf("Present landed on %q, want /p/{session}/notes?t={token}", path)

+ }

+ session, token := m[1], m[2]

+ // holdsStill waits for the page to settle and the stream to have

+ // said its piece (one event on connect), then counts loads over a

+ // window long enough for the broken page to have reloaded hundreds

+ // of times.

+ holdsStill := func(name string) {

+ t.Helper()

+ rig.Run(chromedp.Sleep(1500 * time.Millisecond))

+ before := loads.Load()

+ rig.Run(chromedp.Sleep(3 * time.Second))

+ if n := loads.Load() - before; n != 0 {

+ t.Errorf("the %s loaded %d more times in 3s with nothing changing, want 0", name, n)

+ }

+ // And it holds still because it knows where it is, not only

+ // because present.js refuses to reload twice for one slide:

+ // the script's root names the slide the counter shows.

+ var marked, counter string

+ rig.Run(

+ chromedp.Evaluate(`document.querySelector('[data-session]')?.getAttribute('data-slide') ?? 'missing'`, &marked),

+ chromedp.Evaluate(`(document.querySelector('[slides-counter]')?.textContent ?? '').trim().split(' ')[0]`, &counter),

+ )

+ if marked != counter {

+ t.Errorf("the %s's script root says data-slide=%q; the counter shows slide %q", name, marked, counter)

+ }

+ // movesOnce presses the right arrow — present.js posts `next` — and

+ // the page must reload onto the new slide once, and only once.

+ movesOnce := func(name, counter string) {

+ t.Helper()

+ before := loads.Load()

+ rig.Run(

+ chromedp.KeyEvent(kb.ArrowRight),

+ waitFor(`document.querySelector('[slides-counter]')?.textContent.trim().startsWith('`+counter+` /')`),

+ chromedp.Sleep(3*time.Second),

+ )

+ if n := loads.Load() - before; n != 1 {

+ t.Errorf("the %s loaded %d times after one slide change, want exactly 1", name, n)

+ }

+ holdsStill("presenter view")

+ rig.Screen("body", "presenter view")

+ movesOnce("presenter view", "2")

+ rig.Run(chromedp.Navigate(rig.Origin + session + "/remote?t=" + token))

+ holdsStill("remote")

+ rig.Screen("body", "remote")

+ movesOnce("remote", "3")

diff --git a/internal/slidestest/present_test.go b/internal/slidestest/present_test.go

index b482905..fd69a71 100644

--- a/internal/slidestest/present_test.go

+++ b/internal/slidestest/present_test.go

@@ -567,3 +567,33 @@ func TestFollowOfferedOnlyWhileLive(t *testing.T) {

t.Errorf("following an ended session: status %d, want 303 back to the deck", rec.Code)

}

+// TestEveryPresentingPageSaysWhichSlideItShows: every page that loads

+// static/present.js puts the slide it was rendered for on the script's

+// root as data-slide, the same 1-based number the stream's events

+// carry. The presenter view and the remote once had none, so the

+// script took them to be on slide 0 and reloaded on every event —

+// the stream's first included — forever (polish audit B3). The

+// browser half is TestBrowserPresentingScreensHoldStill; this one is

+// in the plain suite so the gate holds it too.

+func TestEveryPresentingPageSaysWhichSlideItShows(t *testing.T) {

+ a := newApp(t)

+ cookies := signedIn(t, a, allowedAddress)

+ path := sampleDeck(t, a, cookies)

+ session, token := startPresenting(t, a, path, cookies)

+ command(t, a, session, "next", token, "", nil)

+ for _, page := range []string{

+ "/p/" + session + "?t=" + token,

+ "/p/" + session + "/notes?t=" + token,

+ "/p/" + session + "/remote?t=" + token,

+ } {

+ body := mustGet(t, a, page, http.StatusOK, cookies).Body.String()

+ if !strings.Contains(body, `static/present`) {

+ t.Errorf("%s no longer loads present.js; this test is about the pages that do", page)

+ }

+ if !strings.Contains(body, `data-slide="2"`) {

+ t.Errorf("%s: the script root does not say data-slide=\"2\" after one next:\n%s", page, body)

+ }