amadan

oficina / docs Public

← Discussions

paste-fidelity: a Google Docs blank line is one line, not two closed

On branch worktree-paste-fidelity

claude driven by paul@keymail.dev session b67dca22 · claude-code/2.1.258 · DOCS-T1

Paul pasted One / Two / Three / <blank> / Four out of Google Docs. The blank line came back two lines tall, and an extra blank line appeared under the paste.

What was happening

Google spells an empty paragraph as a bare <br> sibling of the <p>s, inside the <b id="docs-internal-guid-…"> wrapper it puts around a copied selection — not as an empty <p>, and not as a <br> inside one. That distinction is the whole bug, and it is why my first four attempts to reproduce it failed: I had the <br> inside the paragraph every time.

Three things then happen, all of them prosemirror-view and prosemirror-model behaving exactly as designed:

  1. The clipboard parser ignores a <br> only when it is the last child of a non-inline parent. Google's has a next sibling, so it is kept, and schema-basic's hard_break rule claims it.
  2. At the open top of a parsed slice there is no block for an inline node to sit in, so the hard_break lands as a top-level sibling of the paragraphs. normalizeSiblings wraps it in the context's wrapper — a paragraph — merging consecutive breaks into one. The blank line arrives as p(hard_break).
  3. addTextblockHacks appends <br class="ProseMirror-trailingBreak"> to any textblock whose last child is not text, because contenteditable needs somewhere to put a cursor. p(hard_break) therefore draws as <p><br><br></p>: two lines where the author left one.

Google's trailing <br> before the closing </b> is step 3 again, and that is the extra blank line under the paste. One cause, both symptoms.

Only the editor was wrong. The no-JavaScript read view gets <p><br></p> from fragment_html and a lone <br> makes exactly one line box; internal/export/pdf.go's paragraph loop consumes a trailing break without emitting a line. So this is a paste bug, and it belongs on the way in.

The fix

fixtures/paste.mjs — a transformPasted prop that replaces a paragraph whose children are all hard_breaks with that many empty paragraphs.

transformPasted is the hook because it runs after the clipboard parser, which is the first moment the distinction is legible: an all-breaks paragraph can only have come from block-level <br>s, since a trailing <br> inside a <p> was already dropped in step 1 and a paragraph the author typed always has text in it.

Deliberately not a parseDOM rule — matching a <br> by its siblings needs a DOM test that the <b> wrapper defeats, and parseDOM is the wire contract. Deliberately not transformPastedHTML — string surgery on untrusted markup, and untestable in a gate with no DOM.

Nothing new crosses the wire: the output is an ordinary empty paragraph, which fragment.rs, pdf.go and markdown.go have each rendered since before this. So unlike align.mjs, this adds no vocabulary that four separate binaries have to keep in agreement.

fixtures/paste.test.mjs is 10 DOM-free tests, added to the gate. The one that matters guards the opposite mistake: a Shift+Enter break inside a line of text must survive verbatim. Turning every hard_break into a paragraph break would destroy soft breaks silently, and only on the paste path — which had no test at all before this.

Accepted knowingly: copying a CARLOS paragraph holding nothing but Shift+Enter breaks and pasting it back gives separate empty paragraphs. Where paragraphs stack like lines the two are indistinguishable on every surface, and the alternative is the two-line bug.

Gate is green (gate: ok, exit 0).

The engine question, reviewed by Fable

Paul asked whether ProseMirror is still the right engine. Fable's answer: yes, and for a reason stronger than the one §6.5 records.

The wire format is y-prosemirror's Y.XmlFragment, and core/src/fragment.rs is a fourth consumer of it that cannot run the JS library. Lexical (@lexical/yjs), Slate (slate-yjs) and Quill (y-quill) each store a different Yjs shape, so switching means rewriting the Rust read path, the PDF and Markdown exporters with it, and orphaning every existing document. None of them fixes paste either — Lexical's $generateNodesFromDOM has its own long-standing Google-Docs <br> bugs. TipTap is not an alternative; it is ProseMirror with a config layer over the identical clipboard code.

Suggested addition to §6.5: "A fourth consumer of the wire format is the Rust read path (core/src/fragment.rs), which cannot run the binding library. Any engine that stores a different Yjs shape re-implements that path and orphans every document; ProseMirror stays for that reason before any other."

Still open, not in this branch

  • Tables. A pasted Google Docs table flattens to one top-level paragraph per cell, row-major, grid gone — there is no table node in the schema. prosemirror-tables is the answer and is an extension, not a switch: colspan/rowspan are numeric, so fragment.rs clamps them the way it already clamps heading.level. Spec §11.2 already commits to tables, so this is scheduled work. Wants a written plan and a Fable review before it lands.
  • image and link are in the browser schema but not in the reader. A paste writes them into the CRDT, where fragment.rs drops the href silently and renders an image as nothing. Until fragment.rs grows a scheme-allowlisted href/src — the decision the MARKS comment defers — Fable's view is that the browser schema should be narrowed to what the reader can show, so a paste cannot write what a reader cannot see. Needs Paul's call.
  • Underline, strikethrough, colour, highlight and Docs' font sizes are dropped on paste. Correct behaviour under the current attribute-free MARKS discipline, not a bug — but it should be stated in the spec rather than read as breakage. Each would be a new vocabulary row across the same four files.
  • A pre-existing drift this fix does not touch: a paragraph ending in a hard_break (Shift+Enter at the end of a line, no paste involved) shows an extra empty line in the editor and none in the read view or the PDF. One CSS rule and one branch in pdf.go would make the three agree — but decide first which of them is right.

claude driven by paul@keymail.dev session b67dca22 · claude-code/2.1.258 · DOCS-T1

Paul's real clipboard bytes closed the second half

Captured off the actual Google Docs copy, and they say something the reconstruction did not. The blank line is a bare <br> sibling between the <p>s, as expected. But the trailing blank line was never Google's:

…<p>…Four…</p></b><br class="Apple-interchange-newline"><br class="Apple-interchange-newline">

Two of them, after the closing </b>, added by WebKit to mark where the copy stopped. ProseMirror's clipboard parser ignores a <br> that is the last child of a non-inline parent, which disposes of exactly one; the first has a next sibling and survives.

Commit 9782cbf ignores that marker by its class. Discriminating on the class rather than on position is what makes it safe: a blank line the author really did leave at the end is a <br> Google puts INSIDE its wrapper carrying no class, and a positional "drop a trailing empty paragraph" rule would have eaten it. Both are checked against the real bytes — Paul's copy now yields exactly the five blocks Google shows, and the same copy with a real trailing blank keeps six.

Local gate green, hub CI green on 1bfbcd7. Twelve tests in paste.test.mjs.

Plan 7 is up for review

docs/superpowers/plans/2026-09-02-images-links-tables.md (3f4ac69), from Paul's brief: "make a plan to handle images, links, and tables. Images can be rendered immediately and uploaded async to object storage". With Fable now.

The load-bearing decision is D1. fragment_html's output is template.HTML written unescaped, and it is safe today only because no byte from a document reaches attribute position. Tables keep that property — colspan/rowspan are integers, clamped like heading.level. Links and images spend it. So the plan buys it back once and deliberately (an escaper, a scheme allowlist, a rejected URL rendering as text rather than as a dead anchor, adversarial tests) before either caller exists — which is why links come before images and why Task 4 lands with no caller at all.

D4 is the ordering trick images need. The src in the CRDT is /d/{sqid}/blobs/{plaintext sha-256}, hashed in the browser before the upload starts, so the browser knows the final URL up front, inserts the node with its real src immediately, and shows a blob: preview through a NodeView until the upload lands — no placeholder attribute, no rewrite pass. §11.5 requires blobs.Sealed, whose address is the ciphertext's hash, so a small owned document_blobs table maps plaintext hash to blob hash and carries the per-document authorisation with it.

D6 is the open risk. A Google Docs paste gives a googleusercontent.com URL, not bytes, and fetching it from the browser is subject to CORS with no canvas fallback. The slice degrades loudly per §11.2 rather than smuggling in a server-side fetcher, which is an SSRF surface and its own decision. Worth testing against a real Doc before Task 7 is built — the answer changes the deliverable.

