-
Notifications
You must be signed in to change notification settings - Fork 0
Review 5394
Astro-Han (contributor, fork) · MERGED · view on GitHub
Verdict: approve · merged 2026-08-24 18:29 UTC
Head reviewed: d47945d53e40453feb31a974d56fd7b56fd516c7. No prior reviews.
On an Apple machine whose browser reports userAgentData.platform: "Unknown" —
the UA-CH spec's own value for "cannot say" — every Kbd drew Ctrl and every
mod hotkey listened for Ctrl. The user reads "Ctrl K", presses ⌘K, nothing
happens. #5325 closed the ''
spelling of the same hole; Unknown still answered "not Apple".
A client-hints platform naming nothing — '', whitespace, or Unknown
(case-insensitive) — falls through to navigator.platform. The two byte-identical
copies of the detection become one internal util both import.
Apple users of any app built on Astryx inside an embedder that reports Unknown
client hints: the hint they read now matches the key that works. Everyone else:
nothing. Six of seven client-hints branches measured identical before and after.
No API change. isApplePlatform is deliberately absent from utils/index.ts;
both removed functions were module-private.
No new theme targets.
- API — no.
-
Visual — one path, and it is the fix:
Ctrl→⌘on an Unknown-hints Apple device. Narrower, so nothing reflows. - Theme — no.
Same single call per render/keydown, no new effect, no listeners, no layout reads. Bundle marginally smaller (one copy deleted). O(1), no perf test needed.
Real Chromium, bare story iframe, frames opened with the read tool:
shots-5394/BEFORE__-unknown-mac.png → Ctrl K;
shots-5394/AFTER__-unknown-mac.png → ⌘ K. The other six branches
byte-identical either side.
Low-risk — no new surface, no regression (6/7 branches identical), no perf change.
approve — no blocking findings. Checks recorded so silence is not read as "did not
look": one detection covers everything the two did, measured across all seven
branches in a real engine (R1d); no other call site of the same shape exists on
main (R3d); dropping the unknown guard turns 2 of 8 tests red, dropping the whole
sentinel check turns 4 red; fix(core) + patch changeset is honest (R1f).
Thanks — good catch on
Unknown, and one detection beats two. Verified in Chrome: an Unknown-platform Mac now draws ⌘ and cmd+K fires. Merging as is.
Posted as drafted.