fix(content): pasted screenshots keep their proportions in posts and comments

SELF-IMPROVE.md

@@ -755,6 +755,32 @@ Both traps and this recipe belong in the helper the first occurrence asked for.
 Every remaining batch of the language work mounts routes, so the helper is now
 the cheaper thing to build.
 
+## 2x — The coverage and mutation gates read HEAD, not the working tree
+
+Both gates ask git for the diff between the merge base and `HEAD`
+(`git diff -U0 <merge-base> HEAD` in `scripts/mutation-check.ts`, the same in
+`diff-coverage-check.ts`). So running either one over uncommitted work does not
+grade that work: it grades the previous commit and reports a confident PASS.
+
+That is exactly how it reads on screen. The run said `3 file(s), 80 line(s) — 12
+executed, 0 never executed` and `PASS: every line this change added was executed
+by a test` — while the new module, its suite and the component it rewired were
+all still unstaged. Nothing in the output says "your change is not in this
+measurement", because from the gate's point of view there is no change.
+
+The cost is a wasted 60-second coverage run and, worse, a moment of believing an
+untested file was covered. Committing first turned the same command into
+`5 file(s), 209 line(s) — 57 executed, 1 never executed` and named the line.
+
+Either would fix it: have both gates refuse to run with a dirty tree, or have
+them diff the working tree (`git diff <merge-base>` without `HEAD`) and say which
+of the two they did in the line they already print about the merge base.
+
+Hit again on a one-line serializer fix: the run over an unstaged change printed
+`Judged 0 file(s), 0 line(s) — 0 executed` and still ended in `PASS: every line
+this change added was executed by a test`. Zero lines judged is the clearest
+possible sign that the gate saw no change, and it is printed as a pass.
+
 ## 1x — A migration passes every local gate and fails CI on schema drift
 
 `bun run db:check-drift` is a CI step (inside the `test` shard, not `check`), and
@@ -899,27 +925,6 @@ and copy it back, or commit the change before probing and use
 `git checkout HEAD -- <file>` knowingly — the probe is then genuinely the only
 thing that gets discarded.
 
-## 1x — The coverage and mutation gates read HEAD, not the working tree
-
-Both gates ask git for the diff between the merge base and `HEAD`
-(`git diff -U0 <merge-base> HEAD` in `scripts/mutation-check.ts`, the same in
-`diff-coverage-check.ts`). So running either one over uncommitted work does not
-grade that work: it grades the previous commit and reports a confident PASS.
-
-That is exactly how it reads on screen. The run said `3 file(s), 80 line(s) — 12
-executed, 0 never executed` and `PASS: every line this change added was executed
-by a test` — while the new module, its suite and the component it rewired were
-all still unstaged. Nothing in the output says "your change is not in this
-measurement", because from the gate's point of view there is no change.
-
-The cost is a wasted 60-second coverage run and, worse, a moment of believing an
-untested file was covered. Committing first turned the same command into
-`5 file(s), 209 line(s) — 57 executed, 1 never executed` and named the line.
-
-Either would fix it: have both gates refuse to run with a dirty tree, or have
-them diff the working tree (`git diff <merge-base>` without `HEAD`) and say which
-of the two they did in the line they already print about the merge base.
-
 ## 1x — A cache key nested under another's prefix, with six copies of the patch that reads it
 
 `inboxKeys.facetCounts()` was deliberately nested under `inboxKeys.lists()`

apps/web/src/lib/shared/__tests__/content-html.test.ts

@@ -4,8 +4,27 @@
  * tiptap-react, or browser globals. These pin the block/mark rendering the email
  * body relies on, and the text-node escaping that stops stored content from
  * injecting raw HTML into a recipient's inbox.
+ *
+ * Image dimensions. The editor stores a width and a height with every image,
+ * and for a pasted screenshot those are the extension's 500x500 defaults, not
+ * the picture's. The reader therefore promises:
+ *
+ * V1 An image is shown in its own proportions. The dimensions saved with it
+ *    may reserve its space before it loads, but never stretch or squash the
+ *    loaded image.
+ * V2 When both dimensions were saved, the reader reserves exactly that box
+ *    (width and height) before the image loads.
+ * V3 When only a width was saved, the reader fixes the width and leaves the
+ *    height to the image: no box is reserved and no ratio is stated.
+ * V4 A saved dimension the reader does not trust (above 4096 px) counts as not
+ *    saved. Stored attributes are input, and `safePositiveInt` is the bound.
+ * V5 An image the reader will not show leaves nothing behind: no tag, no
+ *    placeholder, whether its source was rejected or it never had one.
+ * V6 An image without alternative text carries an empty alt, never invented
+ *    text.
  */
 import { describe, it, expect } from 'vitest'