Coordination

Discussion #20 (worktree-import-documents) is building DOCX/Markdown/PDF import, which produces these same table/image/link nodes. Whoever gets there first should post the node names and attribute shapes here or there so the two branches do not invent different vocabularies. Plan 7 proposes prosemirror-tables' own four types with column resizing off, so colwidth stays null and nothing new-shaped crosses the wire.

claude driven by paul@keymail.dev session b67dca22 · claude-code/2.1.258 · DOCS-T1

Plan 7 revision 2 (d8470ac) — Fable's review changed two decisions

The escaper already existed. core/src/fragment.rs:25 escapes the same five characters in the same order that revision 1 proposed writing. So D1 is not escaper-plus-allowlist; it is the allowlist, the reject-to-text path and the tests, and it is small enough to be the first commit of the links branch rather than a branch landing with no caller — which AGENTS.md's own record is against, having deleted devauth.go for having zero readers.

Three real gaps in D1 that revision 1 missed:

  • The allowlist has to run in the shared path. In node_html alone, markdown.go would emit [text](javascript:…) and the PDF's /Link annotation the same, unless each exporter re-implemented the check — a second copy of a security table, the exact coupling the plan claims to avoid. A rejected href now means the run is emitted with no link mark at all, so no consumer can see a rejected URL because none is produced.
  • A positive scheme grammar, not a denylist. Rejecting control characters misses a value with no colon at all, and misses the code points browsers do not strip. ^[A-Za-z][A-Za-z0-9+.-]*: with the scheme in {http, https, mailto}. blob: joins the adversarial list — it is what this feature manipulates client-side and must never reach the CRDT.
  • src matches its literal shape, ^/d/[A-Za-z0-9]+/blobs/[0-9a-f]{64}$. "App-relative" admits //evil.example/x.png.
  • SVG in the accepted types runs script on the app's origin. The type list, byte-sniffing and the response headers are a D1 safety rule, not a Task 6 detail.

D4 reverses, and this is the important one. Inserting the node with its final src before the upload looked like it removed a placeholder. It did not: crypto.subtle.digest is async and transformPasted is synchronous, and the Google Docs path has no bytes at paste time at all. What it actually bought was a dangling src in a CRDT every tab already holds — other tabs and no-JavaScript readers get a 404 broken image, and a permanently failed upload becomes cross-tab node surgery with no owner.

So: the decoration renders immediately and the node is inserted on 2xx, at the decoration's mapped position. That is §6.1's own rule — placeholders are decorations, never nodes. Paul's brief is met unchanged, the whole failure class disappears, and the server computes the hash.

document_blobs grows content_type (§11.5 forces the stored type to application/octet-stream, so the true one cannot be recovered at serve time), plain_size rather than the sealed Ref.Size, and a unique key so a re-paste does not mint a fresh ciphertext object.

Nothing in the app calls SealSym today. There is no sealing key. That is a decision with a fail-closed shape and a relationship to the eventual per-document DEK, and it blocks Task 6.

D6 flips. A googleusercontent fetch with a cross-origin Origin returns access-control-allow-origin: *, from the same infrastructure serving the clipboard's docsz URLs — which are signed-and-expiring, not cookie-gated. The test still comes first, but the expected answer is now "it works", and the real risk is expiry mid-retry, not CORS.

"Degrade loudly" had nothing to degrade into. There is no paste report anywhere in the codebase and §11.2's import report is unbuilt. Now its own task, with UI, catalog keys and a copy review.

Also corrected: image is inline in schema-basic; internal/pdf's addStream Flate-compresses unconditionally and its /Resources carries only /Font; PNG alpha needs an /SMask; GFM's delimiter row is mandatory and its first row is the header, so a Docs table with no table_header needs one promoted; a Markdown image link is dead once the file leaves the app; noopener does nothing without target. And the link wire fixture must be generated through y-prosemirror rather than hand-built, because a mark's wire value is an attrs object and no existing fixture has that shape.

Two things now need Paul rather than more design: the sealing key, and squash-merge approval for the paste fix on this branch (1bfbcd7, 9782cbf — gate and hub CI green).