Skip to content

Commit ebe8ca8

Browse files
authored
Merge pull request #40 from reclaimprotocol/fix/kbd-occlusion-hardening
Fix/kbd occlusion hardening
2 parents c94fd4b + 8625c01 commit ebe8ca8

67 files changed

Lines changed: 6439 additions & 242 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

images/minimal-vnc-desktop/Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ ENV CDP_INTERNAL_PORT=9223
8282
ENV CHROME_REMOTE_DEBUGGING_PORT=9223
8383
ENV CDP_RESTRICTED_PORT=9222
8484
ENV CDP_FULL_PORT=9226
85-
ENV MVD_EMULATOR_OFF=1
85+
ENV MVD_EMULATOR_OFF=0
8686
ENV APP_COMMAND=/usr/local/bin/start-chromium
8787
ENV APP_URL=
8888
ENV READY_FILE=/tmp/minimal-vnc-ready

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;

0 commit comments

Comments
 (0)