fix(web): seed Edit-and-replay from what replay sends, name each JSON editor
Copilot's fourth pass, one inline plus two suppressed. All three were right. - The modal presented the whole captured frame as editable, but replay does not re-send a frame: it dispatches through the typed InspectorClient methods, whose signatures have no room for `_meta` (a captured `tools/call` carries `_meta.progressToken`) or for any list-request key but `cursor`. So editing `_meta` and pressing Send transmitted something other than what was on screen. `replayableParams` now projects the frame down to what the dispatcher actually reads, the modal seeds from that, and it names what was left out rather than leaving the difference to be discovered. The projection lives beside the dispatch switch because the two are halves of one contract. - `ContentViewer` gains `jsonLabel`, so a read-only JSON editor can say what it holds. Every one of them announced "JSON content", and an expanded Protocol entry has two while the list has many pairs — leaving a screen reader with indistinguishable textboxes and only the visible headings to tell them apart. Protocol entries now name theirs per method and direction, Network entries per request/response body. - `isJsonDocument` became `formatJsonDocument`, returning the formatted text instead of a boolean. The heuristic path parsed the payload to decide and then parsed it again to render; Protocol and Network hand over serialized bodies as untyped text and are not virtualized, so a long history paid twice per payload per render. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
12c00d8c63
commit
5abe289caa
4 changed files with 413 additions and 31 deletions
|
|
@ -232,6 +232,29 @@ describe("ContentViewer", () => {
|
|||
expect(screen.getByText(/"a": 1/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// An expanded Protocol entry holds two of these and a list holds many pairs,
|
||||
// so an unnamed editor leaves a screen reader tabbing through textboxes that
|
||||
// all announce the same thing.
|
||||
it("names the JSON editor from jsonLabel", () => {
|
||||
const block: ContentBlock = { type: "text", text: '{"a":1}' };
|
||||
renderWithMantine(
|
||||
<ContentViewer
|
||||
block={block}
|
||||
mimeType="application/json"
|
||||
jsonLabel="tools/call response JSON"
|
||||
/>,
|
||||
);
|
||||
expect(
|
||||
screen.getByLabelText(/tools\/call response JSON/),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("falls back to a generic name when no label is given", () => {
|
||||
const block: ContentBlock = { type: "text", text: '{"a":1}' };
|
||||
renderWithMantine(<ContentViewer block={block} />);
|
||||
expect(screen.getByLabelText(/JSON content/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("keeps the copy overlay above the JSON editor", () => {
|
||||
const block: ContentBlock = { type: "text", text: '{"a":1}' };
|
||||
renderWithMantine(
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ import { ResourceLinkInfo } from "../ResourceLinkInfo/ResourceLinkInfo";
|
|||
import {
|
||||
formatJson,
|
||||
formatXml,
|
||||
formatJsonDocument,
|
||||
getMimeKind,
|
||||
isJsonDocument,
|
||||
isSafeHref,
|
||||
isTextualKind,
|
||||
looksLikeJson,
|
||||
|
|
@ -60,6 +60,19 @@ export interface ContentViewerProps {
|
|||
* one line — don't pass it for multi-line content.
|
||||
*/
|
||||
wrap?: boolean;
|
||||
/**
|
||||
* Accessible name for the read-only JSON editor a JSON payload renders in.
|
||||
*
|
||||
* Say what the payload *is* whenever a screen can hold more than one: an
|
||||
* expanded Protocol entry carries both its parameters and its response, and
|
||||
* a list of them carries many such pairs — so the default leaves a screen
|
||||
* reader tabbing through textboxes that all announce the same thing, with
|
||||
* only the visible section headings to tell them apart.
|
||||
*
|
||||
* Reaches the JSON branch only; every other renderer here is not a control
|
||||
* and takes no name.
|
||||
*/
|
||||
jsonLabel?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -185,12 +198,24 @@ function HighlightedContent({
|
|||
* It copies the *original* text, not the pretty-printed form, matching every
|
||||
* other branch here.
|
||||
*/
|
||||
function JsonContent({ text, copyable }: { text: string; copyable: boolean }) {
|
||||
function JsonContent({
|
||||
text,
|
||||
formatted,
|
||||
copyable,
|
||||
label,
|
||||
}: {
|
||||
/** The original payload — what the copy button copies. */
|
||||
text: string;
|
||||
/** Its pretty-printed form, already parsed once by whoever decided to use it. */
|
||||
formatted: string;
|
||||
copyable: boolean;
|
||||
label: string;
|
||||
}) {
|
||||
return (
|
||||
<CopyableWrapper copyable={copyable} copyValue={text}>
|
||||
<JsonEditor
|
||||
ariaLabel="JSON content"
|
||||
value={formatJson(text)}
|
||||
ariaLabel={label}
|
||||
value={formatted}
|
||||
readOnly
|
||||
// Sized to the payload, and capped.
|
||||
//
|
||||
|
|
@ -221,17 +246,30 @@ function PlainTextContent({
|
|||
text,
|
||||
copyable,
|
||||
wrap,
|
||||
jsonLabel,
|
||||
}: {
|
||||
text: string;
|
||||
copyable: boolean;
|
||||
wrap: boolean;
|
||||
jsonLabel: string;
|
||||
}) {
|
||||
// Untyped text that really parses as JSON gets the JSON renderer too — the
|
||||
// heuristic is how a server that sent no MIME type still reads well. Not when
|
||||
// `wrap` is false, though: that caller wants one clipped line at a fixed
|
||||
// height, which a multi-line editor is not.
|
||||
if (wrap && isJsonDocument(text)) {
|
||||
return <JsonContent text={text} copyable={copyable} />;
|
||||
//
|
||||
// The formatted text comes back from the same call that decided, so the
|
||||
// payload is parsed once rather than once to test and again to render.
|
||||
const asJson = wrap ? formatJsonDocument(text) : null;
|
||||
if (asJson !== null) {
|
||||
return (
|
||||
<JsonContent
|
||||
text={text}
|
||||
formatted={asJson}
|
||||
copyable={copyable}
|
||||
label={jsonLabel}
|
||||
/>
|
||||
);
|
||||
}
|
||||
const displayText = looksLikeJson(text) ? formatJson(text) : text;
|
||||
return (
|
||||
|
|
@ -262,11 +300,13 @@ function TextualContent({
|
|||
mimeType,
|
||||
copyable,
|
||||
wrap,
|
||||
jsonLabel,
|
||||
}: {
|
||||
text: string;
|
||||
mimeType: string | undefined;
|
||||
copyable: boolean;
|
||||
wrap: boolean;
|
||||
jsonLabel: string;
|
||||
}) {
|
||||
const kind = mimeType ? getMimeKind(mimeType) : "text";
|
||||
switch (kind) {
|
||||
|
|
@ -278,9 +318,23 @@ function TextualContent({
|
|||
// line is exactly what that prop is documented not to be used for, so
|
||||
// that caller keeps the plain renderer.
|
||||
return wrap ? (
|
||||
<JsonContent text={text} copyable={copyable} />
|
||||
<JsonContent
|
||||
text={text}
|
||||
// A *declared* `application/json` renders as JSON even when it does
|
||||
// not parse — the server said what it sent — so this cannot go
|
||||
// through `formatJsonDocument`, which reports unparseable text as
|
||||
// not-JSON. `formatJson` returns the original on failure.
|
||||
formatted={formatJson(text)}
|
||||
copyable={copyable}
|
||||
label={jsonLabel}
|
||||
/>
|
||||
) : (
|
||||
<PlainTextContent text={text} copyable={copyable} wrap={wrap} />
|
||||
<PlainTextContent
|
||||
text={text}
|
||||
copyable={copyable}
|
||||
wrap={wrap}
|
||||
jsonLabel={jsonLabel}
|
||||
/>
|
||||
);
|
||||
case "xml":
|
||||
return (
|
||||
|
|
@ -313,7 +367,14 @@ function TextualContent({
|
|||
</Stack>
|
||||
);
|
||||
default:
|
||||
return <PlainTextContent text={text} copyable={copyable} wrap={wrap} />;
|
||||
return (
|
||||
<PlainTextContent
|
||||
text={text}
|
||||
copyable={copyable}
|
||||
wrap={wrap}
|
||||
jsonLabel={jsonLabel}
|
||||
/>
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -341,11 +402,13 @@ function ResourceContent({
|
|||
mimeType,
|
||||
copyable,
|
||||
wrap,
|
||||
jsonLabel,
|
||||
}: {
|
||||
contents: TextResourceContents | BlobResourceContents;
|
||||
mimeType: string;
|
||||
copyable: boolean;
|
||||
wrap: boolean;
|
||||
jsonLabel: string;
|
||||
}) {
|
||||
if ("text" in contents) {
|
||||
return (
|
||||
|
|
@ -354,6 +417,7 @@ function ResourceContent({
|
|||
mimeType={mimeType}
|
||||
copyable={copyable}
|
||||
wrap={wrap}
|
||||
jsonLabel={jsonLabel}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
@ -382,6 +446,7 @@ function ResourceContent({
|
|||
mimeType={mimeType}
|
||||
copyable={copyable}
|
||||
wrap={wrap}
|
||||
jsonLabel={jsonLabel}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
@ -394,11 +459,13 @@ function BlockContent({
|
|||
mimeType,
|
||||
copyable,
|
||||
wrap,
|
||||
jsonLabel,
|
||||
}: {
|
||||
block: ContentBlock;
|
||||
mimeType: string | undefined;
|
||||
copyable: boolean;
|
||||
wrap: boolean;
|
||||
jsonLabel: string;
|
||||
}) {
|
||||
switch (block.type) {
|
||||
case "text":
|
||||
|
|
@ -408,6 +475,7 @@ function BlockContent({
|
|||
mimeType={mimeType}
|
||||
copyable={copyable}
|
||||
wrap={wrap}
|
||||
jsonLabel={jsonLabel}
|
||||
/>
|
||||
);
|
||||
case "image":
|
||||
|
|
@ -448,6 +516,7 @@ export function ContentViewer({
|
|||
copyable = false,
|
||||
mimeType,
|
||||
wrap = true,
|
||||
jsonLabel = "JSON content",
|
||||
}: ContentViewerProps) {
|
||||
if (contents) {
|
||||
const effective =
|
||||
|
|
@ -458,6 +527,7 @@ export function ContentViewer({
|
|||
mimeType={effective}
|
||||
copyable={copyable}
|
||||
wrap={wrap}
|
||||
jsonLabel={jsonLabel}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
@ -468,6 +538,7 @@ export function ContentViewer({
|
|||
mimeType={mimeType}
|
||||
copyable={copyable}
|
||||
wrap={wrap}
|
||||
jsonLabel={jsonLabel}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ import {
|
|||
getMimeKind,
|
||||
isSafeHref,
|
||||
isTextualKind,
|
||||
isJsonDocument,
|
||||
formatJsonDocument,
|
||||
looksLikeJson,
|
||||
PREVIEW_HTML_CSP,
|
||||
tryDecodeBase64ToBytes,
|
||||
|
|
@ -113,26 +113,26 @@ describe("looksLikeJson", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("isJsonDocument", () => {
|
||||
it("accepts text that really parses as JSON", () => {
|
||||
expect(isJsonDocument(' {"a":1}')).toBe(true);
|
||||
expect(isJsonDocument("[1,2]")).toBe(true);
|
||||
describe("formatJsonDocument", () => {
|
||||
it("pretty-prints text that really parses as JSON", () => {
|
||||
expect(formatJsonDocument(' {"a":1}')).toBe('{\n "a": 1\n}');
|
||||
expect(formatJsonDocument("[1,2]")).toBe("[\n 1,\n 2\n]");
|
||||
});
|
||||
|
||||
// The stricter half of the heuristic (#2151). Text that merely *starts* like
|
||||
// JSON must not be handed to the JSON editor: a note beginning `{` would then
|
||||
// be presented as a document, in a JSON gutter, framed as malformed.
|
||||
it("rejects text that only looks like JSON", () => {
|
||||
expect(isJsonDocument("{ broken")).toBe(false);
|
||||
expect(isJsonDocument("[unclosed")).toBe(false);
|
||||
expect(isJsonDocument("plain")).toBe(false);
|
||||
it("reports text that only looks like JSON as not JSON", () => {
|
||||
expect(formatJsonDocument("{ broken")).toBeNull();
|
||||
expect(formatJsonDocument("[unclosed")).toBeNull();
|
||||
expect(formatJsonDocument("plain")).toBeNull();
|
||||
});
|
||||
|
||||
// A bare scalar is valid JSON but never looked like a document, so it stays
|
||||
// on the plain renderer — otherwise ordinary prose that happens to be a
|
||||
// number would open an editor.
|
||||
it("rejects a scalar that never looked like a document", () => {
|
||||
expect(isJsonDocument("42")).toBe(false);
|
||||
expect(formatJsonDocument("42")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -143,7 +143,8 @@ export function looksLikeJson(text: string): boolean {
|
|||
}
|
||||
|
||||
/**
|
||||
* Whether untyped text really is a JSON document, not merely shaped like one.
|
||||
* Pretty-print untyped text if it really is a JSON document, or report that it
|
||||
* is not.
|
||||
*
|
||||
* {@link looksLikeJson} only tests the first character, which is enough to
|
||||
* decide whether to *try* pretty-printing — a failed parse falls back to the
|
||||
|
|
@ -152,14 +153,18 @@ export function looksLikeJson(text: string): boolean {
|
|||
* JSON document, in a JSON gutter, with the server's plain text framed as
|
||||
* malformed. A declared `application/json` is different — there the server said
|
||||
* what it sent, so its own broken payload is shown as JSON on purpose.
|
||||
*
|
||||
* Returns the formatted text rather than a boolean so the caller does not parse
|
||||
* a second time to render what this already parsed to decide. Protocol and
|
||||
* Network entries hand their serialized bodies over as untyped text and are not
|
||||
* virtualized, so a long history pays for every parse on every render.
|
||||
*/
|
||||
export function isJsonDocument(text: string): boolean {
|
||||
if (!looksLikeJson(text)) return false;
|
||||
export function formatJsonDocument(text: string): string | null {
|
||||
if (!looksLikeJson(text)) return null;
|
||||
try {
|
||||
JSON.parse(text);
|
||||
return true;
|
||||
return JSON.stringify(JSON.parse(text), null, 2);
|
||||
} catch {
|
||||
return false;
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue