oficina / meet Public

devices.js fix round 1: races, error handling, field-select, a11y

Fix round 1 of 5 on Task 7 (branch head was 9911b2c; room.js, meet.css
and room_test.go untouched throughout).

- fillPickers: enumerateDevices wrapped in try (a rejection there no
  longer aborts row-hiding); Join is enabled before the await on both
  the success and failure paths of startPreview, and the fillPickers
  call itself is defensively wrapped in .catch(() => {}), so a
  rejection can no longer leave Join disabled forever.
- startPreview: a generation counter guards against a concurrent call
  (rapid picker changes) leaking a stream or clobbering a newer one -
  a stale success stops its own tracks and returns; a stale failure
  returns without touching prefs or the UI.
- A denied permission (NotAllowedError) no longer erases the
  remembered camera/mic ids. Clearing a remembered id now only
  happens on OverconstrainedError or NotFoundError, one id per retry
  (camera first, then mic) rather than both at once, since
  err.constraint names the failing constraint (typically "deviceId")
  but not which media kind it belonged to.
- No navigator.mediaDevices at all (insecure context, old browser):
  #devices is still revealed, but the preview and pickers stay
  hidden, one sentence explains it in #status, and Join is enabled
  immediately.
- The three pickers are now built from the vendored field-select
  partial (AGENTS.md's hard Fields rule), wrapped in #camera-row,
  #mic-row and #speaker-row; each row hides itself whenever its own
  device count is 0 (recomputed on every fillPickers call, including
  on devicechange), and the speaker row additionally requires
  setSinkId support.
- Mute/camera-off toggle buttons keep a fixed accessible name
  ("Microphone", "Camera") and move only aria-pressed, matching
  room.js's own toggles and meet.css's existing
  [aria-pressed='true'] cue (no meet.css change needed: the buttons
  already sit inside .meet-controls).
- New sentences: NotReadableError ("already in use by another app")
  gets its own message, distinct from the generic "could not reach a
  camera or microphone"; an "Asking for your camera and
  microphone..." status precedes the getUserMedia call.
- The initial getUserMedia attempt now waits for
  document.visibilityState === 'visible' (with a one-shot
  visibilitychange listener) rather than firing in a background tab.
- location.assign in the Join handler is guarded to only ever follow
  a path starting with '/'.
- The camera preview <video> carries aria-label="Your camera
  preview"; #devices keeps its data-sqid for Task 9.
- New test: #devices renders with the hidden attribute in the
  server HTML (checked immediately after data-join-href, so it is
  unambiguously #devices's own attribute); confirmed to fail when
  hidden is removed from the template before restoring the fix.
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 3a209a3b72f599718a67200043faf0038502b2f7 parent 9911b2c
3 files changed, +165 −65
  • internal/meet/static/devices.js +141 −58
  • internal/meet/templates/join.html +7 −7
  • internal/meettest/holding_test.go +17 −0

CI passed — run details

diff --git a/internal/meet/static/devices.js b/internal/meet/static/devices.js
index 97a3fcc..880f7c0 100644
--- a/internal/meet/static/devices.js
+++ b/internal/meet/static/devices.js
@@ -13,22 +13,31 @@
(() => {
'use strict';
const root = document.getElementById('devices');
- if (!root || !navigator.mediaDevices) return;
- root.hidden = false;
+ if (!root) return;
const STORAGE_KEY = 'meet.devices';
const $ = (id) => document.getElementById(id);
const preview = $('preview');
+ const pickers = $('pickers');
+ const cameraRow = $('camera-row');
+ const micRow = $('mic-row');
+ const speakerRow = $('speaker-row');
const pickCamera = $('pick-camera');
const pickMic = $('pick-mic');
const pickSpeaker = $('pick-speaker');
- const speakerRow = $('speaker-row');
const muteButton = $('toggle-mute');
const cameraButton = $('toggle-camera');
const joinButton = $('join');
const status = $('status');
let stream = null;
+ // Bumped on every startPreview() call and captured at its start; a
+ // call whose generation no longer matches when its getUserMedia
+ // settles was superseded by a later device change (a quick switch
+ // between two selects) and must not touch the shared `stream` or
+ // the UI — it stops whatever tracks it was handed and gets out of
+ // the way instead of leaking them or clobbering the newer attempt.
+ let generation = 0;
const prefs = loadPrefs();
// --- preferences: read and written to localStorage only ---
@@ -54,46 +63,124 @@
status.textContent = text;
}
+ // --- mute and camera-off defaults: applied to the live tracks when ---
+ // --- there is a stream, always reflected on the toggle buttons ---
+ // --- even when there isn't one, so the choice still holds at join. ---
+ //
+ // The buttons keep one fixed accessible name each ("Microphone",
+ // "Camera") and move only aria-pressed: a button whose name changes
+ // on every click reads, to a screen reader tracking it, as a
+ // different control each time. meet.css already gives
+ // [aria-pressed="true"] a non-colour cue (room.js's own toggles use
+ // the same rule), so nothing further is needed here beyond setting
+ // the attribute.
+ function applyToggles() {
+ if (stream) {
+ for (const track of stream.getAudioTracks()) track.enabled = !prefs.muted;
+ for (const track of stream.getVideoTracks()) track.enabled = !prefs.cameraOff;
+ }
+ muteButton.setAttribute('aria-pressed', String(!!prefs.muted));
+ cameraButton.setAttribute('aria-pressed', String(!!prefs.cameraOff));
+ }
+
+ function stopPreview() {
+ if (!stream) return;
+ for (const track of stream.getTracks()) track.stop();
+ stream = null;
+ }
+
+ // --- wiring that needs neither media support nor a stream ---
+
+ muteButton.addEventListener('click', () => {
+ prefs.muted = !prefs.muted;
+ savePrefs();
+ applyToggles();
+ });
+ cameraButton.addEventListener('click', () => {
+ prefs.cameraOff = !prefs.cameraOff;
+ savePrefs();
+ applyToggles();
+ });
+ joinButton.addEventListener('click', () => {
+ stopPreview();
+ const href = root.dataset.joinHref || '';
+ // Belt and braces: only ever navigate to a path this page itself
+ // rendered, never to whatever a mangled attribute happened to hold.
+ if (href.startsWith('/')) location.assign(href);
+ });
+ window.addEventListener('pagehide', stopPreview);
+
+ applyToggles();
+ root.hidden = false;
+
+ if (!navigator.mediaDevices || !navigator.mediaDevices.getUserMedia) {
+ // An insecure context or a browser old enough to lack the API:
+ // the pickers and preview have nothing to show, so they stay
+ // hidden rather than sitting there empty and inert. Join still
+ // works — the room is built to allow joining with no local media.
+ preview.hidden = true;
+ pickers.hidden = true;
+ say('This browser cannot reach a camera or microphone here. You can still join with neither.');
+ joinButton.disabled = false;
+ return;
+ }
+
// --- the preview stream ---
async function startPreview() {
+ const myGen = ++generation;
stopPreview();
+ say('Asking for your camera and microphone…');
const constraints = {
audio: prefs.mic ? { deviceId: { exact: prefs.mic } } : true,
video: prefs.camera ? { deviceId: { exact: prefs.camera } } : true,
};
+ let result;
try {
- stream = await navigator.mediaDevices.getUserMedia(constraints);
+ result = await navigator.mediaDevices.getUserMedia(constraints);
} catch (err) {
- if (prefs.camera || prefs.mic) {
- // The remembered device may be gone — unplugged, or its own
- // permission revoked — so forget it and try once more with no
- // constraint before giving up on media entirely.
- delete prefs.camera;
- delete prefs.mic;
+ if (myGen !== generation) return; // superseded; its outcome no longer matters
+ if ((err.name === 'OverconstrainedError' || err.name === 'NotFoundError') && (prefs.camera || prefs.mic)) {
+ // A remembered device is gone — unplugged, or no longer
+ // matching. err.constraint (typically "deviceId") names the
+ // constraint that failed but not which of camera/mic it
+ // belonged to, so only one remembered id is dropped per
+ // attempt — camera first, then mic — rather than discarding
+ // both on a single ambiguous failure; this converges within
+ // two retries at most.
+ if (prefs.camera) delete prefs.camera;
+ else delete prefs.mic;
savePrefs();
return startPreview();
}
preview.srcObject = null;
- say(err.name === 'NotAllowedError'
- ? 'Meet needs permission to use your camera and microphone. You can still join with neither.'
- : 'No camera or microphone was found. You can still join with neither.');
+ say(errorSentence(err));
applyToggles();
- await fillPickers();
joinButton.disabled = false;
+ await fillPickers().catch(() => {});
+ return;
+ }
+ if (myGen !== generation) {
+ // A newer request already won; this one's tracks are unwanted.
+ for (const track of result.getTracks()) track.stop();
return;
}
+ stream = result;
preview.srcObject = stream;
applyToggles();
- await fillPickers();
joinButton.disabled = false;
+ await fillPickers().catch(() => {});
say('');
}
- function stopPreview() {
- if (!stream) return;
- for (const track of stream.getTracks()) track.stop();
- stream = null;
+ function errorSentence(err) {
+ if (err.name === 'NotAllowedError') {
+ return 'Meet needs permission to use your camera and microphone. You can still join with neither.';
+ }
+ if (err.name === 'NotReadableError') {
+ return 'Your camera or microphone is already in use by another app. You can still join with neither.';
+ }
+ return 'Meet could not reach a camera or microphone. You can still join with neither.';
}
// --- pickers: (re)built from enumerateDevices, which only carries ---
@@ -101,7 +188,14 @@
// --- this always runs after a getUserMedia attempt, never before. ---
async function fillPickers() {
- const devices = await navigator.mediaDevices.enumerateDevices();
+ let devices;
+ try {
+ devices = await navigator.mediaDevices.enumerateDevices();
+ } catch {
+ // Treated as "no devices found" below, which hides every row
+ // rather than leaving stale or half-filled selects on screen.
+ devices = [];
+ }
const fillOne = (select, kind, chosen) => {
select.replaceChildren();
let count = 0;
@@ -116,12 +210,17 @@
}
return count;
};
- fillOne(pickCamera, 'videoinput', prefs.camera || currentDeviceId('video'));
- fillOne(pickMic, 'audioinput', prefs.mic || currentDeviceId('audio'));
- if (typeof HTMLMediaElement.prototype.setSinkId === 'function' &&
- fillOne(pickSpeaker, 'audiooutput', prefs.speaker) > 0) {
- speakerRow.hidden = false;
- }
+ const cameraCount = fillOne(pickCamera, 'videoinput', prefs.camera || currentDeviceId('video'));
+ const micCount = fillOne(pickMic, 'audioinput', prefs.mic || currentDeviceId('audio'));
+ const hasSinkId = typeof HTMLMediaElement.prototype.setSinkId === 'function';
+ const speakerCount = hasSinkId ? fillOne(pickSpeaker, 'audiooutput', prefs.speaker) : 0;
+ // A field with nothing to choose from is not help, it's clutter —
+ // hidden rather than shown empty. Re-evaluated on every call, so a
+ // device unplugged or plugged in mid-call (devicechange) is
+ // reflected immediately.
+ cameraRow.hidden = cameraCount === 0;
+ micRow.hidden = micCount === 0;
+ speakerRow.hidden = !hasSinkId || speakerCount === 0;
}
function labelFor(kind, count) {
@@ -133,22 +232,7 @@
return track ? track.getSettings().deviceId : '';
}
- // --- mute and camera-off defaults: applied to the live tracks when ---
- // --- there is a stream, always reflected on the toggle buttons ---
- // --- even when there isn't one, so the choice still holds at join. ---
-
- function applyToggles() {
- if (stream) {
- for (const track of stream.getAudioTracks()) track.enabled = !prefs.muted;
- for (const track of stream.getVideoTracks()) track.enabled = !prefs.cameraOff;
- }
- muteButton.setAttribute('aria-pressed', String(!!prefs.muted));
- muteButton.textContent = prefs.muted ? 'Unmute' : 'Mute';
- cameraButton.setAttribute('aria-pressed', String(!!prefs.cameraOff));
- cameraButton.textContent = prefs.cameraOff ? 'Camera on' : 'Camera off';
- }
-
- // --- wiring ---
+ // --- wiring that needs media support ---
pickCamera.addEventListener('change', () => {
prefs.camera = pickCamera.value;
@@ -164,22 +248,21 @@
prefs.speaker = pickSpeaker.value;
savePrefs();
});
- muteButton.addEventListener('click', () => {
- prefs.muted = !prefs.muted;
- savePrefs();
- applyToggles();
- });
- cameraButton.addEventListener('click', () => {
- prefs.cameraOff = !prefs.cameraOff;
- savePrefs();
- applyToggles();
- });
- joinButton.addEventListener('click', () => {
- stopPreview();
- location.assign(root.dataset.joinHref);
+ navigator.mediaDevices.addEventListener?.('devicechange', () => {
+ fillPickers().catch(() => {});
});
- navigator.mediaDevices.addEventListener?.('devicechange', fillPickers);
- window.addEventListener('pagehide', stopPreview);
- startPreview();
+ // A tab loaded in the background is not somewhere a permission
+ // prompt should appear unannounced; the request waits for the tab
+ // to actually become visible, once, and proceeds normally from
+ // there.
+ if (document.visibilityState === 'visible') {
+ startPreview();
+ } else {
+ document.addEventListener('visibilitychange', function onVisible() {
+ if (document.visibilityState !== 'visible') return;
+ document.removeEventListener('visibilitychange', onVisible);
+ startPreview();
+ });
+ }
})();
diff --git a/internal/meet/templates/join.html b/internal/meet/templates/join.html
index 3405f1b..4afadba 100644
--- a/internal/meet/templates/join.html
+++ b/internal/meet/templates/join.html
@@ -11,15 +11,15 @@
{{if .OpenElsewhere}}<p rst-notice>This meeting is already open in another tab. <a href="/m/{{.Sqid}}/takeover">Use this tab instead</a>.</p>{{end}}
<noscript><p>The call needs JavaScript to reach your camera and the other participants; everything else on this page works without it.</p></noscript>
<div id="devices" data-sqid="{{.Sqid}}" data-join-href="{{if .OpenElsewhere}}/m/{{.Sqid}}/takeover{{else}}/m/{{.Sqid}}{{end}}" hidden>
- <video id="preview" muted autoplay playsinline class="meet-preview"></video>
- <div class="meet-pickers">
- <label>Camera <select id="pick-camera"></select></label>
- <label>Microphone <select id="pick-mic"></select></label>
- <label id="speaker-row" hidden>Speaker <select id="pick-speaker"></select></label>
+ <video id="preview" muted autoplay playsinline aria-label="Your camera preview" class="meet-preview"></video>
+ <div id="pickers" class="meet-pickers">
+ <div id="camera-row">{{template "field-select" dict "ID" "pick-camera" "Name" "camera" "Label" "Camera"}}</div>
+ <div id="mic-row">{{template "field-select" dict "ID" "pick-mic" "Name" "mic" "Label" "Microphone"}}</div>
+ <div id="speaker-row" hidden>{{template "field-select" dict "ID" "pick-speaker" "Name" "speaker" "Label" "Speaker"}}</div>
</div>
<div class="meet-controls">
- <button type="button" id="toggle-mute" rst-btn aria-pressed="false">Mute</button>
- <button type="button" id="toggle-camera" rst-btn aria-pressed="false">Camera off</button>
+ <button type="button" id="toggle-mute" rst-btn aria-pressed="false">Microphone</button>
+ <button type="button" id="toggle-camera" rst-btn aria-pressed="false">Camera</button>
<button type="button" id="join" rst-btn="primary lg" disabled>Join the call</button>
</div>
<p id="status" class="meet-status" role="status"></p>
diff --git a/internal/meettest/holding_test.go b/internal/meettest/holding_test.go
index dba20ad..32f1faf 100644
--- a/internal/meettest/holding_test.go
+++ b/internal/meettest/holding_test.go
@@ -113,6 +113,23 @@ func TestHoldingPageHasDevicesScriptAndNoLegacyPlaceholder(t *testing.T) {
}
}
+// Fix round 1, Important 5: the scriptless floor depends on #devices
+// starting hidden — devices.js's first job is un-hiding it — so a
+// regression that drops the attribute would show a bare, unstyled
+// device block to every browser with script disabled. Matched right
+// after data-join-href so this is unambiguously the #devices element's
+// own hidden attribute, not some other tag's.
+func TestHoldingPageDevicesBlockStartsHidden(t *testing.T) {
+ x, sqid, _ := memberWithMeeting(t)
+ c := x.browser(t)
+ c.signIn("paul@example.com")
+ rec := c.get("/m/" + sqid + "/join")
+ body := rec.Body.String()
+ if !strings.Contains(body, `data-join-href="/m/`+sqid+`" hidden>`) {
+ t.Fatalf("holding page's #devices block should render hidden: %s", body)
+ }
+}
+
func TestHoldingPageIs404ForEndedMeetings(t *testing.T) {
x, sqid, _ := memberWithMeeting(t)
m := x.browser(t)