Skip to content

Commit 5437792

Browse files
authored
Document embedded viewer preview-testing harness (#14870)
Verifying #14869 needed a way to exercise the code in src/webui/quarto-preview/src/frame/ that only runs inside an embedded viewer (RStudio Viewer, VS Code Simple Browser, Posit Workbench). A plain top-level preview never reaches it, and quarto-cli has no e2e test that starts `quarto preview` at all. - Reference for the `quarto-preview-test` skill covering the iframe + postMessage host page pattern, a throwaway reverse proxy for reproducing a proxied deployment's browser/server origin mismatch, the server's first-request client-injection detection, and the precondition assertion needed before trusting a click result - Test workflows skip `.claude/**`-only changes, since agent config cannot affect build or test outcomes Related to #14869, #10696.
1 parent 227676f commit 5437792

7 files changed

Lines changed: 150 additions & 0 deletions

File tree

‎.claude/skills/quarto-preview-test/SKILL.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,13 @@ Invoked with test IDs (e.g. `/quarto-preview-test T17 T18`) or a topic descripti
169169
matching tests from `tests/docs/manual/preview/README.md`. Invoked without IDs or a topic, use
170170
the Edit-Verify Cycle above instead — the test matrix is for targeted regression testing.
171171

172+
## Embedded Viewer (iframe / postMessage)
173+
174+
When testing link classification, viewer `postMessage` events, or code in
175+
`src/webui/quarto-preview/src/frame/`, use the iframe and proxy harness in
176+
`references/embedded-viewer.md`. It covers the first-request constraint and the required
177+
precondition; a top-level preview does not exercise this code.
178+
172179
## Baseline Comparison
173180

174181
Compare dev build against installed release to distinguish regressions:
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
# Embedded Viewer Verification (iframe / postMessage)
2+
3+
Use this harness for link classification, viewer `postMessage` events, and other code in
4+
`src/webui/quarto-preview/src/frame/` that runs only in an embedded viewer (RStudio Viewer,
5+
VS Code Simple Browser, or Posit Workbench). A top-level browser preview does not exercise it.
6+
7+
## Why a harness is required
8+
9+
- `handleExternalLinks` (and similar embedded-viewer logic) early-returns when
10+
`window.self === window.top`. It only runs inside an iframe.
11+
- The client script injects embedded-viewer options only if the server's *first* HTML request
12+
includes `quartoPreviewReqId=`, `capabilities=`, or `vscodeBrowserReqId=` in its URL or
13+
referrer (`viewerIFrameURL` in `src/core/http-devserver.ts`). `injectClientInitialized`
14+
memoizes the result for the process lifetime; later requests do not trigger detection again.
15+
16+
## The host page pattern
17+
18+
Use a parent page that collects the viewer script's `postMessage` events and embeds the preview
19+
URL with a marker query string:
20+
21+
```html
22+
<!doctype html>
23+
<html><body>
24+
<pre id="log">[]</pre>
25+
<iframe id="f" src="/?quartoPreviewReqId=1" width="1000" height="700"></iframe>
26+
<script>
27+
window.__messages = [];
28+
addEventListener("message", (e) => {
29+
window.__messages.push(e.data);
30+
document.getElementById("log").textContent = JSON.stringify(window.__messages);
31+
});
32+
</script>
33+
</body></html>
34+
```
35+
36+
Read results with `agent-browser eval`, e.g.:
37+
38+
```js
39+
(() => { document.getElementById('f').contentDocument.getElementById('lnk-hash').click(); return 'clicked'; })()
40+
(() => JSON.stringify(window.__messages))()
41+
```
42+
43+
### First-request constraint
44+
45+
The iframe's marked request must be the first request the server injects into:
46+
47+
- Do not use a rendered `.qmd` parent. Its unmarked load reaches the render/inject pipeline first
48+
and disables option injection for later requests.
49+
- Send no unmarked request of any kind before the iframe's, with one exception: a path that is
50+
already on disk as a declared static resource. A 404 is not safe either — the `on404` handler
51+
calls `injectClient`, so a single unmarked request to a missing path consumes the detection.
52+
- For readiness, wait for the `Browse at` / `Listening on` lines in the preview output (both go to
53+
stderr) rather than polling an HTTP path. Polling is what produces the unmarked request above,
54+
and before the first render finishes even the parent page's own path still 404s.
55+
- Start preview with `--no-browser`, otherwise the auto-opened browser requests `/` unmarked.
56+
- Use a genuinely static resource for the parent page: declare it under `project.resources:` in
57+
`_quarto.yml`. Preview copies project resources to the output directory itself, so no separate
58+
`quarto render` is needed. Such a file has no corresponding input file, and the server injects the
59+
client script only into output files that map back to an input, so serving the parent page does
60+
not consume the detection.
61+
- If `QuartoPreview.getOptions()` returns `origin: ""` and `search: ""`, restart the preview
62+
process. The client cannot reset the memoized state.
63+
64+
## The proxy pattern (simulating Workbench / RStudio Server)
65+
66+
To make the browser origin differ from the origin inferred by the preview server, run a temporary
67+
reverse proxy on a second port. `127.0.0.1` and `localhost` resolve to the same host but are
68+
distinct browser origins, so they reproduce this mismatch on one machine:
69+
70+
```typescript
71+
// Browser origin is http://127.0.0.1:4445; deleting the Host header makes the preview
72+
// server infer its own backend origin (http://localhost:4444) instead of the proxy's.
73+
const kBackend = "http://localhost:4444";
74+
75+
Deno.serve({ port: 4445, hostname: "127.0.0.1" }, async (req) => {
76+
const url = new URL(req.url);
77+
const headers = new Headers(req.headers);
78+
headers.delete("host");
79+
headers.delete("accept-encoding");
80+
const res = await fetch(kBackend + url.pathname + url.search, {
81+
method: req.method, headers, body: req.body, redirect: "manual",
82+
});
83+
// fetch() decompresses the body but preserves the original encoding and length headers.
84+
// Remove them or the client truncates the decompressed body, potentially dropping the
85+
// injected client script near the end of the page.
86+
const resHeaders = new Headers(res.headers);
87+
resHeaders.delete("content-encoding");
88+
resHeaders.delete("content-length");
89+
return new Response(res.body, { status: res.status, headers: resHeaders });
90+
});
91+
```
92+
93+
Serve the host page from a path the proxy handles locally (for example, `/__host`) so it does
94+
not reach the backend before the iframe.
95+
96+
## Required precondition
97+
98+
Before clicking anything, confirm the intended origin relationship and injected options. Because
99+
this snippet reads the iframe, the parent and iframe must share a browser origin (see Known limits):
100+
101+
```js
102+
(() => {
103+
const w = document.getElementById('f').contentWindow;
104+
return JSON.stringify({
105+
parent: location.origin,
106+
iframe: w.location.origin,
107+
injected: w.QuartoPreview && w.QuartoPreview.getOptions(),
108+
});
109+
})()
110+
```
111+
112+
In the proxy case, `injected.origin` must differ from `location.origin`; in the direct localhost
113+
control case, they must match. In both cases, `injected.search` must contain the marker query
114+
string. Do not use subsequent click results if `injected` is `undefined` or `null`, either field
115+
is empty, or the origins do not have the expected relationship.
116+
117+
## Known limits
118+
119+
- Websocket upgrades (live reload) are not proxied by the script above; a failed
120+
`new WebSocket(...)` does not throw synchronously, so it doesn't affect the code under test.
121+
- A cross-origin parent cannot read the iframe's DOM. `contentDocument` returns `null` (no
122+
throw). Reading a property such as `contentWindow.QuartoPreview` or `contentWindow.location.origin`
123+
throws a `SecurityError`; on the cross-origin `Location` object specifically, only setting
124+
`href` and calling `replace()` are permitted. `postMessage` still works across origins, which
125+
is why it's the escape hatch: keep both frames on one browser origin, as in the proxy pattern
126+
above, or add iframe-side code that sends its location and injected options to the parent with
127+
`postMessage`.

