Skip to content

fix(core): resolve links at boundary positions in editLink and deleteLink - #3081

Open
Ishkirat-Singh wants to merge 1 commit into
TypeCellOS:mainfrom
Ishkirat-Singh:fix/style-manager-link-boundaries
Open

fix(core): resolve links at boundary positions in editLink and deleteLink#3081
Ishkirat-Singh wants to merge 1 commit into
TypeCellOS:mainfrom
Ishkirat-Singh:fix/style-manager-link-boundaries

Conversation

@Ishkirat-Singh

@Ishkirat-Singh Ishkirat-Singh commented Sep 10, 2026

Copy link
Copy Markdown

Summary

Fixes #3073. StyleManager.editLink and deleteLink looked up the link mark at position + 1 unconditionally. At the end of the document that position does not exist and tr.doc.resolve throws a RangeError; at the end of a link (the usual place for the caret after clicking the link text) the lookup misses the link, falls back to the collapsed selection, and editLink inserts the new text next to the old link instead of replacing it.

Rationale

The link mark is not inclusive, so a resolved position at either boundary does not carry the mark. Probing one position inside the link on both sides, and only accepting a link whose range actually contains the caret, finds the link at its start, inside it and at its end, without changing behaviour for a caret that is not on a link at all.

Changes

  • packages/core/src/editor/managers/StyleManager.ts: new private getLinkMarkAround(position) that probes position + 1, position and position - 1, skips positions outside the document, and returns the first link whose [from, to] range contains position. editLink and deleteLink use it instead of getLinkMarkAtPos(position + 1).

Impact

No public API change. Callers that pass a position outside the document no longer crash, and callers with the caret at a link boundary now edit or delete that link, which is the documented intent of both methods.

Testing

New packages/core/src/editor/managers/StyleManager.test.ts (jsdom): edit and delete with the caret at the end of the link, edit with the caret at the start, and no throw for a position at the very end of the document. Ran it with vp test --run together with vp lint src.

Checklist

  • Code follows the project's coding standards.
  • Unit tests covering the new feature have been added.
  • All existing tests pass.
  • The documentation has been updated to reflect the new feature (no docs change needed).

Summary by CodeRabbit

  • Bug Fixes

    • Improved link editing and deletion at the start and end boundaries of linked text.
    • Prevented errors when link actions are triggered at the end of a document.
    • Ensured link deletion correctly converts linked text to plain text in boundary cases.
  • Tests

    • Added coverage for link editing and deletion at document and link boundaries.

…Link

Both methods looked the link up at `position + 1` unconditionally. With the
caret at the end of a link that lookup lands outside the mark, so editLink
inserted the new text next to the old one and deleteLink removed nothing;
with the position at the very end of the document it threw a RangeError
from doc.resolve.

Look inside the link first and then at either boundary, skipping positions
outside the document, and only accept a range that actually contains the
requested position.

Fixes TypeCellOS#3073
@vercel

vercel Bot commented Sep 10, 2026

Copy link
Copy Markdown

@Ishkirat-Singh is attempting to deploy a commit to the TypeCell Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Sep 10, 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: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: ceba5b25-5df1-4af9-8618-5dc8f0cd247d

📥 Commits

Reviewing files that changed from the base of the PR and between 325b071 and c4056f1.

📒 Files selected for processing (2)
  • packages/core/src/editor/managers/StyleManager.test.ts
  • packages/core/src/editor/managers/StyleManager.ts

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


📝 Walkthrough

Walkthrough

StyleManager now resolves links at caret boundaries without exceeding document bounds. New jsdom tests verify link editing, link deletion, and safe handling at the document end.

Changes

Link boundary handling

Layer / File(s) Summary
Boundary-aware link resolution
packages/core/src/editor/managers/StyleManager.ts
getLinkMarkAround checks adjacent in-bounds positions. editLink and deleteLink use the resolved link range.
Boundary behavior tests
packages/core/src/editor/managers/StyleManager.test.ts
Tests verify editing and deletion at link boundaries, plus calls at the document's final position.

Priority: ➖ Normal

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

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to c4056

Link editing and deletion now handle boundary and document-end cursor positions without the prior crashes or incorrect link handling. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 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 clearly and concisely describes the boundary-position fix in editLink and deleteLink.
Description check ✅ Passed The description covers the summary, rationale, changes, impact, testing, and checklist. The omitted screenshots and additional notes sections are non-critical for this code-only change.
Linked Issues check ✅ Passed The implementation addresses issue #3073 by preventing out-of-range lookups, resolving links at start and end boundaries, replacing link text correctly, and removing link marks. Tests cover the requir…
Out of Scope Changes check ✅ Passed The changes are limited to StyleManager link-boundary handling and focused regression tests. They align with the requirements in issue #3073.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

A rabbit found a boundary bright
And checked each link with careful sight
No range error leaped ahead
Old marks now yield as text is read
The editor hops safely tonight

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

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.

Off-By-One RangeError Crash and Text Duplication in StyleManager.editLink and deleteLink

1 participant