scotttomaszewski_obsidian-d.../FOLLOWUP.md
Scott Tomaszewski 5813205586 Render ESV HTML via sanitizeHTMLToDom instead of innerHTML
Resolves FOLLOWUP.md #3. All four sites in BibleReferenceRenderer that
assigned the ESV API response to innerHTML now run it through Obsidian's
sanitizeHTMLToDom() and append the resulting DocumentFragment, dropping
the inline no-unsanitized/property and no-inner-html eslint-disable
comments. The verse-preview extraction queries/clones paragraphs off the
sanitized fragment instead of a temporary innerHTML div.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-31 11:47:41 -04:00

5.1 KiB
Raw Blame History

Follow-up work

This file tracks known issues that were intentionally deferred during the 0.13.0 standards/compliance pass. The codebase currently passes npx eslint . and npm run build with zero errors/warnings; the items below are silenced with scoped eslint-disable comments (each referencing this file) so the deferral is explicit rather than hidden.

Scope decision at the time: do the "safe subset" of standards fixes and leave the file-API internals and the event-handler internals functionally unchanged.


1. Hover-preview event listeners leak (highest priority) — RESOLVED

Files: src/core/BibleEventHandlers.ts, src/components/BibleReferenceRenderer.ts, src/components/BibleReferenceInlineExtension.ts, src/core/DisciplesJournalPlugin.ts

What was wrong: BibleEventHandlers was instantiated on every mouseover/mouseout, and each constructor added mousemove/click listeners to the global document that were only removed by a cleanup() that was never called (empty onunload), so listeners leaked for the app session. It also used bare document and setInterval, breaking pop-out windows.

Fix applied: BibleEventHandlers now extends Component and is a single, plugin-owned instance created in onload and registered with addChild(...). Its document listeners are attached via registerDomEvent (lazily per document, deduped through a WeakSet, so pop-out windows are tracked too) and the close-poll runs on a single registerInterval timer — all torn down automatically on unload. The hover callbacks reuse the shared instance instead of constructing new ones. The file-level eslint-disable banner (obsidianmd/prefer-active-doc, obsidianmd/prefer-window-timers, @typescript-eslint/no-deprecated) was removed.


2. File access via vault.adapter instead of the Vault/FileManager APIs — RESOLVED

Files: src/services/ESVApiService.ts, src/services/BibleFiles.ts, src/core/DisciplesJournalPlugin.ts

What was wrong (Obsidian guideline rules 1922):

  • vault.adapter.write(...) was used to create/overwrite notes (ESVApiService.saveESVApiResponseAsMdNote, DisciplesJournalPlugin.updateAllBibleNoteFrontmatter).
  • vault.adapter.exists / vault.adapter.mkdir were used for existence checks and directory creation.
  • BibleFiles.clearData used vault.adapter.rmdir(...), bypassing the user's trash settings.
  • Paths were built by string concatenation without normalizePath().

Fix applied:

  • ESVApiService.saveESVApiResponseAsMdNote now creates the note body with Vault.create() (reusing the existing TFile when present) and writes the API response + custom fields via FileManager.processFrontMatter(). Folder creation goes through Vault.getAbstractFileByPath() / Vault.createFolder().
  • DisciplesJournalPlugin.updateAllBibleNoteFrontmatter reads canonical from metadataCache and updates frontmatter via FileManager.processFrontMatter().
  • The manual YAML string builders in FrontmatterUtil.ts (buildFrontmatterString / mergeCustomFrontmatterIntoExisting) were replaced by a single applyCustomFrontmatter(fm, customYaml) that mutates the frontmatter object handed to processFrontMatter (dropping the stringifyYaml round-tripping).
  • BibleFiles.fileExistsForPassage uses getAbstractFileByPath (now synchronous), and BibleFiles.clearData deletes through FileManager.trashFile().
  • All passage/content paths run through normalizePath().

3. Rendering ESV HTML via innerHTML RESOLVED

File: src/components/BibleReferenceRenderer.ts (4 sites)

What was wrong: passage HTML returned by the ESV API was injected with element.innerHTML = passage.html. Flagged by no-unsanitized/property and @microsoft/sdl/no-inner-html.

Fix applied: all four sites now run the passage HTML through Obsidian's sanitizeHTMLToDom() and append the returned DocumentFragment, so no raw HTML string is written to the DOM. The verse-preview extraction (showVersePreview) parses into the sanitized fragment and queries/clones paragraphs off of it instead of a temporary innerHTML div. The inline eslint-disable comments at each site were removed.


4. setTimeout for scroll-to-verse

File: src/services/BibleChapterFiles.ts

The scroll-to-verse uses a fixed window.setTimeout(..., 300) to wait for the note to render before querying for the verse element. This was left as-is (only prefixed with window. for pop-out compatibility). A more robust approach would key off a render/layout event rather than a magic delay.


5. Smaller cleanups noticed in passing

  • src/services/BibleContentService.ts and src/services/ESVApiService.ts both contain // TODO notes about duplicated ESV-response → BiblePassage conversion logic that could be unified.
  • src/services/BibleChapterFiles.ts has a // TODO - logic in this class needs to move to BibleFiles note; BibleFiles.ts has TODOs for openChapterNote / openPassageNote helpers.
  • openChapterNote takes a string and re-parses it; several call sites already hold a BibleReference and could pass it directly (existing TODO).