Skip to content

fix(desktop): keep markdown link and image attributes as strings so a paste can't stop the editor saving - #7678

Merged
saddlepaddle merged 3 commits into
mainfrom
sentry/desktop-1a6-link-title-string
Sep 19, 2026
Merged

saddlepaddle merged 3 commits into
mainfrom
sentry/desktop-1a6-link-title-string

Conversation

@saddlepaddle

@saddlepaddle saddlepaddle commented Sep 19, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Pasting rich text into a markdown editor could put it into a state where it silently stops saving. The pasted content appears on screen, but the editor's onChange never fires again and onBlur no longer saves — every later serialization of that document throws.

Reported as DESKTOP-1A6 (desktop 1.29.0, renderer, unhandled, on a paste event).

Root cause

@tiptap/core's default attribute parser runs element.getAttribute(name) through fromString, which coerces numeric- and boolean-looking strings:

function fromString(value) {
	if (typeof value !== "string") return value;
	if (value.match(/^[+-]?(?:\d*\.)?\d+$/)) return Number(value);
	if (value === "true") return true;
	if (value === "false") return false;
	return value;
}

Any attribute that does not declare its own parseHTML gets that treatment. @tiptap/extension-link declares title: { default: null } with no parseHTML (its href has one, which is why only title was exposed), so an anchor carrying title="2024" loads a link mark whose title is the number 2024. prosemirror-markdown then does:

mark.attrs.title ? ` "${mark.attrs.title.replace(/"/g, '\\"')}"` : ""

Truthy, not a string — TypeError. Both markdown surfaces serialize on every update (onUpdate → getMarkdown), so once such a link is in the document the throw repeats on every keystroke and nothing downstream ever receives the content again.

This is the same coercion #6663 fixed for image src/alt/title. That fix landed on SafeImage in the renderer and never reached MarkdownEditor, because the two surfaces keep separate copies of these extensions. So the editor had two live instances of the same crash class: the link title reported here, and image alt/title — LinearImage overrode only src, and only incidentally, to rewrite Linear-hosted URLs through the proxy.

Fix

Declare the verbatim parsing once, in renderer/lib/tiptap/markdown-attributes.ts, and use it from every extension that feeds the markdown serializer:

  • verbatimStringAttributes(...names) — used by SafeImage (src/alt/title) and LinearImage (alt/title, composing with its own proxying src).
  • SafeLink — one shared mark, imported by both extension lists, each still applying its own .configure().

renderer/lib/tiptap/ already exists and is already imported by both surfaces, so this adds no new coupling direction. Both files come out net smaller, and the duplication that let the earlier fix drift is gone.

No behaviour change for attributes that were already strings, and the Linear image proxy rewrite is untouched.

Verification

Reproduced first — the new tests fail on main with the exact production error:

484 |    : "](" + mark.attrs.href.replace(...) + (mark.attrs.title ? ` "${mark.attrs.title.replace(/"/g, '\"')}"` : "") + ")";
                                                                                  ^
TypeError: mark.attrs.title.replace is not a function.

(fail) link attribute parsing > keeps a numeric link title as a string and serializes it
(fail) link attribute parsing > keeps a boolean-looking link title as a string

 9 pass
 2 fail

After the fix:

$ cd apps/desktop && bun test src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/
 66 pass
 0 fail
 151 expect() calls
Ran 66 tests across 5 files. [922.00ms]
$ bunx biome check <the four changed files>
Checked 4 files in 8ms. No fixes applied.
$ cd apps/desktop && bun run typecheck
$ tsc --noEmit      (clean)

The round-trip tests cover SafeLink and the helper — the code both surfaces now share, including the exact production path. LinearImage stays module-local inside MarkdownEditor.tsx and is covered by typecheck rather than by its own test; its extension list is built inline in the component, which cannot be mounted without the authenticated route provider tree.

No user-facing strings changed, so check:i18n does not apply.

Refs DESKTOP-1A6
Refs DESKTOP-100

Summary by CodeRabbit

  • Bug Fixes

    • Fixed Markdown links and images so text-based attributes, including titles and alternative text such as “2024” or “true,” retain their original string values.
    • Improved Markdown round-tripping to preserve link titles and title-less links without unintended conversions.
  • Tests

    • Added coverage for preserving link attributes and maintaining unchanged Markdown through editing and rendering.

…'t kill markdown serialization

@tiptap/core's default attribute parser runs getAttribute through
fromString, which coerces numeric/boolean-looking strings, and
@tiptap/extension-link declares title with no parseHTML of its own. A
pasted anchor carrying title="2024" therefore loads a link mark with
title: 2024 (number), and prosemirror-markdown's link serializer does
mark.attrs.title.replace(...) — a TypeError on every serialization of
that document from then on.

