fix(cockpit): a provider's methods keep their this, a hold answers true or false, and a cut text never splits a surrogate pair

Provider methods and worded members are called on the provider itself,
so a class-based provider works. A hold that answers false refuses entry
and is released; any answer but true or false refuses it too, released,
with one warning. An answer whose `then` getter throws is handled as a
throw, and a text cut at its cap never leaves half an emoji.

docs/APPLICATION.md

@@ -239,7 +239,8 @@ providers, checked once when the controls are built
 warning. Entry rides the most recently selected subject of all kinds and needs
 the provider's `layerId` on; the provider then holds its subject, hands the
 camera a frame and the generic HUD a view model on every update, and releases
-it on exit. Provider methods are synchronous; one that throws or returns a
+it on exit. Provider methods are synchronous and are called on the provider,
+so a class instance's methods keep their `this`; one that throws or returns a
 promise is treated as having failed.
 
 ```js
@@ -317,7 +318,11 @@ user's selection.
 A frame with `heightM: null` stands on the shared floor, or where it last
 stood while that floor is cold. If no height has been known since entry, the
 view waits, showing ACQUIRING SURFACE, for at most five seconds
-(`COCKPIT_GROUND_WAIT_TIMEOUT_MS`), then ends and releases the subject. A
+(`COCKPIT_GROUND_WAIT_TIMEOUT_MS`), then ends and releases the subject.
-`hold` that throws or returns a promise refuses entry and is followed by
+
-`release(id, { restore: false })`, again once a returned promise settles, so
+`hold(id)` returns `true` once it holds the subject, or `false` to refuse
-`release` must be harmless when called more than once.
+entry; any other answer, `undefined` included, refuses entry too, with one
+warning. A refused hold, and one that throws or returns a promise, is followed
+by `release(id, { restore: false })`, again once a returned promise settles,
+so `release` must be harmless for a subject it does not hold and when called
+more than once.

src/cockpitSubjectProviders.js

@@ -66,12 +66,17 @@ const clamp = (value, low, high) => Math.min(high, Math.max(low, value));
 
 /**
  * `value` as display text: trimmed, at most `max` characters (the last one
- * an ellipsis when cut); '' for anything that is not a string or a number.
+ * an ellipsis when cut, the cut never inside a surrogate pair, so an emoji is
+ * kept whole or left out); '' for anything that is not a string or a number.
  */
 export function cockpitSubjectText(value, max) {
   if (typeof value !== 'string' && !finite(value)) return '';
   const text = String(value).replace(/\s+/g, ' ').trim();
-  return text.length > max ? `${text.slice(0, max - 1)}…` : text;
+  if (text.length <= max) return text;
+  let end = max - 1;
+  const last = text.charCodeAt(end - 1);
+  if (last >= 0xd800 && last <= 0xdbff) end -= 1;
+  return `${text.slice(0, end)}…`;
 }
 
 const tone = (value) => (TONES.has(value) ? value : 'normal');
@@ -101,13 +106,13 @@ function labelsOf(raw) {
 
 /**
  * A provider's worded member as it reads now: the value, or what the
- * function it is returns (so it can follow the page's language); undefined
+ * function it is returns when called on `provider` (so it can follow the
- * when that throws.
+ * page's language); undefined when that throws.
  */
-function wordedNow(value) {
+function wordedNow(value, provider) {
   if (typeof value !== 'function') return value;
   try {
-    return value();
+    return value.call(provider);
   } catch {
     return undefined;
   }
@@ -132,10 +137,11 @@ function refusal(provider, kinds) {
     typeof provider.isEntryAllowed !== 'function'
   )
     return 'isEntryAllowed is not a function';
-  if (!faceOf(wordedNow(provider.entryFace))) return 'malformed entry face';
+  if (!faceOf(wordedNow(provider.entryFace, provider)))
-  if (!labelsOf(wordedNow(provider.labels)))
+    return 'malformed entry face';
+  if (!labelsOf(wordedNow(provider.labels, provider)))
     return 'labels need view, exit and exitText';
-  const look = wordedNow(provider.look);
+  const look = wordedNow(provider.look, provider);
   if (
     provider.look != null &&
     (!isObject(look) || !cockpitSubjectText(look.aheadLabel, 64))
@@ -159,21 +165,24 @@ function refusal(provider, kinds) {
  *   on a bridge, resting at `restPitchDeg` (default 0, -90..90), its Look
  *   ahead button named `aheadLabel`;
  *   `entryFace`, `labels` and `look` may each be a function returning the
- *   object: it is called again whenever they are shown and when the page's
+ *   object: it is called on the provider again whenever they are shown and
- *   language changes, so its words can follow it (`./i18n/locale`). A read
+ *   when the page's language changes, so its words can follow it
- *   that throws or is malformed keeps the last good one; `look` is on or off
+ *   (`./i18n/locale`). A read that throws or is malformed keeps the last
- *   and `restPitchDeg` fixed as first read;
+ *   good one; `look` is on or off and `restPitchDeg` fixed as first read;
  * - `cameraIntervalMs` (optional): 0..1000, null for the Cockpit's default; 0
  *   updates the camera every rendered frame;
  * - `headingSlewDps` (optional): how fast the view turns to a new heading,
  *   positive or Infinity, null for the Cockpit's default;
  * - `candidate()`, `isEntryAllowed(candidate)` (optional), `hold(id)`,
  *   `read(nowMs, ctx)`, `hud(frame, ctx)` and `release(id, {restore})`, all
- *   synchronous (createCockpitSubjectCaller). `isEntryAllowed` allows entry
+ *   synchronous (createCockpitSubjectCaller) and called on the provider, so
- *   only by returning `true`. A `hold` that throws or returns a promise
+ *   a class instance's methods keep their `this`. `isEntryAllowed` allows
- *   refuses entry and is followed by `release(id, {restore: false})`, and by
+ *   entry only by returning `true`. `hold` returns `true` once it holds the
- *   another once that promise settles; `release` must therefore be harmless
+ *   subject, or `false` to refuse entry; any other answer refuses it too,
- *   when called again for a subject it already gave back.
+ *   with one warning. A refused hold, and one that throws or returns a
+ *   promise, is followed by `release(id, {restore: false})`, and by another
+ *   once that promise settles; `release` must therefore be harmless for a
+ *   subject it does not hold or already gave back.
  *
  * A provider that is not so is left out with one warning naming it.
  * @param {unknown} list
@@ -199,9 +208,10 @@ export function cockpitSubjectProviders(
     kinds.add(provider.kind);
     // Worded members are read again on each use (see above); the last good
     // read stands in for a bad one.
-    let face = faceOf(wordedNow(provider.entryFace));
+    const worded = (value) => wordedNow(value, provider);
-    let labels = labelsOf(wordedNow(provider.labels));
+    let face = faceOf(worded(provider.entryFace));
-    const firstLook = wordedNow(provider.look);
+    let labels = labelsOf(worded(provider.labels));
+    const firstLook = worded(provider.look);
     const lookOf = (raw) =>
       Object.freeze({
         restPitchDeg: finite(firstLook.restPitchDeg)
@@ -217,16 +227,16 @@ export function cockpitSubjectProviders(
         priority: finite(provider.priority) ? provider.priority : 0,
         order,
         get entryFace() {
-          face = faceOf(wordedNow(provider.entryFace)) ?? face;
+          face = faceOf(worded(provider.entryFace)) ?? face;
           return face;
         },
         get labels() {
-          labels = labelsOf(wordedNow(provider.labels)) ?? labels;
+          labels = labelsOf(worded(provider.labels)) ?? labels;
           return labels;
         },
         get look() {
           if (!look) return null;
-          const raw = wordedNow(provider.look);
+          const raw = worded(provider.look);
           if (isObject(raw) && cockpitSubjectText(raw.aheadLabel, 64))
             look = lookOf(raw);
           return look;
@@ -242,12 +252,16 @@ export function cockpitSubjectProviders(
           (finite(provider.headingSlewDps) && provider.headingSlewDps > 0)
             ? provider.headingSlewDps
             : null,
-        candidate: provider.candidate,
+        // Bound, so a method that uses `this` (a class instance's, say)
-        isEntryAllowed: provider.isEntryAllowed ?? (() => true),
+        // still reads its own provider when called on this copy.
-        hold: provider.hold,
+        candidate: provider.candidate.bind(provider),
-        read: provider.read,
+        isEntryAllowed: provider.isEntryAllowed
-        hud: provider.hud,
+          ? provider.isEntryAllowed.bind(provider)
-        release: provider.release,
+          : () => true,
+        hold: provider.hold.bind(provider),
+        read: provider.read.bind(provider),
+        hud: provider.hud.bind(provider),
+        release: provider.release.bind(provider),
       }),
     );
   });
@@ -506,6 +520,10 @@ export function cockpitSubjectHudView(raw) {
  * would be (its rejection, if any, is swallowed), with one warning, and
  * `onSettled`, when given, runs once the promise has settled, so a caller
  * can undo work that lands late.
+ *
+ * `call.warnOnce(provider, method, message, error)` warns on the same
+ * once-per-provider-and-method footing, for an answer the caller finds
+ * malformed.
  * @param {{warn?: (message: string, error: unknown) => void}} [options]
  */
 export function createCockpitSubjectCaller({
@@ -521,7 +539,7 @@ export function createCockpitSubjectCaller({
       error,
     );
   };
-  return function call(
+  function call(
     provider,
     method,
     args = [],
@@ -529,17 +547,20 @@ export function createCockpitSubjectCaller({
     onSettled = null,
   ) {
     let result;
+    let thenable;
     try {
       result = provider[method](...args);
+      // Reading `then` runs provider code too (a getter may throw), so it is
+      // guarded as the call is.
+      thenable =
+        result !== null &&
+        (typeof result === 'object' || typeof result === 'function') &&
+        typeof result.then === 'function';
     } catch (error) {
       warnOnce(provider, method, 'failed', error);
       return fallback;
     }
-    if (
+    if (thenable) {
-      result !== null &&
-      (typeof result === 'object' || typeof result === 'function') &&
-      typeof result.then === 'function'
-    ) {
       // Its work may still land: `onSettled` runs once it has, either way.
       const settled = () => {
         if (typeof onSettled === 'function') onSettled();
@@ -554,5 +575,7 @@ export function createCockpitSubjectCaller({
       return fallback;
     }
     return result;
-  };
+  }
+  call.warnOnce = warnOnce;
+  return call;
 }

src/cockpitSubjectProviders.test.mjs

@@ -389,6 +389,21 @@ test('texts are trimmed, collapsed and cut with an ellipsis', () => {
   assert.equal(cockpitSubjectText(NaN, 10), '');
 });
 
+test('a cut never splits a surrogate pair', () => {
+  // The cut would fall between the emoji's halves: it is left out whole.
+  assert.equal(cockpitSubjectText('abcd😀efgh', 6), 'abcd…');
+  // One that fits before the cut is kept whole.
+  assert.equal(cockpitSubjectText('abc😀efgh', 6), 'abc😀…');
+  const lone =
+    /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]/;
+  for (let max = 2; max <= 24; max += 1)
+    assert.equal(
+      lone.test(cockpitSubjectText('🚆 Example 12 🚉 Central', max)),
+      false,
+      `cut at ${max}`,
+    );
+});
+
 test('a provider method that throws returns the fallback and warns once', () => {
   const warnings = [];
   const call = createCockpitSubjectCaller({
@@ -427,6 +442,114 @@ test('a provider method that returns a promise is refused as a throw is, warning
   await new Promise((resolve) => setTimeout(resolve, 0));
 });
 
+test('an answer whose `then` throws when read is handled as a throw', () => {
+  const warnings = [];
+  const call = createCockpitSubjectCaller({
+    warn: (message) => warnings.push(message),
+  });
+  const tricky = {
+    kind: 'example-kind',
+    hold: () => ({
+      get then() {
+        throw new Error('no then');
+      },
+    }),
+  };
+  assert.equal(call(tricky, 'hold', ['a1'], 'refused'), 'refused');
+  assert.equal(warnings.length, 1);
+  assert.match(warnings[0], /example-kind hold\(\) failed/);
+});
+
+test('the caller’s warnOnce shares its once-per-provider-and-method record', () => {
+  const warnings = [];
+  const call = createCockpitSubjectCaller({
+    warn: (message) => warnings.push(message),
+  });
+  const ride = {
+    kind: 'example-kind',
+    hold() {
+      throw new Error('boom');
+    },
+  };
+  call.warnOnce(ride, 'read', 'is odd');
+  call.warnOnce(ride, 'read', 'is odd again');
+  call(ride, 'hold', ['a1']);
+  call.warnOnce(ride, 'hold', 'is odd');
+  assert.deepEqual(warnings, [
+    '[Cockpit] subject provider example-kind read() is odd',
+    '[Cockpit] subject provider example-kind hold() failed',
+  ]);
+});
+
+test('a class-based provider keeps its `this`, in its methods and its worded members', () => {
+  class Ride {
+    #word = 'RIDE';
+    #held = null;
+    kind = 'example-kind';
+    layerId = 'example-layer';
+    speak(word) {
+      this.#word = word;
+    }
+    entryFace() {
+      return { icon: 'commute', label: this.#word };
+    }
+    get labels() {
+      return {
+        view: `${this.#word} view`,
+        exit: 'Exit ride view',
+        exitText: 'EXIT RIDE',
+      };
+    }
+    look() {
+      return { aheadLabel: `Look ahead, ${this.#word}` };
+    }
+    candidate() {
+      return { id: 'a1', name: this.#word };
+    }
+    isEntryAllowed(candidate) {
+      return candidate.id === 'a1' && this.#held === null;
+    }
+    hold(id) {
+      this.#held = id;
+      return true;
+    }
+    read() {
+      return this.#held;
+    }
+    hud(frame) {
+      return { title: `${this.#word} ${frame}` };
+    }
+    release() {
+      this.#held = null;
+      return 'released';
+    }
+  }
+  const instance = new Ride();
+  const { kept, warnings } = keep([instance]);
+  assert.deepEqual(warnings, []);
+  const [ride] = kept;
+  assert.equal(ride.entryFace.label, 'RIDE');
+  assert.equal(ride.labels.view, 'RIDE view');
+  assert.equal(ride.look.aheadLabel, 'Look ahead, RIDE');
+  instance.speak('FAHRT');
+  assert.equal(ride.entryFace.label, 'FAHRT', 'read again, on the instance');
+  assert.equal(ride.look.aheadLabel, 'Look ahead, FAHRT');
+  // Called on the kept copy, as the Cockpit calls them.
+  const failures = [];
+  const call = createCockpitSubjectCaller({
+    warn: (message) => failures.push(message),
+  });
+  assert.deepEqual(call(ride, 'candidate'), { id: 'a1', name: 'FAHRT' });
+  assert.equal(ride.isEntryAllowed({ id: 'a1' }), true);
+  assert.equal(call(ride, 'hold', ['a1']), true);
+  assert.equal(ride.isEntryAllowed({ id: 'a1' }), false);
+  assert.equal(call(ride, 'read', [0, {}]), 'a1');
+  assert.deepEqual(call(ride, 'hud', ['a1', {}]), { title: 'FAHRT a1' });
+  assert.equal(call(ride, 'release', ['a1', { restore: true }]), 'released');
+  assert.equal(call(ride, 'read', [0, {}]), null);
+  assert.deepEqual(failures, []);
+});
+
 test('labels, entry face and look given as functions are read again on each use', () => {
   let lang = 'en';
   const words = {

src/ui/cockpitSubjectView.test.mjs

@@ -485,15 +485,16 @@ test('leaving by the exit control asks the provider to restore its subject', asy
   assert.deepEqual(lastRelease(ride.calls), ['release', 'a1', true]);
 });
 
-test('entry fails, releasing nothing it did not hold, when hold or the first frame fails', async (t) => {
+test('entry fails when the gate is shut, the hold fails or no first frame comes, releasing only what it held', async (t) => {
-  const refused = subjectProvider();
+  const shut = subjectProvider();
-  refused.subject.hold = false;
+  shut.subject.allowed = false;
-  let app = fixture(t, { providers: [refused] });
+  let app = fixture(t, { providers: [shut] });
   app.cockpit.syncEntry();
   assert.equal(app.cockpit.enter(), false);
   assert.equal(
-    refused.calls.some(([name]) => name === 'release'),
+    shut.calls.some(([name]) => name === 'hold' || name === 'release'),
     false,
+    'a shut gate neither holds nor releases',
   );
   const blank = subjectProvider();
   blank.subject.frame = null;
@@ -525,6 +526,108 @@ test('entry fails, releasing nothing it did not hold, when hold or the first fra
   assert.match(app.warned[0], /returned a promise/);
 });
 
+test('a hold that answers false refuses entry; any answer but true or false warns once; each is released', async (t) => {
+  for (const [answer, warns] of [
+    [false, false],
+    [undefined, true],
+    [1, true],
+    ['held', true],
+  ]) {
+    const ride = subjectProvider();
+    ride.subject.hold = answer;
+    const { cockpit, warned } = fixture(t, { providers: [ride] });
+    const holdWarnings = () =>
+      warned.filter((message) =>
+        /example-kind hold\(\) must return true or false/.test(message),
+      ).length;
+    cockpit.syncEntry();
+    assert.equal(cockpit.enter(), false, `hold answered ${String(answer)}`);
+    assert.equal(cockpit.active, false);
+    assert.deepEqual(lastRelease(ride.calls), ['release', 'a1', false]);
+    assert.equal(holdWarnings(), warns ? 1 : 0);
+    assert.equal(warned.length, warns ? 1 : 0, 'no other warning');
+    // Once per provider and method.
+    assert.equal(cockpit.enter(), false);
+    assert.equal(holdWarnings(), warns ? 1 : 0);
+    assert.equal(
+      ride.calls.filter(([name]) => name === 'release').length,
+      2,
+      'released each time',
+    );
+  }
+});
+
+test('a class-based provider keeps its `this` from entry to release', async (t) => {
+  manualClock(t, 10_000);
+  const { frame } = subjectProvider().subject;
+  class Ride {
+    #word = 'RIDE';
+    #held = null;
+    calls = [];
+    kind = 'example-kind';
+    layerId = 'example-layer';
+    entryFace() {
+      return { icon: 'commute', label: this.#word };
+    }
+    get labels() {
+      return {
+        view: `${this.#word} view`,
+        exit: 'Exit ride view',
+        exitText: `EXIT ${this.#word}`,
+      };
+    }
+    look() {
+      return { aheadLabel: `Look ahead, ${this.#word}` };
+    }
+    candidate() {
+      return { id: 'a1', name: `Example ${this.#word}` };
+    }
+    isEntryAllowed(candidate) {
+      return this.#held === null && candidate.id === 'a1';
+    }
+    hold(id) {
+      this.#held = id;
+      this.calls.push(['hold', id]);
+      return true;
+    }
+    read() {
+      this.calls.push(['read', this.#held]);
+      return this.#held ? { ...frame, id: this.#held } : null;
+    }
+    hud(view) {
+      this.calls.push(['hud', view.id]);
+      return { ...HUD, title: `${this.#word} ${view.id}` };
+    }
+    release(id, { restore }) {
+      this.calls.push(['release', id, restore]);
+      this.#held = null;
+    }
+  }
+  const ride = new Ride();
+  const { $, cockpit, warned } = fixture(t, {
+    providers: [{ definition: ride }],
+  });
+  cockpit.syncEntry();
+  await nextTick();
+  assert.match($('#cockpit-entry').textContent, /RIDE/);
+  $('#cockpit-entry').click();
+  await settle();
+  assert.equal(cockpit.active, true);
+  assert.equal($('#cockpit-callsign').textContent, 'RIDE a1');
+  assert.equal($('#cockpit-hud').getAttribute('aria-label'), 'RIDE view');
+  assert.match($('#map-view-switch').textContent, /EXIT RIDE/);
+  assert.equal(
+    $('#cockpit-look-ahead').getAttribute('aria-label'),
+    'Look ahead, RIDE',
+  );
+  assert.ok(ride.calls.some(([name, id]) => name === 'read' && id === 'a1'));
+  assert.ok(ride.calls.some(([name, id]) => name === 'hud' && id === 'a1'));
+  $('#map-view-switch').click();
+  assert.equal(cockpit.active, false);
+  assert.deepEqual(lastRelease(ride.calls), ['release', 'a1', true]);
+  assert.deepEqual(warned, []);
+});
+
 test('a frame with no height and a cold floor never stands the eye on the ellipsoid', async (t) => {
   const clock = manualClock(t, 10_000);
   const ride = subjectProvider();

src/ui/cockpitTrackingController.js

@@ -388,9 +388,9 @@ export function _adoptTrackedEntity(nowMs, suppliedInfo = null) {
 
 /**
  * Enter a provider's view on its candidate: hold it, read a first frame, then
- * take the camera as the built-in views do. Entry fails, releasing what was
+ * take the camera as the built-in views do. Entry fails when the gate is shut,
- * held, when the gate is shut, the provider refuses the hold or no frame
+ * the hold answers anything but `true` or no frame comes; past the gate, a
- * comes.
+ * failed entry releases the subject, since the hold may have taken it.
  * @param {object} provider From cockpitSubjectProviders.
  * @returns {boolean}
  */
@@ -419,12 +419,20 @@ function enterSubject(provider) {
     failed,
     releaseLate,
   );
-  if (held === failed) {
+  if (held !== true) {
-    // A hold that threw (or broke the contract) may have half happened.
+    // `false` refuses entry; a hold that threw, returned a promise or
+    // answered anything else (warned of once) may still have taken the
+    // subject. Each is released.
+    if (held !== false && held !== failed)
+      this.callSubject.warnOnce(
+        provider,
+        'hold',
+        'must return true or false; entry refused',
+        held,
+      );
     release();
     return false;
   }
-  if (held !== true) return false;
   const frame = cockpitSubjectFrame(
     this.callSubject(
       provider,