Skip to content

docs(reports): JPEG image paste investigation + proposed fix#257

Open
amikofalvy wants to merge 1 commit intomainfrom
worktree-jpeg-image-support-investigation
Open

docs(reports): JPEG image paste investigation + proposed fix#257
amikofalvy wants to merge 1 commit intomainfrom
worktree-jpeg-image-support-investigation

Conversation

@amikofalvy
Copy link
Copy Markdown
Contributor

Summary

  • Investigation + report for why "JPEGs aren't supported for some reason" in the markdown editor.
  • Root cause located in the clipboard dispatcher (packages/app/src/editor/clipboard/handle-paste.ts), not the upload pipeline — the upload endpoint, sibling-asset filter, drag-and-drop, slash-command picker, markdown-source rendering, and sirv serving all already work for JPEG (verified with Playwright against a live dev server).
  • The bug is clipboard-paste-specific and triggered when the clipboard carries both a File and a text/html payload — typical for macOS Preview, Photos, Finder "Copy" on an image, and browser "Copy image". In that case Branch D (tryBranchHtml) handles the HTML first, inserts a PM image node with src="file://..." (which the browser can't load from an http:// origin), and returns true, preempting @tiptap/extension-file-handler's paste handler that would have uploaded the actual File.
  • Not JPEG-specific — PNG/GIF/WebP fail identically with the same clipboard shape. Appears JPEG-biased because Cmd+Shift+4 screenshots (the main PNG source) produce file-only clipboards that fall through cleanly.

Proposed fix

~5-line guard at the top of createHandlePaste that returns false when clipboardData.files contains an image on ALLOWED_IMAGE_MIME_TYPES, deferring to FileHandler. Full diff, rationale, rejected alternatives, and unit + Playwright test plan are in the report. Not applied in this PR — this is the proposal for your review.

Evidence

Six Playwright probes under reports/jpeg-image-paste-support/evidence/ reproduce both the bug and the happy paths:

Probe What it proves
jpeg-mixed-paste-probe.mjs JPEG paste with File + text/html → 0 uploads, broken <img src="file://..."> inserted (bug)
png-mixed-paste-probe.mjs PNG with same shape → identical failure (bug is not JPEG-specific)
jpeg-paste-probe.mjs JPEG paste with File only → 1 upload, image renders 1x1 (control)
jpeg-drop-probe.mjs JPEG drag-and-drop → 1 upload, image renders 1x1 (control)
jpeg-browser-probe.mjs Markdown-source ![alt](photo.jpg) renders correctly (control)
jpeg-html-probe.mjs Dumps editor HTML to confirm <img> structure

Test plan

  • Read reports/jpeg-image-paste-support/REPORT.md and confirm the proposed fix is in the right place (top of createHandlePaste, above all branches).
  • Confirm the "out of scope" items in §7 of the report are correctly scoped out (Chrome re-encoding, data-URL images, SVG inline embedding, paste-dispatcher redesign).
  • If approved, I'll open a follow-up PR that applies the diff, adds the unit tests described in §6 to handle-paste.test.ts, and adds the Playwright e2e file described in §6 to packages/app/tests/stress/paste-image.e2e.ts.

🤖 Generated with Claude Code

Root cause for "JPEGs aren't supported" is in the clipboard dispatcher,
not the upload pipeline. When the clipboard carries both a File and
text/html (typical for Preview/Photos/Finder/browser image copy on
macOS), `createHandlePaste` Branch D fires on the HTML, inserts a
broken `<img src="file://...">` PM node, and returns true — preempting
`@tiptap/extension-file-handler`, so the actual File is never uploaded.
Not JPEG-specific (PNG/GIF/WebP fail identically with the same clipboard
shape); appears JPEG-biased because Cmd+Shift+4 screenshots produce
file-only clipboards that fall through cleanly.

Report proposes a ~5-line guard at the top of createHandlePaste that
defers to FileHandler when clipboardData.files contains an
ALLOWED_IMAGE_MIME_TYPES image. Evidence probes (jpeg-paste-probe,
jpeg-mixed-paste-probe, png-mixed-paste-probe, jpeg-drop-probe,
jpeg-browser-probe, jpeg-html-probe) reproduce the failure and confirm
every other path (upload endpoint, markdown source reference, drop,
slash-command picker, file-only paste) already works for JPEG.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 22, 2026

⚠️ No Changeset found

Latest commit: 3582582

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Copy Markdown

@inkeep-internal-ci inkeep-internal-ci Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review Summary

(4) Total Issues | Risk: Low

🟠⚠️ Major (1) 🟠⚠️

Inline Comments:

  • 🟠 Major: REPORT.md:1-5 Frontmatter schema doesn't match repository report format

🟡 Minor (2) 🟡

Inline Comments:

  • 🟡 Minor: REPORT.md:188 Recommendation wording is confusing ("option 1" mismatch)
  • 🟡 Minor: REPORT.md:243 Evidence location statement is outdated

💭 Consider (1) 💭

💭 1) REPORT.md:60 Brittle line number reference

Issue: The root cause section references TiptapEditor.tsx:138 which may become stale after refactoring.
Why: Line number references in documentation become misleading over time.
Fix: Consider describing the wiring without the line number: "wired as editorProps.handlePaste in TiptapEditor.tsx via the handlePaste prop on EditorContent"