‎.github/workflows/performance-check.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,13 @@ on:
44
workflow_dispatch:
55
pull_request:
66
branches: [main]
7+
paths-ignore:
8+
- ".claude/**"
79
push:
810
# only trigger on branches, not on tags
911
branches: [main]
12+
paths-ignore:
13+
- ".claude/**"
1014
schedule:
1115
- cron: 0 * * * *
1216

‎.github/workflows/test-bundle.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,13 @@ on:
66
workflow_dispatch:
77
pull_request:
88
branches: [main]
9+
paths-ignore:
10+
- ".claude/**"
911
push:
1012
# only trigger on branches, not on tags
1113
branches: [main]
14+
paths-ignore:
15+
- ".claude/**"
1216

1317
jobs:
1418
test-bundle:

‎.github/workflows/test-ff-matrix.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ on:
1515
paths-ignore:
1616
- "news/**"
1717
- "src/resources/language/**"
18+
- ".claude/**"
1819
- ".github/workflows/create-release.yml"
1920
- ".github/workflows/performance-check.yml"
2021
- ".github/workflows/stale-needs-repro.yml"
@@ -28,6 +29,7 @@ on:
2829
paths-ignore:
2930
- "news/**"
3031
- "src/resources/language/**"
32+
- ".claude/**"
3133
- ".github/workflows/create-release.yml"
3234
- ".github/workflows/performance-check.yml"
3335
- ".github/workflows/stale-needs-repro.yml"

‎.github/workflows/test-quarto-latexmk.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@ on:
33
workflow_dispatch:
44
push:
55
branches: main
6+
paths-ignore:
7+
- ".claude/**"
68
pull_request:
79
branches: main
10+
paths-ignore:
11+
- ".claude/**"
812
name: Test quarto-latexmk
913

1014
jobs:

‎.github/workflows/test-smokes-parallel.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ on:
2323
# are not relevant to the tested features
2424
- "src/resources/language/**"
2525
- "dev-docs/**"
26+
- ".claude/**"
2627
# don't run on PR working on other workflows
2728
- ".github/workflows/create-release.yml"
2829
- ".github/workflows/performance-check.yml"
@@ -39,6 +40,7 @@ on:
3940
- "v[1-9].[0-9]+" # run also on released version branch (for patch releases)
4041
paths-ignore:
4142
- "news/**"
43+
- ".claude/**"
4244

4345
concurrency:
4446
# Use github.run_id on main branch

0 commit comments

Comments
 (0)