Skip to content

fix: append link respects caret when focused in a frontmatter property (#768)#1340

Draft
Ivikhostrup wants to merge 3 commits into
chhoumann:masterfrom
Ivikhostrup:fix/768-append-link-in-property-fields
Draft

fix: append link respects caret when focused in a frontmatter property (#768)#1340
Ivikhostrup wants to merge 3 commits into
chhoumann:masterfrom
Ivikhostrup:fix/768-append-link-in-property-fields

Conversation

@Ivikhostrup

@Ivikhostrup Ivikhostrup commented Jun 13, 2026

Copy link
Copy Markdown

Problem

Fixes #768. When Append link to current file runs while the caret is in a
frontmatter property field (Obsidian's Properties widget), the link is
inserted at the first body line (right after the YAML) instead of at the
cursor. It works fine when the caret is in the document body.

Steps to reproduce (on master)

Manual, in a live vault:

  1. Settings → QuickAdd: create (or edit) a Template or Capture choice and enable
    Append link to current file.
  2. Open any note that has frontmatter, so the Properties panel renders in
    Live Preview, e.g.:
    ---
    title: My Note
    ---
    some existing text
  3. Click into a property value field (e.g. title) so the caret is inside
    the Properties widget — not the note body.
  4. Run the QuickAdd choice.

Expected: the link is appended at the caret (in the property).
Actual: the link is inserted on the first body line, right after the ---:

---
title: My Note
---
[[Created Note]]      <-- inserted here instead of at the caret
some existing text

Automated (no Obsidian needed) — the new test doubles as a repro:

pnpm install
pnpm exec vitest run src/utilityObsidian.appendLink-768.test.ts

On master the insertLinkWithPlacement (#768 property-field routing) case
fails (the body editor's replaceSelection is called instead of the focused
property); with this PR all 7 cases pass.

Root cause

The whole append-link path reads the cursor from the markdown body editor:

insertFileLinkToActiveView (src/utilityObsidian.ts) →
insertLinkWithPlacement, which uses editor.listSelections() /
editor.getCursor().

In Live Preview the Properties panel is a separate set of input elements, not
the CodeMirror document. When a property is focused, the body editor isn't, so
getCursor() returns a stale body caret (the first line after the frontmatter).
There was no check for this case, so the link landed in the body.

Fix

insertLinkWithPlacement now detects when focus is inside the Properties widget
(the focused DOM element under .metadata-property /
.metadata-properties-container) and inserts the link at that element's
caret
, dispatching an input event so Obsidian persists it to the
frontmatter. It falls back to the existing body-editor behavior when no property
field is focused or the element type is unsupported. Placement modes
(replaceSelection, etc.) only describe body insertion, so they don't apply in
the property case.

Two small helpers (getFocusedEditablePropertyEl, insertTextIntoEditableEl)
are added and exported via __test for unit coverage.

Validation

  • pnpm exec vitest run src/utilityObsidian.appendLink-768.test.ts7 passed
    (focus detection, editable-element insertion, and property-vs-body routing).
  • Regression check on related suites
    (utilityObsidian, linkPlacement, engine/, formatter-linkcurrent) →
    345 passed, 2 skipped, no regressions.
  • eslint on the changed files → clean; tsc -noEmit -skipLibCheck → no src/ errors.
  • Also reproduced/confirmed at runtime by driving the real
    insertFileLinkToActiveView over a frontmatter document with a jsdom
    Properties widget: pre-fix the link lands at the first body line; post-fix it
    lands in the focused property and the body editor is left untouched.

Note: automated tests + a jsdom-level runtime repro are green, but I have not
yet clicked through this in a live Obsidian vault. A manual check (the steps
above) would be good before merge.

Notes for reviewers

  • main.js / styles.css are gitignored in this repo, so only source is included.
  • The contenteditable branch of insertTextIntoEditableEl covers Obsidian's
    rich text/list property inputs; the <input>/<textarea> branch covers the
    simpler ones.

Summary by CodeRabbit

  • Bug Fixes

    • Link insertion now properly supports placement directly within Obsidian's frontmatter Properties widget when the cursor is positioned in a property field.
  • Tests

    • Added regression tests validating link insertion behavior within property widgets, text caret positioning in editable elements, and fallback behavior when no property field is focused.

… fields

When "Append link to current file" runs while the caret is inside a
frontmatter property (Obsidian's Properties widget), the markdown body
editor is not focused, so editor.getCursor()/listSelections() report a
stale body caret. The link was inserted at the first body line instead
of at the user's cursor.

insertLinkWithPlacement now detects when focus is in a property field
(via the focused DOM element inside .metadata-property /
.metadata-properties-container) and inserts the link at that element's
caret, firing an input event so Obsidian persists it to the frontmatter.
It falls back to the existing body-editor behavior when no property field
is focused or the element type is unsupported.

Adds src/utilityObsidian.appendLink-768.test.ts covering the focus
detection helper, the editable-element insertion helper, and the
property-vs-body routing in insertLinkWithPlacement.

Co-Authored-By: Claude <noreply@anthropic.com>
@Ivikhostrup Ivikhostrup requested a review from chhoumann as a code owner June 13, 2026 22:53
@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8692c773-9786-4f7e-a2f9-d8ab5f92ac3a

📥 Commits

Reviewing files that changed from the base of the PR and between c2ee449 and dda12e5.

📒 Files selected for processing (2)
  • src/utilityObsidian.appendLink-768.test.ts
  • src/utilityObsidian.ts

📝 Walkthrough

Walkthrough

insertLinkWithPlacement now inserts at the caret inside a focused Obsidian Properties field before falling back to body-editor placement. The change adds DOM insertion helpers, exposes them through __test, and adds regression tests for focused-property, caret-editable, and fallback behavior.

Changes

Properties Widget Link Insertion

Layer / File(s) Summary
DOM helpers and insertLinkWithPlacement short-circuit
src/utilityObsidian.ts
Adds focused Properties-field detection and caret insertion helpers, updates insertLinkWithPlacement to use that path first, and exports the helpers through __test.
Regression tests for property insertion
src/utilityObsidian.appendLink-768.test.ts
Adds test scaffolding and regression coverage for focused property insertion, editable caret updates with input events, and fallback to markdown body insertion when no property field is focused.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I tucked a link in a property row,
Right where the little caret chose to go.
If fields aren’t focused, back body-side we hop,
With tests on watch so regressions stop.
Nibble, patch, and bounce away!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main fix: appending links respects the caret in frontmatter properties for issue #768.
Linked Issues check ✅ Passed The changes implement the exact #768 behavior by inserting at the focused property caret and falling back to body insertion.
Out of Scope Changes check ✅ Passed The PR stays focused on the #768 fix and its regression tests, with no unrelated code changes apparent.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


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 and usage tips.

@Ivikhostrup Ivikhostrup changed the title Fix #768: append link respects caret in frontmatter property fields fix: append link respects caret when focused in a frontmatter property (#768) Jun 13, 2026
Funnel the focus-detection + insertion behind a single intention-revealing
tryInsertIntoFocusedProperty(text) call, name the Properties-widget selector
and the editable-element check, and use descriptive variable names.

@chhoumann chhoumann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for tackling this, @Ivikhostrup — the diagnosis in #768 is exactly right (a focused property is a separate input, not the CodeMirror caret), and the naming refactor in dda12e5b reads well. I want to call out what's solid before the problems: the body-insertion path is left completely untouched (purely additive, multi-cursor + every placement mode byte-for-byte identical) so there's zero regression to the existing feature, the early-return is positioned correctly, and there's no injection vector (text nodes / element.value, never innerHTML).

I took this into a live Obsidian vault — the part you flagged as not-yet-verified — and that's unfortunately where the property helper runs into trouble. Requesting changes; details are inline, but the headlines:

🔴 Blockers (reproduced live — data loss)

  • Number / Date / Datetime properties get wiped. They render as native <input type="number"> / type="date". Slicing [[Note]] into them makes the browser reject the value, so element.value becomes "", insertTextAtCaret returns true (suppressing the body fallback), and Obsidian persists the emptied value on blur. Reproduced: count: 5 → count: (empty), date: 2026-06-14 → (empty).
  • Caret in the property KEY field corrupts the key name (count[[Note]]: 5) — .metadata-property-key-input matches the same .metadata-property selector as the value.

🟠 Majors

  • Default text properties are contenteditable .metadata-input-longtext, not <input>. So the branch the tests cover (<input type="text">) isn't the one real text properties use, and the contenteditable branch is untested. The good news: that branch does persist for longtext — but only on blur, and the helper never blurs (so it can sit uncommitted / be discarded on re-render).
  • List / tags (.multi-select-input) silently swallow the text — they commit pills on Enter, not on a synthetic input event — so it's a no-op that still returns true and discards the link.
  • Focus timing: append-link runs after the choice. Any choice with a prompt/suggester/modal (or launched from the command palette) moves focus off the property before insertion → it falls back to the stale body caret → the original #768. The focused target isn't captured before the UI opens. Worth verifying with the built PR across capture-with-prompt / template-with-VALUE-suggester / command-palette / split panes.
  • textContent fallback flattens an already-linked property's child nodes and drops the existing link.

Suggested direction

The brittle part is using Obsidian's private Properties DOM to both detect the caret and persist the value. Consider (1) capturing the focused property target before the choice opens any UI, and (2) persisting via app.fileManager.processFrontMatter(...) keyed to the resolved property + the target TFile, using the DOM only to read which property/caret. That sidesteps the typed-input wipe, the blur dependency, pop-out windows, and list/tags in one move.

At minimum the two blockers need fixing, plus a live-vault e2e (text, longtext, number, date, list/tags, key field) before merge. Happy to review a follow-up — the core idea is right and worth getting in. 🙏

(Reviewed at head dda12e5b. Findings verified by driving a real Obsidian dev vault, not just jsdom.)

Comment thread src/utilityObsidian.ts
const endOffset = element.selectionEnd ?? element.value.length;
const before = element.value.slice(0, startOffset);
const after = element.value.slice(endOffset);
element.value = before + text + after;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Blocker (data loss): this <input> branch wipes existing data for non-text inputs. Obsidian renders Number/Date/Datetime properties as native typed inputs (<input type="number">, type="date", type="datetime-local"). element.value = before + text + after makes the browser reject the non-numeric string, so element.value becomes "" and Obsidian persists null on blur — the original value is destroyed (reproduced live: count: 5 → count: empty, date: 2026-06-14 → empty). Because insertTextAtCaret then returns true, the body fallback is suppressed too. Please restrict this branch to text-like inputs (e.g. only when element.type is text/search/empty) and return false otherwise so it falls back to the body editor.

Comment thread src/utilityObsidian.ts
if (typeof document === "undefined") return false;
const focused = document.activeElement;
if (!(focused instanceof HTMLElement)) return false;
if (!focused.closest(PROPERTY_WIDGET_SELECTOR)) return false;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Blocker (key corruption): a .metadata-property row contains both .metadata-property-key-input (the property NAME) and the value editor, and both are editable elements inside .metadata-property. If the caret is in the key field (e.g. mid-naming a new property), the link is injected into the property name and persisted as a corrupted key on blur (count[[Note]]: 5). Please scope detection to the value side — require focused.closest(".metadata-property-value") and/or bail when focused.closest(".metadata-property-key").

Minor on PROPERTY_WIDGET_SELECTOR (line 563): the real container class is .metadata-properties (not .metadata-properties-container), so that half of the selector never matches — detection currently survives only via .metadata-property.

Comment thread src/utilityObsidian.ts
return true;
}

if (element.isContentEditable) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Two real cases land in this contenteditable branch:

  1. Default text properties render as a contenteditable .metadata-input-longtext div (not the <input> the tests mount), so this is the real text path. It does persist to frontmatter — but only on blur, and insertTextAtCaret never blurs, so the change can sit uncommitted and be discarded if Obsidian re-renders the widget first.
  2. List / tags render as .multi-select-input (also contenteditable) and commit pills on Enter, not on a synthetic input event. So this inserts the text, fires input, returns true (suppressing the body fallback), but the text never reaches frontmatter and is discarded on blur (reproduced live: tags stayed [a, b]).

Please detect .multi-select-container/.multi-select-input and return false (so the body fallback runs) rather than reporting false success; and consider explicitly committing (blur) after a longtext insert.

Comment thread src/utilityObsidian.ts
selection.removeAllRanges();
selection.addRange(range);
} else {
element.textContent = `${element.textContent ?? ""}${text}`;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Setting element.textContent = existing + text on a contenteditable removes ALL child nodes (replacing them with one flat text node) and appends at the end, ignoring the caret. For an already-linked longtext property (rendered as .metadata-link child nodes whose textContent is only the display text) this silently drops the original link. This branch is reachable when focus is restored after a modal closes without an in-element selection. Please return false here so the caller falls back to the body editor instead of doing a destructive append.

Comment thread src/utilityObsidian.ts
*/
function tryInsertIntoFocusedProperty(text: string): boolean {
if (typeof document === "undefined") return false;
const focused = document.activeElement;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Use the popout-aware document, not the global one. This codebase's convention is getOwnerDocument/activeDocument (see activeWindow.ts, suggest.ts, fileSuggester.ts). With global document.activeElement, a property focused in a popped-out window is never detected and #768 silently falls back to body insertion in popouts. The active view is in scope at the insertLinkWithPlacement call site, so you can thread view.containerEl.ownerDocument.activeElement in.

Also cross-realm: a popout input is not instanceof the main realm's HTMLInputElement (see isEditableElement, lines 566-572), so even after fixing the document it would be rejected — prefer duck-typing (element.matches("input,textarea") / "selectionStart" in element).

Comment thread src/utilityObsidian.ts
// #768: a focused frontmatter property field leaves the body editor's caret
// stale; insert into the property at its caret instead. Placement modes only
// apply to body insertion.
if (tryInsertIntoFocusedProperty(text)) return;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Two concerns at this routing point:

  1. Focus timing: insertFileLinkToActiveView runs after the Template/Capture choice. Any flow that opens a prompt/suggester/modal — or is launched from the command palette — can move focus off the property before this runs, so tryInsertIntoFocusedProperty returns false and we fall back to the stale body caret (the original [BUG] Append link to current file in properties #768). Capturing the focused property target before QuickAdd opens UI (and threading it through) would make this robust.
  2. Wrong-file write: there's no check that the focused property belongs to the active view. With split panes / multiple MarkdownViews, document.activeElement could be a property in a different leaf than getActiveViewOfType(MarkdownView) returns, so the link is written into the wrong note's frontmatter. Consider asserting view.containerEl.contains(focused) before inserting.

Both need a live-vault check with the built PR (capture-with-prompt, template-with-VALUE-suggester, command-palette, split panes).

}

/** Mounts an Obsidian-like Properties widget with one focused text property. */
function mountFocusedTextProperty(initialValue = ""): HTMLInputElement {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This suite only mounts <input type="text">, which is the one shape that trivially persists and isn't how a default text property renders (default text = contenteditable .metadata-input-longtext). So the contenteditable Range branch, the textContent fallback, the typed-input wipe (number/date), and the key-field case are all untested, and the 7 green tests give false confidence for the most common real property shapes. Note: jsdom doesn't implement isContentEditable, so a contenteditable test must force it (e.g. Object.defineProperty(el, "isContentEditable", { value: true })). Also, the input-event assertion only proves a locally-attached listener fired — it can't prove Obsidian persists — so it's worth a real dev-vault e2e gating merge rather than relying on these unit tests.

@Ivikhostrup Ivikhostrup marked this pull request as draft June 13, 2026 23:30
@Ivikhostrup

Copy link
Copy Markdown
Author

Marking this draft — the fix is verified non-functional in real Obsidian. Posting the verification so the approach can be corrected rather than silently shipped.

Verified against a live Obsidian 1.9.14 instance, inspected over its remote-debugging (CDP) port in an isolated throwaway vault.

Mechanism — confirmed ✅

With the caret in a frontmatter property, editor.getCursor() returns {line: 4} — the first body line after the YAML. The body editor's reported caret is stale, and insertLinkWithPlacement faithfully inserts there. That part of the diagnosis holds.

Why this PR's remedy does not work ❌

The real property value editor is a div.metadata-input-longtext (contenteditable), managed by Obsidian's MetadataEditor. Mutating it from outside its own input pipeline does not persist to the file:

Insertion approach Persists to file?
DOM text-node insert + new Event('input') (this PR) ❌ no change
document.execCommand('insertText', …) ❌ no change
app.fileManager.processFrontMatter(file, fm => …) ✅ persists

insertTextAtCaret runs, but Obsidian re-renders the property from its model and discards the DOM change — the link is never written. The unit test passed only because it drove a plain <input> (which Obsidian doesn't use here) with the same assumption baked into the fixture, so it never exercised the real path.

Correct direction

  • .metadata-property is a valid selector (confirmed via closest()); .metadata-properties-container does not exist in the DOM and should be dropped.
  • The focused property row exposes data-property-key (and .metadata-property-key-input), so the target property is identifiable.
  • Persist via processFrontMatter on that key instead of touching the DOM.

Open design questions before reworking

  1. processFrontMatter appends to the property value; it can't insert at the in-value caret. "Append to the focused property" is the persistable reading of "at the cursor position."
  2. String vs. list property handling differ (append to string vs. push a list item).
  3. processFrontMatter re-serializes the YAML (e.g. trims tags: tags:).

Happy to rework around processFrontMatter if that direction looks right.

AI-assisted verification (Claude). DOM facts gathered via CDP inspection of a live vault, not inferred.

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.

[BUG] Append link to current file in properties

3 participants