Both markdown surfaces serialize on every update, so the editor stops
propagating onChange and onBlur stops saving: the pasted text is on
screen but nothing downstream ever sees it.

This is the same coercion #6663 fixed for image src/alt/title; the link
mark was missed. Give the link mark the same verbatim parseHTML in both
places that register it, and pin the round-trip as tests.

Refs DESKTOP-1A6
@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 06d32ffa-0e53-49dd-b1c0-a26d87b76479

📥 Commits

Reviewing files that changed from the base of the PR and between 8d10ce6 and 359aefc.

📒 Files selected for processing (4)
  • apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx
  • apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/createMarkdownExtensions.test.ts
  • apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/createMarkdownExtensions.ts
  • apps/desktop/src/renderer/lib/tiptap/markdown-attributes.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The change adds shared verbatim attribute parsing for Tiptap links and images. The editor and renderer use SafeLink. Tests cover numeric, boolean-looking, and missing link titles.

Changes

Safe link title handling

Layer / File(s) Summary
SafeLink implementation
apps/desktop/src/renderer/lib/tiptap/markdown-attributes.ts
Adds verbatimStringAttributes and SafeLink. Link titles are parsed from HTML without type coercion.
Editor and renderer wiring
apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx, apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/createMarkdownExtensions.ts
Registers SafeLink and applies shared verbatim parsing to image attributes.
Link title round-trip tests
apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/createMarkdownExtensions.test.ts
Tests numeric, boolean-looking, and missing link titles during markdown round trips.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: kitenite

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title uses conventional commit format and clearly describes the primary fix: preserving markdown link and image attributes as strings to prevent editor saving failures.
Description check ✅ Passed The description clearly explains the problem, root cause, fix, affected components, and verification results. It does not include the template's explicit Checklist section, but the missing section is …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…the image gap

Copying these extensions between the two markdown surfaces is what let
#6663's image fix miss MarkdownEditor in the first place, so the second
copy of SafeLink is a liability rather than a smaller diff. Move the
link mark and the attribute helper to renderer/lib/tiptap, where both
surfaces already share code, and use the helper from all three
extensions.

That also closes the gap the duplication left: LinearImage overrode only
src (for the proxy rewrite), so a pasted image with a numeric alt or
title still threw the same TypeError out of the same paste handler in
the same component the link crash came from.
…output

The serialization assertion covers the coercion transitively; asserting
attrs.title pins the actual defect the way the image tests above do, and
justifies the raw editor form over the roundTrip helper.
@saddlepaddle saddlepaddle changed the title fix(desktop): keep link titles as strings so a pasted titled link can't kill markdown serialization fix(desktop): keep markdown link and image attributes as strings so a paste can't stop the editor saving Sep 19, 2026
@saddlepaddle
saddlepaddle merged commit e60856b into main Sep 19, 2026
13 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch

Thank you for your contribution! 🎉

Kitenite added a commit that referenced this pull request Sep 23, 2026
* docs: update weekly product documentation for September 23

Source of truth: live Notion SOP https://www.notion.so/3b3b9d5bf616810dbacaca7dd2305259
Window: GitHub merged:>=2026-09-16, through this run on 2026-09-23. gh returned 103 PRs (limit 1000, avoiding the SOP example’s 50-PR truncation).
Workspace baseline: 2158895. Prior session stopped at a usage limit; the checkout was clean.

## Changes

- Pages, CLI, and MCP: public sharing, workspace menus, origin labels, pagination, first-publish opening, version previews, mobile comments, and link handling: #7570, #7658, #7726, #7472, #7532, #7718. Files: pages.mdx; cli/cli-reference.mdx; mcp-server.mdx.
- iPhone launch, requirements, setup, host states, and Pro checklist: #7703, #7686, #7697, #7661, #7642, #7494. Files: install.mdx; faq.mdx; overview.mdx; remote-access.mdx.
- Editor save conflicts and Markdown link actions: #7711, #7621. Files: editor.mdx.
- Terminal locations, copying, and ended-session recovery: #7712, #7709, #7618. Files: terminal-integration.mdx.
- Model catalog, embedded sign-in, and leaderboard history: #7741, #7740, #7651. Files: agent-integration.mdx; usage.mdx; cli/cli-reference.mdx.
- Repository picker, cloud working branches and attachments, mobile sessions, sidebar naming and ordering: #7694, #7620, #7622, #7633, #7604, #7650, #7575. Files: first-workspace.mdx; workspaces.mdx.
- Slack plugin tools and gated thread follow-ups, queuing, mute and stop controls: #7583, #7585, #7586, #7588, #7592, #7594. Files: use-with-slack.mdx.
- New CLI transcript, environment, and page deletion commands; connected-service plugins and MCP CLI migration: #7602, #7317. Files: cli/cli-reference.mdx; skills.mdx.
- Startup recovery and pre-account workspace migration: #7764, #7649. Files: troubleshooting.mdx.

Current source wins over PR descriptions: page ordering is newest-created first, search includes description, default CLI output remains a bare array, and the leaderboard automatically backfills since launch (there is no history-range picker). Slack thread commands and Mobile settings retain their availability qualifications. No assumption that every merged fix is already in the latest download.

## Deliberately skipped

- [#7765](#7765) fix(host-service): report GitHub being unreachable as unavailable, not a 500: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7756](#7756) fix(pages): restore page.list's bare-array contract for released clients: Restores compatibility for released clients; checked the current listPaginated implementation when documenting #7726.
- [#7749](#7749) fix(build): build every dist-types a filtered mobile typecheck needs: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7748](#7748) feat(mobile): refresh the App Store listing and load review creds from .env: Marketing/store copy, layout, or launch assets; no separate product-doc change. The released iPhone workflow is covered by #7686 and #7703.
- [#7747](#7747) feat(mobile): ask for a rating on returning Home after real use: App Store rating timing needs no setup or reference instructions.
- [#7746](#7746) feat(mobile): start Sentry natively so launch crashes carry native context: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7744](#7744) fix(email): make the welcome email unsubscribable via a Resend automation: Welcome email unsubscribe delivery is not a product workflow needing docs.
- [#7734](#7734) fix(desktop): restore locale provider test mocks: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7733](#7733) fix(marketing): hide leaderboard vertical guide lines: Marketing/store copy, layout, or launch assets; no separate product-doc change. The released iPhone workflow is covered by #7686 and #7703.
- [#7732](#7732) chore(desktop): bump version to 1.30.2: Version bump only.
- [#7723](#7723) fix(desktop): preserve sidebar order and missing group members: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7717](#7717) fix(desktop): keep terminal links within URL boundaries: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7715](#7715) fix(ci): unblock CLI publishing and retry macOS installs: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7714](#7714) fix(desktop): prevent tab close clicks from starting a drag: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7710](#7710) feat(marketing): add Vladimir mobile testimonial and rank quotes: Marketing/store copy, layout, or launch assets; no separate product-doc change. The released iPhone workflow is covered by #7686 and #7703.
- [#7704](#7704) feat(marketing): add App Store icon to footer: Marketing/store copy, layout, or launch assets; no separate product-doc change. The released iPhone workflow is covered by #7686 and #7703.
- [#7701](#7701) chore(desktop): bump version to 1.30.1: Version bump only.
- [#7700](#7700) fix(marketing): refine hero copy and demo details: Marketing/store copy, layout, or launch assets; no separate product-doc change. The released iPhone workflow is covered by #7686 and #7703.
- [#7698](#7698) feat(marketing): replace mobile feature cards with customer testimonials: Marketing/store copy, layout, or launch assets; no separate product-doc change. The released iPhone workflow is covered by #7686 and #7703.
- [#7696](#7696) fix: write managed configs and workspace files through symlinks: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7695](#7695) fix(marketing): isolate and scale the mobile phone demo: Marketing/store copy, layout, or launch assets; no separate product-doc change. The released iPhone workflow is covered by #7686 and #7703.
- [#7691](#7691) fix(terminal): preserve agent paste mode across host restarts: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7690](#7690) feat(marketing): showcase mobile workflows and simplify the homepage: Marketing/store copy, layout, or launch assets; no separate product-doc change. The released iPhone workflow is covered by #7686 and #7703.
- [#7689](#7689) chore(mobile): bump version to 1.1.1 for TestFlight: Version bump only.
- [#7688](#7688) fix(desktop): resolve dev Dock names across workspace path aliases: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7685](#7685) docs(marketing): add September 20 weekly changelog: Marketing/store copy, layout, or launch assets; no separate product-doc change. The released iPhone workflow is covered by #7686 and #7703.
- [#7684](#7684) docs(skills): share mobile demo recording and film tooling: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7683](#7683) fix(ui): truncate the page title instead of running it under the header actions: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7680](#7680) fix(host-service): treat a socket torn down mid-request as the cloud being unreachable: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7678](#7678) fix(desktop): keep markdown link and image attributes as strings so a paste can't stop the editor saving: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7676](#7676) docs(db): migration safety checklist and deploy workflow pitfalls from the 17 September outage: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7675](#7675) fix(integrations): backfill existing connections into the table #7317 reads: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7672](#7672) fix(ci): a failed production migration can no longer pass as a success: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7671](#7671) fix(db,ci): make migration 0119 online-safe and bound production migrations: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7670](#7670) fix(desktop): send text to running terminals as a bracketed paste, not raw keystrokes: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7669](#7669) feat(skills): audit page usage in the 10x skill: Existing skills page already describes the 10x audit; page-usage audit is an internal skill refinement.
- [#7668](#7668) fix(mobile): scroll-to-bottom button drives Claude Code's fullscreen view: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7666](#7666) fix(host-service): treat a gh that cannot start as no token: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7665](#7665) chore(desktop): bump version to 1.30.0: Version bump only.
- [#7664](#7664) refactor(marketing): share the store badges with the mobile launch post: Marketing/store copy, layout, or launch assets; no separate product-doc change. The released iPhone workflow is covered by #7686 and #7703.
- [#7662](#7662) chore(mobile): simulator verification skill, stand-in host, and host-state flows: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7659](#7659) fix(host-service): stop the Linux file watcher from taking the host down, and make superset start recover: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7655](#7655) feat(marketing): add /mobile landing page behind the mobile-launch flag: Marketing/store copy, layout, or launch assets; no separate product-doc change. The released iPhone workflow is covered by #7686 and #7703.
- [#7647](#7647) fix(agent-setup): restore Codex working status: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7644](#7644) fix(desktop): refresh commits after external changes: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7643](#7643) fix(mobile): a long press lifts the whole session tab, not its text: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7641](#7641) fix(desktop): harden renderer error boundaries and recovery: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7639](#7639) fix(desktop): keep the editor selection valid when external content replaces the document: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7638](#7638) fix(desktop): stop the file tree throwing when a lookup walks through a file: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7636](#7636) fix(host-service): classify an index its storage cannot deliver as environmental: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7615](#7615) fix(cli-framework,i18n): `bun run dev` runs the CLI instead of dying: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7613](#7613) fix(host-service): a terminal in a cloud workspace knows it is in one: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7611](#7611) feat(marketing): Superset Mobile launch post (hidden until launch): Marketing/store copy, layout, or launch assets; no separate product-doc change. The released iPhone workflow is covered by #7686 and #7703.
- [#7608](#7608) fix(sandbox): the release probe checks the display from inside the box: Internal build, tests, migration, developer tooling, or recovery plumbing; no new user-facing instructions.
- [#7606](#7606) fix(relay): tolerate slow connections with coordinated deadlines: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7605](#7605) fix(desktop): drop duplicate create-workspace submits while one is in flight: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7603](#7603) fix(desktop): preserve translation context for route errors: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7601](#7601) feat(mobile): let the workspace list scroll under a transparent header: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7598](#7598) fix(desktop): keep viewport emulation on a crashed browser pane from crashing the app: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7597](#7597) fix(host-service): pin git's diff prefixes so the Changes pane can parse every patch: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7593](#7593) fix(api): post the Slack agent's reply after its run budget is spent: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7587](#7587) fix(desktop): keep new sidebar group orders on the integer lane: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7582](#7582) fix(api): bound every Slack agent call by the run budget: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7581](#7581) feat(api): Slack agent foundation — shared handler, execution guard, thread window, model and output fixes: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.
- [#7573](#7573) feat(pages): publish without a workspace: Standalone publishing is already documented in pages.mdx; preserved that guidance and included its contract in the new CLI/MCP reference.
- [#7255](#7255) fix(host-service): preserve consumed Claude prompts in handoffs: Reliability or visual fix restores expected behavior; no new workflow or necessary workaround to document.

## Validation

- bun install: passed; no tracked dependency changes.
- bun run lint:fix: passed, no code fixes.
- bun run lint: passed.
- bun run --cwd apps/docs typecheck: passed.
- Vale on all 16 changed docs: zero errors and warnings. Small pre-existing prose violations in touched pages were corrected to satisfy the SOP.
- git diff --check: passed.

Only apps/docs/content/docs/ and the required append-only root TODO log are included. No push or PR.

37 PRs drove changes; 66 were deliberately skipped as separate documentation work.

* docs: correct stale claims and consolidate weekly updates

Replace outdated remote-port and privacy claims, correct the 90-day leaderboard backfill cap and link settings location, and describe credential use and comment-watch registration accurately. Consolidate iPhone requirements and troubleshooting, trim repeated Pages prose, and keep launch effort details in the CLI reference.

Remove the #7764 startup entry because it does not identify a fixed release and its upgrade instructions are too vague. This supersedes that item in the original PR audit. Validate with docs typecheck, lint:fix, lint, Vale (zero errors or warnings), and git diff --check.

* docs: trim repeated explanations across user guides

* docs: clarify iPhone cloud workspace requirements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant