oficina / meet Public

client: the Task 7 and Task 8 carry-overs

room.js:

- the fresh-stream ladder waited twice a round — once in the stream's
  own onerror and once again in diagnoseFresh — which doubled every
  round's backoff and made the ceiling take three times as long to
  reach as the numbers say. onerror now hands straight to the
  diagnosis, which keeps the single wait.
- the stall ceiling is a count *and* a stretch of time: eight rounds
  with no hello, and sixty seconds since the last one. Either alone
  gives up on something worth waiting for.
- signed out means a redirect that came out somewhere else. `redirected`
  on its own also covers a proxy normalising a URL in front of this app,
  and ending a working call with a sentence about signing in is not a
  thing to do on that evidence.
- pageshow's handler is no longer an async listener: its rejection had
  nowhere to land. Same for devices.js's, which fires startPreview and
  does not await it.
- every terminal screen now ends with the holding screen, `replaced`
  included — the takeover still comes first, as the answer, with the
  holding screen behind it as the fallback.

meet.css: the blanket `[hidden] { display: none !important }` becomes
`.meet-pickers[hidden] { display: none }`. Exactly one element in these
two pages is both hidden and given a display of its own, so exactly one
rule restores it — the framework gallery's own precedent.

room.html and join.html: aria-hidden on the two toggle state spans. The
accessible name is fixed in aria-label and the state is in aria-pressed;
the word in the span is for eyes, and leaving it in the name would make
the two disagree.

call.mjs: the renewal scenario is production-shaped — a six-second life
rather than two, which is above the client's own healthy-stream mark, so
it measures the renewal rather than the backoff ladder. HEALTHY_STREAM
is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CzvnuKhLzudvq7jZ7SvVt2

Paul Campbell pushed by paul@keymail.dev 1 month ago 6354af679f4395103fd181de9f984f644c6e03ad parent 03227f2

8 files changed, +112 −42

internal/meet/static/devices.js +7 −1
internal/meet/static/meet.css +9 −5
internal/meet/static/room.js +70 −20
internal/meet/templates/join.html +2 −2
internal/meet/templates/room.html +2 −2
internal/meettest/holding_test.go +2 −2
internal/meettest/room_test.go +2 −2
test/browser/call.mjs +18 −8

CI passed — run details

diff --git a/internal/meet/static/devices.js b/internal/meet/static/devices.js

index 7423036..40ad506 100644

--- a/internal/meet/static/devices.js

+++ b/internal/meet/static/devices.js

@@ -115,7 +115,13 @@

// Coming back through the back/forward cache restores the DOM but not

// the tracks pagehide stopped, so the preview is asked for again.

window.addEventListener('pageshow', (e) => {

- if (e.persisted && navigator.mediaDevices && navigator.mediaDevices.getUserMedia) startPreview();

+ if (!e.persisted || !navigator.mediaDevices || !navigator.mediaDevices.getUserMedia) return;

+ // startPreview is async and its result is not awaited here, so its

+ // rejection needs somewhere to land: an unhandled one is invisible

+ // to the person and to the console alike.

+ startPreview().catch((err) => {

+ console.error('meet: preview after a bfcache return', err);

+ });

});

applyToggles();

diff --git a/internal/meet/static/meet.css b/internal/meet/static/meet.css

index dfc0a48..6dd85d1 100644

--- a/internal/meet/static/meet.css

+++ b/internal/meet/static/meet.css

@@ -4,11 +4,15 @@

the theme rather than fixing its own colours. */

/* The hidden attribute is how both scripts show and hide whole regions,

- and its UA rule is display: none at author-origin-beating strength

- only until something here authors display on the same element —

- .meet-pickers and .meet-controls both do. This restores it. */

-[hidden] {

- display: none !important;

+ and the UA's display: none for it loses to any author rule that sets

+ display on the same element. Exactly one element in these two pages is

+ both hidden and given a display of its own — the holding screen's

+ pickers — so exactly one rule restores it, rather than a blanket

+ [hidden] override with !important, which is the framework gallery's

+ own precedent. Anything else authoring display on an element it also

+ hides needs its own line here, and should say so. */

+.meet-pickers[hidden] {

+ display: none;

}

.meet-grid {

diff --git a/internal/meet/static/room.js b/internal/meet/static/room.js

index 4b64fd7..9b6644f 100644

--- a/internal/meet/static/room.js

+++ b/internal/meet/static/room.js

@@ -64,8 +64,14 @@

// A stream refusal the probe cannot name — a full room behind a 204,

// a 500, a `last` the server would not parse — would otherwise retry

// for ever behind "Connecting to the call." After this many rounds

- // that produced no `hello`, the client stops and says so.

- const MAX_STALLS = 6;

+ // that produced no `hello`, and this long since the last one, the

+ // client stops and says so. Both conditions, because either alone

+ // gives up on something worth waiting for: a count alone would quit on

+ // a handful of fast failures at the start of the backoff ladder, and a

+ // clock alone would quit on one slow round that was still going to

+ // succeed.

+ const MAX_STALLS = 8;

+ const STALL_WINDOW = 60000;

// A stream that lived at least this long was a working stream, not a

// refusal: only that resets the backoff. A stream dying moments after

// its `hello` would otherwise reconnect in a tight loop.

@@ -107,6 +113,10 @@

let es = null; // the live EventSource

let backoff = BACKOFF_MIN;

let stalls = 0; // consecutive stream rounds that produced no hello

+ // When this browser last reached the room. It starts at load rather

+ // than at zero: a page that has never once got a `hello` has been

+ // trying since it opened, and that is the stretch STALL_WINDOW means.

+ let lastHelloAt = Date.now();

let reconnectTimer = 0;

let statsTimer = 0;

let fullTimer = 0;

@@ -356,6 +366,7 @@

function stall() {

stalls += 1;

if (stalls < MAX_STALLS) return false;

+ if (Date.now() - lastHelloAt < STALL_WINDOW) return false;

terminal('unreachable');

return true;

}

@@ -385,7 +396,15 @@

// resetting on the hello alone is how a tight reconnect loop

// starts.

if (Date.now() - openedAt > HEALTHY_STREAM) backoff = BACKOFF_MIN;

- later(state.peer ? resume : onFail);

+ if (state.peer) {

+ later(resume);

+ return;

+ }

+ // No participation to come back to, so this goes to the fresh

+ // ladder's own diagnosis — which waits before it retries. Waiting

+ // here as well would put two backoffs in every round, and double

+ // the ladder's own doubling.

+ onFail();

};

}

@@ -393,7 +412,7 @@

// No Accept header: the probe forms are exempt from the stream's

// text/event-stream gate and must not claim to be one.

const res = await fetch(path, { credentials: 'same-origin', cache: 'no-store' });

- return await readRefusal(res, 204);

+ return await readRefusal(res, 204, path);

}

// readRefusal turns one response into the shape every caller here

@@ -404,7 +423,14 @@

// /signin, fetch follows it, and what comes back is a 200 carrying a

// sign-in page, which read as a status alone is indistinguishable

// from success.

- async function readRefusal(res, emptyOK) {

+ //

+ // A redirect on its own is not that, though, which is why `asked` is

+ // passed in: a redirect that lands back on the path it was sent to —

+ // a proxy normalising a URL, a trailing slash, anything a deployment

+ // might do in front of this app — is not a sign-out, and treating it

+ // as one would end a working call with a sentence about signing in.

+ // Only a redirect that came out somewhere else counts.

+ async function readRefusal(res, emptyOK, asked) {

let data = null;

if (res.status !== emptyOK) {

try {

@@ -413,11 +439,22 @@

data = null;

}

- const signedOut = res.redirected || res.status === 401 || res.status === 403 ||

+ const signedOut = wentElsewhere(res, asked) || res.status === 401 || res.status === 403 ||

(res.status === 200 && res.status !== emptyOK && data === null);

return { status: res.status, data: data || {}, signedOut, retryAfter: res.headers.get('Retry-After') };

}

+ function wentElsewhere(res, asked) {

+ if (!res.redirected) return false;

+ try {

+ return new URL(res.url).pathname !== new URL(asked, location.origin).pathname;

+ } catch {

+ // A URL neither of them can parse is not one this browser asked

+ // for: treat it the way the old check did.

+ return true;

+ }

function later(fn) {

if (state.stopped) return;

clearTimeout(reconnectTimer);

@@ -455,6 +492,7 @@

// backoff itself is cleared by attach, and only for a stream that

// then stayed open.

stalls = 0;

+ lastHelloAt = Date.now();

const fresh = !state.peer;

state.peer = msg.peer || null;

state.boot = msg.boot || state.boot;

@@ -962,15 +1000,16 @@

// postSignal is the one place this file talks upstream. csrf.Protect

// is origin-based, so a same-origin fetch needs no token.

async function postSignal(body, keepalive) {

+ const path = '/m/' + encodeURIComponent(sqid) + '/signal';

try {

- const res = await fetch('/m/' + encodeURIComponent(sqid) + '/signal', {

+ const res = await fetch(path, {

method: 'POST',

credentials: 'same-origin',

headers: { 'Content-Type': 'application/json' },

body: JSON.stringify(body),

keepalive: !!keepalive,

});

- return await readRefusal(res, 200);

+ return await readRefusal(res, 200, path);

} catch {

return { status: 0, data: {}, signedOut: false, retryAfter: null };

}

@@ -1038,21 +1077,21 @@

wayOut(reason);

}

- // wayOut is the rule that no terminal screen is a dead end. `replaced`

- // gets the takeover confirm page and nothing else — it is the one

- // sentence with a real next step, and a second link beside it would

- // only compete with it. Being signed out is answered by signing in.

- // Everything else goes back to the holding screen, which is where a

- // person can look at their camera and try again.

+ // wayOut is the rule that no terminal screen is a dead end. Being

+ // signed out is answered by signing in, and by nothing else: the

+ // holding screen would only bounce this person to the same sign-in

+ // page. Every other reason ends with the holding screen, which is

+ // where a person can look at their camera and try again, and two of

+ // them offer something better first — `replaced` the takeover, which

+ // is what this tab is actually for, and `unreachable` another go at

+ // the room. The specific link comes first, so it reads as the answer

+ // and the holding screen as the fallback.

function wayOut(reason) {

- if (reason === 'replaced') {

- link('/m/' + encodeURIComponent(sqid) + '/takeover', 'Use this tab instead');

- return;

- }

if (reason === 'signedout') {

link('/signin', 'Sign in again');

return;

}

+ if (reason === 'replaced') link('/m/' + encodeURIComponent(sqid) + '/takeover', 'Use this tab instead');

if (reason === 'unreachable') link('/m/' + encodeURIComponent(sqid), 'Try again');

link('/m/' + encodeURIComponent(sqid) + '/join', 'Back to the holding screen');

}

@@ -1344,13 +1383,24 @@

leave();

});

- window.addEventListener('pageshow', async (e) => {

+ // The listener itself is not async: an async handler's rejection goes

+ // nowhere an event target can report it, and getUserMedia is one await

+ // away. The work is a named function with a catch of its own, so a

+ // camera that will not restart after a bfcache return is a logged

+ // error and a call that carries on, not a silent unhandled rejection.

+ window.addEventListener('pageshow', (e) => {

+ restoreFromCache(e).catch((err) => {

+ console.error('meet: pageshow', err);

+ });

+ async function restoreFromCache(e) {

if (!e.persisted || state.stopped || state.leaveSent) return;

if (!local) await startLocal();

renderTiles();

if (state.peer) resume();

else connectStream();

- });

+ }

// ---------------------------------------------------------------

// The test hook (Task 9's Playwright gate)

diff --git a/internal/meet/templates/join.html b/internal/meet/templates/join.html

index 363e583..1a61704 100644

--- a/internal/meet/templates/join.html

+++ b/internal/meet/templates/join.html

@@ -18,8 +18,8 @@

<div id="speaker-row" hidden>{{template "field-select" dict "ID" "pick-speaker" "Name" "speaker" "Label" "Speaker"}}</div>

</div>

- <button type="button" id="toggle-mute" rst-btn aria-label="Microphone" aria-pressed="false">Microphone<span id="toggle-mute-state" class="meet-toggle-state">On</span></button>

- <button type="button" id="toggle-camera" rst-btn aria-label="Camera" aria-pressed="false">Camera<span id="toggle-camera-state" class="meet-toggle-state">On</span></button>

+ <button type="button" id="toggle-mute" rst-btn aria-label="Microphone" aria-pressed="false">Microphone<span id="toggle-mute-state" class="meet-toggle-state" aria-hidden="true">On</span></button>

+ <button type="button" id="toggle-camera" rst-btn aria-label="Camera" aria-pressed="false">Camera<span id="toggle-camera-state" class="meet-toggle-state" aria-hidden="true">On</span></button>

</div>

diff --git a/internal/meet/templates/room.html b/internal/meet/templates/room.html

index e32906f..eb37853 100644

--- a/internal/meet/templates/room.html

+++ b/internal/meet/templates/room.html

@@ -12,8 +12,8 @@

- <button type="button" id="mute" rst-btn aria-label="Microphone" aria-pressed="false">Microphone<span id="mute-state" class="meet-toggle-state">On</span></button>

- <button type="button" id="camera" rst-btn aria-label="Camera" aria-pressed="false">Camera<span id="camera-state" class="meet-toggle-state">On</span></button>

+ <button type="button" id="mute" rst-btn aria-label="Microphone" aria-pressed="false">Microphone<span id="mute-state" class="meet-toggle-state" aria-hidden="true">On</span></button>

+ <button type="button" id="camera" rst-btn aria-label="Camera" aria-pressed="false">Camera<span id="camera-state" class="meet-toggle-state" aria-hidden="true">On</span></button>

<button type="button" id="leave" rst-btn="danger">Leave</button>

</div>

<div id="queue" class="meet-queue" hidden><h2>Waiting to join</h2><ul id="knocks"></ul></div>

diff --git a/internal/meettest/holding_test.go b/internal/meettest/holding_test.go

index 362b9ce..cf3f17d 100644

--- a/internal/meettest/holding_test.go

+++ b/internal/meettest/holding_test.go

@@ -144,9 +144,9 @@ func TestHoldingPageTogglesNameThemselvesAndShowTheirState(t *testing.T) {

body := c.get("/m/" + sqid + "/join").Body.String()

for _, want := range []string{

`id="toggle-mute" rst-btn aria-label="Microphone" aria-pressed="false"`,

- `<span id="toggle-mute-state" class="meet-toggle-state">On</span>`,

+ `<span id="toggle-mute-state" class="meet-toggle-state" aria-hidden="true">On</span>`,

`id="toggle-camera" rst-btn aria-label="Camera" aria-pressed="false"`,

- `<span id="toggle-camera-state" class="meet-toggle-state">On</span>`,

+ `<span id="toggle-camera-state" class="meet-toggle-state" aria-hidden="true">On</span>`,

} {

if !strings.Contains(body, want) {

t.Fatalf("holding page's toggles: missing %q\n%s", want, body)

diff --git a/internal/meettest/room_test.go b/internal/meettest/room_test.go

index d99c5dc..e2ab5b4 100644

--- a/internal/meettest/room_test.go

+++ b/internal/meettest/room_test.go

@@ -459,9 +459,9 @@ func TestRoomPageLoadsRoomScriptFromTheHeadAndHidesTheRoom(t *testing.T) {

// word room.js rewrites, never in colour alone.

for _, want := range []string{

`id="mute" rst-btn aria-label="Microphone" aria-pressed="false"`,

- `<span id="mute-state" class="meet-toggle-state">On</span>`,

+ `<span id="mute-state" class="meet-toggle-state" aria-hidden="true">On</span>`,

`id="camera" rst-btn aria-label="Camera" aria-pressed="false"`,

- `<span id="camera-state" class="meet-toggle-state">On</span>`,

+ `<span id="camera-state" class="meet-toggle-state" aria-hidden="true">On</span>`,

} {

if !strings.Contains(body, want) {

t.Fatalf("room page's toggles: missing %q\n%s", want, body)

diff --git a/test/browser/call.mjs b/test/browser/call.mjs

index 0faadad..42d706c 100644

--- a/test/browser/call.mjs

+++ b/test/browser/call.mjs

@@ -443,9 +443,14 @@ const run = {

await ctxB.close();

- // A stream lives five minutes and is then renewed by a resume. Two

+ // A stream lives five minutes and is then renewed by a resume. Six

// seconds here, through a header only development honours, so the

- // renewal happens several times while the call is being watched.

+ // renewal happens several times while the call is watched — and six

+ // rather than two because a stream shorter than the client's own

+ // "this one was healthy" mark never clears its backoff, so the

+ // renewals would slow down and this scenario would be measuring the

+ // backoff ladder rather than the renewal. The client's mark stays

+ // where it is; the header moves.

async renewal() {

const ctxA = await newContext('member A', { cookie: cookieA });

const ctxB = await newContext('member B', { cookie: cookieB });

@@ -458,7 +463,7 @@ const run = {

await ctx.route('**/stream*', async (route) => {

const url = route.request().url();

if (!url.includes('probe=1')) attaches += 1;

- await route.continue({ headers: { ...route.request().headers(), 'x-meet-test-ttl': '2s' } });

+ await route.continue({ headers: { ...route.request().headers(), 'x-meet-test-ttl': '6s' } });

});

}

const pageA = await openRoom(ctxA);

@@ -472,8 +477,9 @@ const run = {

}

const before = { a: await state(pageA), b: await state(pageB) };

- // Long enough for at least four renewals at a two-second life.

- await sleep(10000);

+ // Long enough for at least one renewal each at a six-second life,

+ // and in practice two.

+ await sleep(16000);

const after = { a: await state(pageA), b: await state(pageB) };

check(after.a.peer === before.a.peer,

@@ -487,10 +493,14 @@ const run = {

check((after.a.frames[before.b.peer] || 0) > (before.a.frames[before.b.peer] || 0),

'A is still receiving video after the renewals (' +

(before.a.frames[before.b.peer] || 0) + ' -> ' + (after.a.frames[before.b.peer] || 0) + ')');

- // Two browsers, a two-second life, ten seconds of watching: the

+ // Two browsers, a six-second life, sixteen seconds of watching: the

// first two attaches are the two fresh streams, and everything past

- // that is a renewal that actually happened.

- check(attaches >= 6, 'the streams really were renewed (' + attaches + ' attaching GETs, want at least 6)');

+ // that is a renewal that actually happened. The floor is four — one

+ // renewal each — rather than the number a stopwatch would predict,

+ // because what this counts is that the header was honoured at all: a

+ // server that ignored it would leave both calls up on one

+ // five-minute stream and stop at two.

+ check(attaches >= 4, 'the streams really were renewed (' + attaches + ' attaching GETs, want at least 4)');

await leaveRoom(pageA);

await leaveRoom(pageB);