💡 APPROVE WITH SUGGESTIONS

Summary: Excellent investigation report with thorough root cause analysis, clear reproduction steps, and a well-reasoned proposed fix. The technical analysis is accurate — I verified the proposed fix location in handle-paste.ts and the dispatcher branch ordering matches the report's description. The six Playwright probes correctly reproduce the bug and control cases.

The only blocking issue is the frontmatter schema mismatch, which prevents proper catalogue indexing. The other suggestions are minor documentation polish.

On the proposed fix itself: The guard placement at the top of createHandlePaste (above all branches) is the correct approach. It's small (~5 lines), well-scoped, and doesn't introduce coupling between tryBranchHtml and FileHandler. Looking forward to the follow-up PR that implements it with the described unit and E2E tests.


Discarded (0)

No findings discarded.

Reviewers (2)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
pr-review-docs 6 0 1 0 3 0 2
pr-review-standards 0 0 0 0 0 0 0
Total 6 0 1 0 3 0 2

Note: pr-review-standards found no issues in the evidence probes — they correctly simulate clipboard events and reproduce the documented bug.

Comment on lines +1 to +5
---
title: JPEG images aren't supported on paste (it's actually all images — JPEG-biased by observation)
description: Investigation of why JPEGs "don't work" when pasted into the markdown editor. Root cause is in the clipboard dispatcher, not the upload pipeline, and the same bug affects PNG/GIF/WebP when the clipboard carries both a `File` object and a `text/html` payload.
tags: [report, editor, clipboard, paste, images]
---
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 MAJOR: Frontmatter schema doesn't match repository report format

Issue: The frontmatter uses tags: [...] but the repository's established report format uses a richer schema with createdAt, updatedAt, subjects, and topics fields.

Why: The report won't be properly indexed by the catalogue generator (generate-catalogue.ts) which expects these fields. The catalogue at reports/CATALOGUE.md displays these in its index table.

Fix:

Suggested change
---
title: JPEG images aren't supported on paste (it's actually all images — JPEG-biased by observation)
description: Investigation of why JPEGs "don't work" when pasted into the markdown editor. Root cause is in the clipboard dispatcher, not the upload pipeline, and the same bug affects PNG/GIF/WebP when the clipboard carries both a `File` object and a `text/html` payload.
tags: [report, editor, clipboard, paste, images]
---
---
title: "JPEG Image Paste Support: Investigation and Proposed Fix"
description: "Investigation of why JPEGs don't work when pasted into the markdown editor. Root cause is in the clipboard dispatcher, not the upload pipeline, and the same bug affects PNG/GIF/WebP when the clipboard carries both a File object and a text/html payload."
createdAt: 2026-04-22
updatedAt: 2026-04-22
subjects:
- TipTap
- ProseMirror
- "@tiptap/extension-file-handler"
- clipboard API
topics:
- clipboard paste behavior
- image upload
- paste dispatcher
- FileHandler
---

Refs:

3. **Let both handlers run and de-dupe** — not possible; first-returns-true stops dispatch. Rejected.
4. **Stop using `tryBranchHtml` for `<img>`-only HTML** — plausible follow-up but doesn't fix clipboards that carry mixed `<img>` + real rich-text content. Insufficient.

Recommend option 1 as documented in the diff above. Small, local, well-scoped.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor: Recommendation wording is confusing

Issue: "Recommend option 1" refers to the first alternative listed (return early inside Branch D), but the proposed fix is actually a guard at the top of createHandlePaste — which is distinct from all the listed alternatives.

Why: A reader following the alternatives section would be confused about which approach is being recommended.

Fix:

Suggested change
Recommend option 1 as documented in the diff above. Small, local, well-scoped.
Recommend the guard at the top of `createHandlePaste` as documented in the diff above. Small, local, well-scoped.

- `png-mixed-paste-probe.mjs` — Same shape with PNG; confirms bug is not JPEG-specific.
- `jpeg-drop-probe.mjs` — Drag-and-drop of JPEG; verifies `FileHandler.handleDrop` works.

These should move into `reports/jpeg-image-paste-support/evidence/` as part of the PR that lands the fix (or be deleted once the Playwright e2e test described above exists).
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor: Evidence location statement is outdated

Issue: This says the probes "should move into reports/jpeg-image-paste-support/evidence/" but they're already there in this PR.

Why: Creates confusion about the current state of the PR.

Fix:

Suggested change
These should move into `reports/jpeg-image-paste-support/evidence/` as part of the PR that lands the fix (or be deleted once the Playwright e2e test described above exists).
These are located in `evidence/` in this report directory (or can be deleted once the Playwright e2e test described above exists).

@github-actions github-actions Bot deleted a comment from inkeep-internal-ci Bot Apr 22, 2026
@github-actions
Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had recent activity.
It will be closed in 7 days if no further activity occurs.

If this PR is still relevant:

  • Rebase it on the latest main branch
  • Add a comment explaining its current status
  • Request a review if it's ready

Thank you for your contributions!

@github-actions github-actions Bot added the stale label Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant