Skip to content

Commit 8625c01

Browse files
kryptocodeskryptocodes
committed
fix: harden the embedded keyboard and make embed misconfiguration visible
1 parent 2ac068e commit 8625c01

35 files changed

Lines changed: 3879 additions & 82 deletions

‎images/minimal-vnc-desktop/README.md‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,160 @@ python3 -m http.server 8080 # from images/minimal-vnc-desktop
8080
# http://localhost:8080/host/test-host.html (+ debug panel, buttons, ?nest=1)
8181
```
8282

83+
### The embed layout contract (mobile sharpness)
84+
85+
The stream is a bitmap, so whatever raster scale the embedding page's compositor
86+
picks for the iframe's layer is the scale the user sees. Mobile compositors pick
87+
one BELOW device scale for a layer they consider cheap to redraw, and every number
88+
the viewer can report stays identical while the render goes visibly soft — same
89+
framebuffer, same canvas CSS size, zoom 1.00, byte-identical to a sharp top-level
90+
tab. So sharpness on a phone is a property of the embedder, not of the encoder,
91+
and lowering `quality`/`compression` cannot buy it back.
92+
93+
The rules, at **every** hop of a chain (customer page → portal → liveview):
94+
95+
- the viewer iframe is a plain fixed full-viewport layer — `position: fixed;
96+
inset: 0; width: 100%; height: 100%; border: 0` — and a **direct child of
97+
`<body>`**, with no wrapper;
98+
- page chrome is **layered over** it (`position: fixed` + `z-index`), never a
99+
layout sibling that resizes it;
100+
- no ancestor carries a transform, `zoom`, filter, containment,
101+
`content-visibility`, `opacity < 1`, `will-change`, or scrolls or animates;
102+
- the iframe carries `allow="clipboard-read; clipboard-write; virtual-keyboard"`.
103+
Without the `virtual-keyboard` token the VirtualKeyboard API exists inside the
104+
frame but stays mute, which makes YOUR geometry the only thing keeping the
105+
keyboard usable — the viewer reports `no-virtual-keyboard` when it finds itself
106+
in that configuration;
107+
- **do not take the focus while the keyboard is up.** Anything in the embedding
108+
page that calls `focus()` — an input of your own, a scroll-into-view, a consent
109+
banner mounting, an analytics widget — closes the user's keyboard mid-word.
110+
Each document owns its own `activeElement`, so the viewer cannot see this
111+
happening and cannot re-open a soft keyboard without a user gesture; all it can
112+
do is notice via `document.hasFocus()` and report `focus-stolen`. Track
113+
`.on('kbdstate')` and leave the focus alone while `active` is true.
114+
115+
`host/popcorn-host.js` both applies and checks this:
116+
117+
```js
118+
const frame = document.createElement('iframe');
119+
PopcornHost.layer(frame); // the contract, before src
120+
frame.src = viewerBase + '/liveview.html?' +
121+
['magnify=1', 'parentOrigin=' + encodeURIComponent(location.origin)]
122+
.concat(PopcornHost.forwardParams()).join('&'); // params survive this hop
123+
document.body.appendChild(frame);
124+
const host = PopcornHost.attach(frame, { childOrigin: new URL(viewerBase).origin });
125+
host.on('layout', (a) => { if (!a.ok) report(a.issues); }); // codes only
126+
host.on('scale', (s) => report(s.deviceScale)); // remote px per device px
127+
host.on('health', (h) => report(h.code, h.detail)); // this embed is breaking the keyboard
128+
```
129+
130+
`auditLayout()` runs itself on hello and on first paint, warns to the console with
131+
the offending codes, emits `.on('layout')`, and posts the finding down to the
132+
viewer's structural session log — so a "the stream is blurry" report is
133+
attributable from the pod side without asking anyone to open devtools on a phone.
134+
`.on('scale')` carries the four numbers that separate the causes: `fbWidth/fbHeight`
135+
(remote px sent), `cssWidth/cssHeight` (the box on the device), `scale` (fb/CSS)
136+
and `deviceScale` (fb per DEVICE px — the one that predicts what the user sees).
137+
138+
Reproduce the failure on purpose with `host/test-host.html?badlayout=1`, which
139+
leaves the iframe inside a flex + `overflow: auto` + transformed wrapper. Combine
140+
with `&nest=1` for the full three-level chain.
141+
142+
### Geometry: the failure that looks like a broken keyboard
143+
144+
An embedder that posts geometry it cannot measure is worse than one that posts
145+
nothing. A middle frame whose `PopcornHost` falls back to measuring *itself* is a
146+
cross-origin iframe whose `visualViewport` never shrinks, so it reports
147+
`occludedBottom: 0` forever — and host geometry deliberately **suppresses** the
148+
viewer's own detectors (two detectors driving the lift with different heights is what
149+
causes keyboard-open jitter). The result is no lift, no pan budget to reach the
150+
field, and the local-echo pill positioned behind the keyboard, so the one mechanism
151+
that masks per-keystroke round-trip latency becomes invisible and typing appears
152+
dead until the remote's pixels arrive.
153+
154+
Three defences, so a misconfigured embedder degrades to "no help" rather than
155+
"actively broken":
156+
157+
- an embedded fallback measurer that sees no occlusion stays **silent**;
158+
- the viewer only lets a host silence its detectors once that host has reported a
159+
real occlusion at least once;
160+
- the legacy `{type:'parent-viewport', innerHeight, viewportHeight}` message — what
161+
the deployed portal sends — is **translated** into `POPCORN_HOST_GEOMETRY`, so that
162+
portal works unmodified. Opt out with `attach(frame, { legacyGeometry: false })`.
163+
164+
`host/test-host.html?legacybridge=1` exercises the translation; add `&legacyxlate=0`
165+
to reproduce the original break.
166+
167+
### `.on('health')` — the viewer's verdict on your integration
168+
169+
Every failure in this chain that has cost real time degraded *silently*: the viewer
170+
knew something was wrong and the only place it could say so was a console inside a
171+
cross-origin iframe on somebody's phone. The integrator saw a working page, the
172+
user saw a broken keyboard, and nobody had both halves at once.
173+
174+
So the viewer reports its own health up the bridge, in codes. Alert on them, or log
175+
them beside your own session id — they are structural (short strings plus rounded
176+
numbers, never anything derived from page content), so they are safe to forward
177+
into your own logging.
178+
179+
| code | what it means |
180+
| --- | --- |
181+
| `host-geometry-blind` | you are feeding geometry but have never seen an occlusion, while the viewer's own detectors say the keyboard is up — you are measuring the wrong window |
182+
| `host-geometry-stale` | your feed stopped while the keyboard was up; the viewer has fallen back to local detection |
183+
| `host-geometry-disagrees` | both sides see a keyboard, with materially different occlusion — usually an iframe that is not full-viewport, so the lift is wrong by the difference |
184+
| `focus-stolen` | something in your page took the focus while the keyboard was open |
185+
| `no-virtual-keyboard` | embedded without `allow="virtual-keyboard"`, so your geometry is load-bearing |
186+
| `remote-unconfirmed` | keystrokes were sent that the remote field never reported holding — a real lost-input signal, as opposed to a slow repaint |
187+
188+
Each code is reported at most once per 30s, and every message carries the
189+
cumulative `codes` list, so a listener that mounts late still learns what went
190+
wrong. `host/test-host.html` logs them in its debug panel.
191+
192+
### Sharpness on a phone: the supersampled framebuffer
193+
194+
Even with the layout contract satisfied, the framebuffer is sized in the phone's CSS
195+
pixels (`deviceScaleFactor: 1`), so a 411px viewport streams 411 remote pixels onto
196+
~1080 device pixels — `dev=0.38` in the scale line, i.e. every remote pixel is
197+
upscaled ~2.6x by the phone. No encoder setting can put that detail back.
198+
199+
`?fbscale=` raises CDP `deviceScaleFactor` **and** grows the framebuffer with it, so
200+
the page still lays out as a 411px mobile viewport (same reflow, no reload — and
201+
`injected.js` pins `devicePixelRatio` to 1, so the site sees no change) while the
202+
raster carries k times the detail per axis: `dev` 0.38 → 0.76 at k=2.
203+
204+
| value | behaviour |
205+
| --- | --- |
206+
| `auto` | Opt-in adaptive mode: 2x once the link is measured healthy — magnify + touch + DPR≥2 + not in desktop-fit + RTT<400ms sustained 3s + no saveData/2g/3g. **Cold start is always 1x**, and it drops back to 1x if the link degrades. |
207+
| `1` (default) | Off — today's behaviour byte-for-byte. Use this default until device A/B data proves supersampling improves input-to-paint latency as well as sharpness. |
208+
| `2`, `3` | pinned, ignoring link health. For a device A/B. |
209+
210+
**It costs k² pixels per frame** — ~4x encode CPU on the pod and ~4x bytes on the
211+
wire at k=2. That trades against paint latency, which is why `auto` never spends it
212+
on a link it has not measured. The CPU side is the one to watch: this image runs
213+
TigerVNC 1.12, whose Tight/JPEG encoding is single-threaded per client (no equivalent
214+
of KasmVNC's `-RecThreads`), so 4x the pixels is 4x the work on ONE core. KasmVNC
215+
ships the same idea on by default — its Medium/High presets auto-scale the remote
216+
resolution to the client and explicitly scale upward on mobile — but its encoder fans
217+
out across cores. The mechanism is proven; the cost profile is not the same. Pages that hit desktop-fit are excluded because they
218+
are already supersampled (980 remote px into a 411px viewport ≈ `dev=0.91`, which is
219+
why desktop-fallback pages look sharp while responsive ones look soft).
220+
221+
### Diagnostics
222+
223+
All opt-in, all structural — no typed text, no field values, no coordinates, no
224+
URLs. Append to the viewer URL (they survive every embedding hop; see
225+
`PopcornHost.VIEWER_PARAMS`):
226+
227+
| param | what it adds |
228+
| --- | --- |
229+
| `diag=1` | ship the structural keyboard/input log to the proxy's `/klog` |
230+
| `kbddebug=1` | the same, plus an on-screen overlay and console mirror |
231+
| `fbscale=1` | disable the supersampled framebuffer (see above) — first thing to try if sharpness improved but latency got worse |
232+
| `e2e=1` | input→paint traces: `e2e tap g#7 sent=+3ms written=+58ms paint=+412ms total=473ms`. Needs `diag=1`. Reads a 5×5 grid of 12×12 pixel patches to detect localized paints, reduced to a checksum and discarded, so it costs a GPU readback per poll — bounded to 8 traces per load, one in flight, 2.5s each. `e2e=N` for N traces. |
233+
234+
`scale` lines (`fb=… css=… dpr=… sc=… dev=…`) and `host layout …` lines land in
235+
the same log, which is where a blur report should be read from first.
236+
83237
noVNC HTTP/WebSocket is served on `6080`. Restricted CDP is served on `9222`
84238
and full CDP is served on `9226` for trusted internal routing. Raw VNC listens
85239
on `127.0.0.1:5900` inside the container, and Chromium's raw DevTools endpoint

‎images/minimal-vnc-desktop/extensions/proxy/background.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,13 +236,19 @@ function mergeFrames() {
236236
let vw = 0, vh = 0, sw = 0, sb = -1, sc = null, pid = null, origin = null, novp = false, ol = 0, olw = 0, xf = null;
237237
const rects = [];
238238
let rtrunc = false; // merged list hit MERGED_MAX_RECTS -> the viewer must not read a miss as off-field
239+
// Some frame reported that it HAS editable fields but cannot place them yet
240+
// (content.js emitBlind: a cross-origin frame still waiting to be positioned).
241+
// Same meaning as rtrunc for the viewer — our rect coverage is known-incomplete,
242+
// so a tap matching nothing proves nothing.
243+
let blind = false;
239244
for (const [key, entry] of kbdFrames) {
240245
if (now - entry.ts > FRAME_STALE_MS) { kbdFrames.delete(key); continue; }
241246
if (entry.tabId !== kbdActiveTab) continue; // background tabs are kept fresh, never published
242247
const frameId = Number(key.slice(key.indexOf(':') + 1));
243248
const s = entry.state;
244249
// content.js caps rects PER FRAME, so a page full of same-origin iframes multiplies that cap. Bound the
245250
// merged list too, or the focus message outgrows the hub's frame limit and the whole state is dropped.
251+
if (s.blind) blind = true;
246252
if (Array.isArray(s.rects)) {
247253
for (const r of s.rects) {
248254
if (rects.length >= MERGED_MAX_RECTS) { rtrunc = true; break; }
@@ -297,6 +303,7 @@ function mergeFrames() {
297303
}
298304
const merged = { editable, rects, vw, vh };
299305
if (rtrunc) merged.rtrunc = true; // whitelist field, like every other one below
306+
if (blind) merged.blind = true; // ditto — an unwhitelisted field is dropped here
300307
if (sw > 0) merged.sw = sw;
301308
if (sb >= 0) merged.sb = sb; // 0 must survive the merge — see the whitelist note above
302309
if (sc) merged.sc = sc;

‎images/minimal-vnc-desktop/extensions/proxy/content.js‎

Lines changed: 74 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,16 @@
724724
// a frame whose parent could move it anyway; a forged FIELD is no longer possible.
725725
let absOffset = null; // our viewport origin in TOP coords, published by our parent
726726
let absAsks = 0;
727-
const ABS_MAX_ASKS = 8;
727+
const ABS_MAX_ASKS = 10;
728+
// Retry ladder for "parent, where am I?". The old shape was 300/300/300 then a
729+
// flat 2000, which put a ~2s cliff right where a three-level embed lands: the
730+
// portal's form frame asks before its own parent has been positioned, that ask
731+
// is answered with silence, and the next attempt is 2s later. Until it lands
732+
// the frame cannot report ANY coordinate, so the first tap on the first field
733+
// waits that whole time for the remote's editable:true instead of raising the
734+
// keyboard from a local rect hit. Ramp instead of stepping: the same total
735+
// coverage (~7s) with no single gap wide enough to be felt.
736+
const ABS_ASK_DELAYS = [120, 200, 320, 500, 800, 1200, 2000];
728737
const publishedAbs = new WeakMap(); // iframe element -> last published "x,y"
729738

730739
// Our offset to the top document, or null when we are inside a cross-origin
@@ -744,8 +753,10 @@
744753
if (frameOffset().reachedTop) return; // same-origin chain positions itself
745754
absAsks++;
746755
try { window.parent.postMessage({ __pcnKbdAbsReq: 1 }, '*'); } catch (_) {}
747-
// Retry: at document_start the parent's content script may not be listening yet.
748-
setTimeout(askForOffset, absAsks < 4 ? 300 : 2000);
756+
emitBlind(); // keep the "my fields are invisible to you" notice fresh while we wait
757+
// Retry: at document_start the parent's content script may not be listening
758+
// yet, and a parent that is not positioned ITSELF answers nothing at all.
759+
setTimeout(askForOffset, ABS_ASK_DELAYS[Math.min(absAsks - 1, ABS_ASK_DELAYS.length - 1)]);
749760
}
750761

751762
// Tell every child frame where it sits. Positions change on scroll/resize/layout,
@@ -770,14 +781,55 @@
770781
}
771782
}
772783

784+
// Coordinate-free "my fields are invisible to you" notice, sent once while this
785+
// frame is still waiting to be positioned.
786+
//
787+
// Silence was the old behaviour, and it hides the one fact the viewer needs: some
788+
// frame on this page HAS editable fields whose rects are missing from the merged
789+
// list. Without it a tap over a cross-origin form is indistinguishable from a tap
790+
// on a page of buttons — both look like 'unknown' coverage — so the viewer
791+
// refuses to raise the keyboard optimistically and the first tap waits a full
792+
// tunnel round-trip for the remote's editable:true (measured ~2s on mobile).
793+
//
794+
// Carries NO geometry and NO content: no rect, no rects, no hints, no value. Just
795+
// the blind flag, which is exactly the amount of information needed to say "do
796+
// not read a miss here as off-field". Cleared implicitly — the next real emit()
797+
// replaces this frame's slot in the background's per-frame map.
798+
//
799+
// RE-ASSERTED on a slow heartbeat rather than sent once: the background expires a
800+
// frame that has gone quiet (FRAME_STALE_MS, 6s), and a frame that is still
801+
// unpositioned past that point is exactly the one whose fields are still missing,
802+
// so letting the notice age out would silently restore the old behaviour at the
803+
// worst moment.
804+
// The heartbeat rides the ask ladder (askForOffset), which is already running for
805+
// exactly as long as this frame is unpositioned — no extra timer, and no layout
806+
// read per beat, because whether we hold fields is remembered rather than
807+
// recomputed. Once the ladder gives up we stop asserting too: a frame that can
808+
// never be positioned would otherwise leave every unknown tap on the page
809+
// raising the keyboard forever.
810+
const BLIND_REASSERT_MS = 2000;
811+
let blindSentAt = 0;
812+
let sawFields = false;
813+
function noteFields(state) {
814+
if (state.editable === true || (Array.isArray(state.rects) && state.rects.length > 0)) sawFields = true;
815+
}
816+
function emitBlind() {
817+
if (!sawFields) return;
818+
if (blindSentAt && Date.now() - blindSentAt < BLIND_REASSERT_MS) return;
819+
blindSentAt = Date.now();
820+
try { chrome.runtime.sendMessage({ type: 'PCN_KBD', state: { editable: false, rects: [], blind: 1 } }); } catch (_) {}
821+
}
822+
773823
// Report our own state, in top coords, directly to the background.
774824
function emit(state) {
775825
const off = ownOffset();
776826
if (!off) {
777827
// Not positioned yet. Reporting frame-local coords would put every rect in the
778-
// wrong place, so stay silent and keep asking — the same outcome the previous
779-
// relay had when there was nobody above us to relay through.
828+
// wrong place, so keep asking — but say that our coverage is missing rather
829+
// than going completely dark (see emitBlind).
830+
noteFields(state);
780831
askForOffset();
832+
emitBlind();
781833
return;
782834
}
783835
offsetState(state, off.x, off.y);
@@ -948,14 +1000,23 @@
9481000
// layout change (an iframe that loads into a static page would wait forever).
9491001
if (!IS_TOP) askForOffset();
9501002

951-
if (IS_TOP) {
952-
const reportInitialLayout = () => report(deepActiveElement(), true);
953-
if (document.readyState === 'loading') {
954-
document.addEventListener('DOMContentLoaded', reportInitialLayout, { once: true });
955-
} else {
956-
reportInitialLayout();
957-
}
958-
window.addEventListener('load', reportInitialLayout, { once: true });
1003+
// Runs in EVERY frame, not just the top one. A cross-origin form frame (the
1004+
// portal's hosted signup form) often renders its fields once, before our
1005+
// MutationObserver is watching or with no further mutation at all, and never
1006+
// focuses anything by itself — so its rects used to reach the viewer only when
1007+
// the user focused a field, which is the very tap that needed them. The tap then
1008+
// hit-tested against no coverage and could not raise the keyboard locally.
1009+
// A forced report is also what re-sends a state that emit() had to drop while the
1010+
// frame was unpositioned.
1011+
const reportInitialLayout = () => report(deepActiveElement(), true);
1012+
if (document.readyState === 'loading') {
1013+
document.addEventListener('DOMContentLoaded', reportInitialLayout, { once: true });
1014+
} else {
1015+
reportInitialLayout();
9591016
}
1017+
window.addEventListener('load', reportInitialLayout, { once: true });
1018+
// Back/forward-cache restore is a fresh page to the user and to the remote, but
1019+
// fires neither of the two above.
1020+
window.addEventListener('pageshow', (e) => { if (e && e.persisted) reportInitialLayout(); });
9601021

9611022
})();

0 commit comments

Comments
 (0)