+import fc from 'fast-check'
 import type { JSONContent } from '@tiptap/core'
 import { generateContentHTML } from '../content-html'
 
@@ -173,3 +192,69 @@ describe('generateContentHTML', () => {
     )
   })
 })
+
+describe('generateContentHTML image dimensions', () => {
+  const doc = (attrs: Record<string, unknown>): JSONContent => ({
+    type: 'doc',
+    content: [
+      { type: 'resizableImage', attrs: { src: 'https://cdn.example.com/shot.png', ...attrs } },
+    ],
+  })
+
+  it('reserves the saved box before load and lets the image keep its own proportions (V1, V2)', () => {
+    const html = generateContentHTML(doc({ width: 500, height: 500 }))
+    expect(html).toContain('width="500" height="500"')
+    expect(html).toContain('style="aspect-ratio: auto 500 / 500;"')
+    expect(html).toContain('class="max-w-full h-auto rounded-lg"')
+  })
+
+  it('never states a ratio the browser must obey, whatever dimensions were saved (V1, V2, V4)', () => {
+    // A `500 / 500` without `auto` is what squashed pasted screenshots into
+    // squares. Every ratio the reader writes has to yield to the loaded image.
+    //
+    // The range is the extension's own maximum (16384), which reaches past the
+    // reader's trust bound. The first run of this property, written as "the
+    // ratio is always stated", failed at [1, 4097]: a dimension above 4096 is
+    // dropped by `safePositiveInt` and the image falls back to the width-only
+    // form. That bound predates this change and is deliberate input
+    // validation, so the property now states it (V4) instead of asserting a
+    // box the reader must not trust.
+    const TRUSTED_MAX = 4096
+    fc.assert(
+      fc.property(
+        fc.integer({ min: 1, max: 16384 }),
+        fc.integer({ min: 1, max: 16384 }),
+        (w, h) => {
+          const html = generateContentHTML(doc({ width: w, height: h }))
+          // Unguarded, across both branches: no ratio without `auto`, ever.
+          expect(html).not.toMatch(/aspect-ratio:(?!\s*auto\b)/)
+          if (w <= TRUSTED_MAX && h <= TRUSTED_MAX) {
+            expect(html).toContain(`width="${w}" height="${h}"`)
+            expect(html).toContain(`aspect-ratio: auto ${w} / ${h};`)
+          } else {
+            expect(html).not.toContain('height=')
+            expect(html).not.toContain('aspect-ratio')
+          }
+        }
+      )
+    )
+  })
+
+  it('fixes only the width when no height was saved, so the image decides its height (V3)', () => {
+    const html = generateContentHTML(doc({ width: 640 }))
+    expect(html).toContain('style="width:640px;"')
+    expect(html).not.toContain('height=')
+    expect(html).not.toContain('aspect-ratio')
+  })
+
+  it('leaves nothing behind for an image it will not show (V5)', () => {
+    expect(generateContentHTML(doc({ src: 'javascript:alert(1)', width: 500, height: 500 }))).toBe(
+      ''
+    )
+    expect(generateContentHTML({ type: 'doc', content: [{ type: 'image' }] })).toBe('')
+  })
+
+  it('gives an image without alternative text an empty alt (V6)', () => {
+    expect(generateContentHTML(doc({ width: 500, height: 500 }))).toContain(' alt="" ')
+  })
+})

apps/web/src/lib/shared/content-html.ts

@@ -173,7 +173,13 @@ export function generateContentHTML(content: JSONContent): string {
         const imgHeight =
           node.attrs?.height !== undefined ? safePositiveInt(node.attrs.height, 0) : 0
         if (imgWidth && imgHeight) {
-          const style = `style="aspect-ratio: ${imgWidth} / ${imgHeight};"`
+          // `auto` is load-bearing. A pasted screenshot is inserted with no
+          // dimensions, so what gets stored is the editor extension's 500x500
+          // default, and a bare `aspect-ratio: 500 / 500` forces a wide
+          // screenshot into a square. With `auto` the browser reserves the
+          // stored box only until the image has loaded, then uses the image's
+          // own proportions.
+          const style = `style="aspect-ratio: auto ${imgWidth} / ${imgHeight};"`
           return `<img src="${src}" alt="${alt}" width="${imgWidth}" height="${imgHeight}" class="max-w-full h-auto rounded-lg" ${style} />`
         }
         // Only apply width (not height) so h-auto preserves aspect ratio