diff --git a/AGENTS.md b/AGENTS.md index ddc447d8..d81feeaa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -345,6 +345,35 @@ The TODO.md should be: - **Global `app` variable**: In Obsidian plugins, `app` is a globally available variable that provides access to the Obsidian API. It's automatically available in all files without needing to import or declare it. +### Picking the right `document` / `window` (popout-window safety) + +Obsidian supports pop-out windows. The plugin loads in the main window but views can live in any window. Picking the wrong `Document` / `Window` produces stale references, off-screen popovers, listeners on the wrong window, or DOM nodes that never render. Use this decision order: + +1. **`element.doc` / `element.win`** — preferred. Obsidian augments every `Node` with `.doc: Document` and `.win: Window` that always reflect the element's current owner. Use whenever you have any DOM node in scope (a ref, an event target, a component's container, a `Range`'s `startContainer`). + - `containerRef.current?.doc.addEventListener(...)` + - `range.startContainer.win.innerWidth` + - `editor.getRootElement()?.doc` +2. **`global activeDocument` / `activeWindow`** — fallback only. These point to whichever window is _focused right now_. Correct semantics for actions that follow user focus (e.g., the AddImageModal file picker; selectionchange registration at plugin load), but wrong when the action belongs to a specific view (a chat in a popout while the user clicks back to the main window). +3. **`document` / `window` globals** — almost always wrong. They are aliases for the main window even when the user is interacting with a popout. Avoid in new code. If you find yourself reaching for them, it's a sign the surrounding code should be taking a `Document`/`Window` parameter or deriving from a DOM ref. +4. **`element.ownerDocument`** — works (standard DOM), but prefer `.doc` for consistency with the codebase. They return the same `Document` for any mounted `HTMLElement`. `.doc` is shorter and typed non-nullable. + +**Listeners that may outlive a window migration:** capture the `Document` / `Window` at registration and remove on the same one: + +```ts +const doc = containerRef.current?.doc; +if (!doc) return; +doc.addEventListener("keydown", handler); +return () => doc.removeEventListener("keydown", handler); +``` + +Do **not** rely on `activeDocument` at registration _and_ removal — it can shift between the two calls if focus moves. + +**View migrated to a new window:** for a view that owns React or other long-lived renderers, register `this.containerEl.onWindowMigrated((win) => { ... })` in `onOpen`. The callback fires when Obsidian reparents the element into a different window's document. Tear down and rebuild the renderer there so it captures the new window. Save the returned destroy function and call it in `onClose` to avoid leaks. `CopilotView` is the canonical example — it unmounts and recreates the React root on migration so Lexical re-binds to the popout's window. + +**Cross-realm `instanceof`:** popout windows have their own `Element`, `MouseEvent`, etc., so standard `instanceof` checks fail across windows. Use Obsidian's `element.instanceOf(HTMLElement)` and `event.instanceOf(MouseEvent)` when checking type across realms. + +**Tests (jsdom):** `jest.setup.js` polyfills `Node.doc` / `Node.win` so plugin code using these properties works under jsdom. Don't add `instanceof` guards that depend on the Obsidian-augmented globals without considering the test environment. + ### Architecture Migration Notes - **SharedState Removed**: The legacy `src/sharedState.ts` has been completely removed diff --git a/__mocks__/obsidian.js b/__mocks__/obsidian.js index 73c135e7..c6959999 100644 --- a/__mocks__/obsidian.js +++ b/__mocks__/obsidian.js @@ -52,7 +52,7 @@ module.exports = { }, })), ItemView: jest.fn().mockImplementation(function () { - this.containerEl = document.createElement("div"); + this.containerEl = window.document.createElement("div"); this.onOpen = jest.fn(); this.onClose = jest.fn(); this.getDisplayText = jest.fn().mockReturnValue("Mock View"); @@ -61,7 +61,7 @@ module.exports = { }), Notice: jest.fn().mockImplementation(function (message) { this.message = message; - this.noticeEl = document.createElement("div"); + this.noticeEl = window.document.createElement("div"); this.hide = jest.fn(); }), TFile: jest.fn().mockImplementation(function (path) { diff --git a/jest.setup.js b/jest.setup.js index 97b5d944..6be58919 100644 --- a/jest.setup.js +++ b/jest.setup.js @@ -1,5 +1,25 @@ +/* eslint-env browser */ import "web-streams-polyfill/dist/polyfill.min.js"; import { TextEncoder, TextDecoder } from "util"; global.TextEncoder = TextEncoder; global.TextDecoder = TextDecoder; + +// Polyfill Obsidian's Node.doc / Node.win augmentation so plugin code that +// reads `element.doc` / `element.win` works under jsdom. +if (typeof Node !== "undefined" && !Object.prototype.hasOwnProperty.call(Node.prototype, "doc")) { + Object.defineProperty(Node.prototype, "doc", { + get() { + return this.ownerDocument ?? global.document; + }, + configurable: true, + }); +} +if (typeof Node !== "undefined" && !Object.prototype.hasOwnProperty.call(Node.prototype, "win")) { + Object.defineProperty(Node.prototype, "win", { + get() { + return this.ownerDocument?.defaultView ?? global.window; + }, + configurable: true, + }); +} diff --git a/src/commands/CustomCommandChatModal.tsx b/src/commands/CustomCommandChatModal.tsx index eab425f9..2f4346c7 100644 --- a/src/commands/CustomCommandChatModal.tsx +++ b/src/commands/CustomCommandChatModal.tsx @@ -504,7 +504,7 @@ export class CustomCommandChatModal { * the modal is positioned relative to the correct window. */ private resolveWindow(view?: MarkdownView | null): Window { - return view?.containerEl?.ownerDocument?.defaultView ?? window; + return view?.containerEl?.win ?? window; } /** @@ -513,7 +513,7 @@ export class CustomCommandChatModal { * appended to the document that owns the triggering view. */ private resolveDocument(view?: MarkdownView | null): Document { - return view?.containerEl?.ownerDocument ?? document; + return view?.containerEl?.doc ?? activeDocument; } /** diff --git a/src/components/CopilotView.tsx b/src/components/CopilotView.tsx index d5ec8a60..6b655876 100644 --- a/src/components/CopilotView.tsx +++ b/src/components/CopilotView.tsx @@ -22,6 +22,7 @@ export default class CopilotView extends ItemView { private drawerHideObserver: MutationObserver | null = null; private layout: ChatViewLayout | null = null; private lastDrawerEl: HTMLElement | null = null; + private windowMigrationDestroy: (() => void) | null = null; eventTarget: EventTarget; constructor( @@ -67,6 +68,19 @@ export default class CopilotView extends ItemView { this.setupMobileKeyboardObserver(); this.setupDrawerHideObserver(); + // Reason: When the leaf is dragged to (or back from) an Obsidian popout, the + // containerEl is reparented into a different window's document but onOpen + // does not re-fire. Lexical's editor._window stays bound to the original + // window, so input events fired in the popout never reach the editor. + // Tearing down and recreating the React root forces Lexical to re-register + // its root element under the new window — typing works again. + this.windowMigrationDestroy = this.containerEl.onWindowMigrated(() => { + if (!this.root) return; + this.root.unmount(); + this.root = createRoot(this.containerEl.children[1]); + this.renderView(handleSaveAsNote, updateUserMessageHistory); + }); + // Reason: The view can move between containers (e.g. editor tab → drawer) // without onOpen firing again. Re-bind the drawer observer on layout changes // so it always watches the correct drawer element. @@ -109,13 +123,13 @@ export default class CopilotView extends ItemView { // querying by data-type which is more brittle across Obsidian versions. const isCopilotActive = !!this.containerEl.closest(".workspace-drawer-active-tab-content"); const kbHeight = parseFloat( - document.documentElement.style.getPropertyValue("--keyboard-height") || "0" + this.containerEl.doc.documentElement.style.getPropertyValue("--keyboard-height") || "0" ); drawer.classList.toggle("copilot-keyboard-open", isCopilotActive && kbHeight > 0); }; this.keyboardObserver = new MutationObserver(syncKeyboardClass); - this.keyboardObserver.observe(document.documentElement, { + this.keyboardObserver.observe(this.containerEl.doc.documentElement, { attributes: true, attributeFilter: ["style"], }); @@ -209,6 +223,8 @@ export default class CopilotView extends ItemView { this.keyboardObserver = null; this.drawerHideObserver?.disconnect(); this.drawerHideObserver = null; + this.windowMigrationDestroy?.(); + this.windowMigrationDestroy = null; this.layout?.destroy(); this.layout = null; // Reason: Clean up the class on the tracked drawer element when the view is closed. diff --git a/src/components/chat-components/ChatInput.tsx b/src/components/chat-components/ChatInput.tsx index 7d3d7fe4..53ac2dd1 100644 --- a/src/components/chat-components/ChatInput.tsx +++ b/src/components/chat-components/ChatInput.tsx @@ -7,7 +7,6 @@ import { useProjectLoading, } from "@/aiParams"; import { ChainType } from "@/chainFactory"; -import { AddImageModal } from "@/components/modals/AddImageModal"; import { Button } from "@/components/ui/button"; import { ModelSelector } from "@/components/ui/ModelSelector"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; @@ -654,8 +653,10 @@ const ChatInput: React.FC = ({ } }; - document.addEventListener("keydown", handleKeyDown); - return () => document.removeEventListener("keydown", handleKeyDown); + const doc = containerRef.current?.doc; + if (!doc) return; + doc.addEventListener("keydown", handleKeyDown); + return () => doc.removeEventListener("keydown", handleKeyDown); }, [editMode, onEditCancel]); // Handle tool button toggle-off events - remove corresponding pills @@ -853,8 +854,17 @@ const ChatInput: React.FC = ({ variant="ghost2" size="fit" className="tw-text-muted hover:tw-text-accent" - onClick={() => { - new AddImageModal(app, onAddImage).open(); + onClick={(e) => { + const input = e.currentTarget.doc.createElement("input"); + input.type = "file"; + input.accept = "image/*"; + input.multiple = true; + input.addEventListener( + "change", + () => onAddImage(Array.from(input.files || [])), + { once: true } + ); + input.click(); }} > diff --git a/src/components/chat-components/ChatSingleMessage.test.tsx b/src/components/chat-components/ChatSingleMessage.test.tsx index 7bf06c07..473e6692 100644 --- a/src/components/chat-components/ChatSingleMessage.test.tsx +++ b/src/components/chat-components/ChatSingleMessage.test.tsx @@ -97,7 +97,7 @@ describe("think block rendering — closing tags are not consumed by indented co }); beforeAll(() => { - (globalThis as any).activeDocument = document; + (window as any).activeDocument = window.document; }); /** @@ -204,7 +204,7 @@ describe("normalizeFootnoteRendering", () => { }); it("removes separator and backref while preserving non-footnote elements", () => { - const container = document.createElement("div"); + const container = window.document.createElement("div"); container.innerHTML = `

Body 1-1

@@ -229,7 +229,7 @@ describe("normalizeFootnoteRendering", () => { }); it("leaves non-numeric footnote references untouched", () => { - const container = document.createElement("div"); + const container = window.document.createElement("div"); container.innerHTML = `

Body Note-A

@@ -269,7 +269,7 @@ describe("ChatSingleMessage", () => { }); beforeAll(() => { - (globalThis as any).activeDocument = document; + (window as any).activeDocument = window.document; }); it("normalizes rendered footnotes for assistant messages", async () => { diff --git a/src/components/chat-components/ChatSingleMessage.tsx b/src/components/chat-components/ChatSingleMessage.tsx index 2f7a08df..e9ba7a7a 100644 --- a/src/components/chat-components/ChatSingleMessage.tsx +++ b/src/components/chat-components/ChatSingleMessage.tsx @@ -116,10 +116,15 @@ export const linkInlineCitations = (root: HTMLElement): void => { if (citationAnchors.size === 0) return; + // Bind DOM ops to the document that owns `root` — the chat message may live + // in an Obsidian popout while a different window is focused, in which case + // `activeDocument` would create nodes with the wrong owner. + const doc = root.doc; + // Collect text nodes that contain citation patterns (outside sources section) const sourcesEl = root.querySelector(".copilot-sources"); const textNodes: Text[] = []; - const walker = document.createTreeWalker(root, NodeFilter.SHOW_TEXT, { + const walker = doc.createTreeWalker(root, NodeFilter.SHOW_TEXT, { acceptNode(node) { if (sourcesEl?.contains(node)) return NodeFilter.FILTER_REJECT; if (node.parentElement?.closest("code, pre")) return NodeFilter.FILTER_REJECT; @@ -140,27 +145,27 @@ export const linkInlineCitations = (root: HTMLElement): void => { const text = node.textContent || ""; INLINE_CITATION_RE.lastIndex = 0; - const fragment = document.createDocumentFragment(); + const fragment = doc.createDocumentFragment(); let lastIndex = 0; let match: RegExpExecArray | null; while ((match = INLINE_CITATION_RE.exec(text)) !== null) { // Text before the citation if (match.index > lastIndex) { - fragment.appendChild(document.createTextNode(text.slice(lastIndex, match.index))); + fragment.appendChild(doc.createTextNode(text.slice(lastIndex, match.index))); } const nums = match[1].split(/\s*,\s*/).map((s) => parseInt(s.trim(), 10)); const allResolved = nums.every((num) => citationAnchors.has(num)); if (allResolved) { - const span = document.createElement("span"); + const span = doc.createElement("span"); span.className = "copilot-citation-group"; - span.appendChild(document.createTextNode("[")); + span.appendChild(doc.createTextNode("[")); nums.forEach((num, i) => { - if (i > 0) span.appendChild(document.createTextNode(", ")); + if (i > 0) span.appendChild(doc.createTextNode(", ")); const sourceAnchor = citationAnchors.get(num)!; - const link = document.createElement("a"); + const link = doc.createElement("a"); // Copy all attributes from the source anchor so Obsidian internal-link // metadata (e.g. data-href, class="internal-link") is preserved. for (const attr of Array.from(sourceAnchor.attributes)) { @@ -172,17 +177,17 @@ export const linkInlineCitations = (root: HTMLElement): void => { link.setAttribute("aria-label", `Source ${num}`); span.appendChild(link); }); - span.appendChild(document.createTextNode("]")); + span.appendChild(doc.createTextNode("]")); fragment.appendChild(span); } else { - fragment.appendChild(document.createTextNode(match[0])); + fragment.appendChild(doc.createTextNode(match[0])); } lastIndex = match.index + match[0].length; } if (lastIndex < text.length) { - fragment.appendChild(document.createTextNode(text.slice(lastIndex))); + fragment.appendChild(doc.createTextNode(text.slice(lastIndex))); } // If the text node is inside a placeholder span, replace the span itself @@ -668,6 +673,9 @@ const ChatSingleMessage: React.FC = ({ const parsedMessage = parseToolCallMarkers(processedMessage, messageId.current); if (!isUnmountingRef.current) { + // Bind DOM ops to the document that owns the message container so + // popout-window chats don't pick up the wrong document if focus shifts. + const doc = contentRef.current.doc; // Track existing tool call and error block IDs const existingToolCallIds = new Set(); const existingErrorIds = new Set(); @@ -695,7 +703,7 @@ const ChatSingleMessage: React.FC = ({ // Find where to insert this text segment const insertBefore = contentRef.current!.children[currentIndex]; - const textDiv = document.createElement("div"); + const textDiv = doc.createElement("div"); textDiv.className = "message-segment"; if (insertBefore) { @@ -715,11 +723,11 @@ const ChatSingleMessage: React.FC = ({ currentIndex++; } else if (segment.type === "toolCall" && segment.toolCall) { const toolCallId = segment.toolCall.id; - let container = document.getElementById(`tool-call-${toolCallId}`); + let container = doc.getElementById(`tool-call-${toolCallId}`); if (!container) { const insertBefore = contentRef.current!.children[currentIndex]; - const toolDiv = document.createElement("div"); + const toolDiv = doc.createElement("div"); toolDiv.className = "tool-call-container"; toolDiv.id = `tool-call-${toolCallId}`; @@ -747,12 +755,12 @@ const ChatSingleMessage: React.FC = ({ currentIndex++; } else if (segment.type === "error" && segment.error) { const errorId = segment.error.id; - let container = document.getElementById(`error-block-${errorId}`); + let container = doc.getElementById(`error-block-${errorId}`); if (!container) { // Insert error block at the current stream position const insertBefore = contentRef.current!.children[currentIndex]; - const errorDiv = document.createElement("div"); + const errorDiv = doc.createElement("div"); errorDiv.className = "error-block-container"; errorDiv.id = `error-block-${errorId}`; @@ -791,7 +799,7 @@ const ChatSingleMessage: React.FC = ({ existingToolCallIds.forEach((id) => { if (!currentToolCallIds.has(id)) { - const element = document.getElementById(`tool-call-${id}`); + const element = doc.getElementById(`tool-call-${id}`); if (element) { removeToolCallRoot(messageId.current, rootsRef.current, id, "tool call removal"); element.remove(); @@ -808,7 +816,7 @@ const ChatSingleMessage: React.FC = ({ existingErrorIds.forEach((id) => { if (!currentErrorIds.has(id)) { - const element = document.getElementById(`error-block-${id}`); + const element = doc.getElementById(`error-block-${id}`); if (element) { removeErrorBlockRoot( messageId.current, diff --git a/src/components/chat-components/ChatViewLayout.ts b/src/components/chat-components/ChatViewLayout.ts index ead84ff2..667d3ded 100644 --- a/src/components/chat-components/ChatViewLayout.ts +++ b/src/components/chat-components/ChatViewLayout.ts @@ -49,7 +49,7 @@ export class ChatViewLayout { const syncClearance = () => { // Re-query each time to avoid stale references after theme reloads. - const statusBar = document.querySelector(".status-bar"); + const statusBar = this.containerEl.doc.querySelector(".status-bar"); const viewContent = this.containerEl.querySelector(".view-content"); if (!statusBar || !viewContent) return; diff --git a/src/components/chat-components/TypeaheadMenuPortal.tsx b/src/components/chat-components/TypeaheadMenuPortal.tsx index 1251de24..fc21bdad 100644 --- a/src/components/chat-components/TypeaheadMenuPortal.tsx +++ b/src/components/chat-components/TypeaheadMenuPortal.tsx @@ -60,30 +60,36 @@ export function TypeaheadMenuPortal({ null ); - // Calculate dynamic width based on content - const calculateWidth = useCallback(() => { - const maxAllowedWidth = Math.floor(window.innerWidth * MAX_WIDTH_PERCENTAGE); + // Derive the popout-aware window/document from the range itself so positioning + // and the portal target follow the chat's window, not whichever happens to be focused. + const targetWin: Window = range?.startContainer.win ?? window; - if (options.length === 0) return Math.min(MENU_WIDTH, maxAllowedWidth); + const calculateWidth = useCallback( + (win: Window) => { + const maxAllowedWidth = Math.floor(win.innerWidth * MAX_WIDTH_PERCENTAGE); - const maxTitleLength = Math.max(...options.map((opt) => opt.title.length)); - const maxSubtitleLength = Math.max(...options.map((opt) => opt.subtitle?.length || 0)); + if (options.length === 0) return Math.min(MENU_WIDTH, maxAllowedWidth); - const estimatedWidth = Math.max(maxTitleLength * 8 + 32, maxSubtitleLength * 6 + 32); - const preferredWidth = Math.min(Math.max(estimatedWidth, 300), MENU_WIDTH); + const maxTitleLength = Math.max(...options.map((opt) => opt.title.length)); + const maxSubtitleLength = Math.max(...options.map((opt) => opt.subtitle?.length || 0)); - return Math.min(preferredWidth, maxAllowedWidth); - }, [options]); + const estimatedWidth = Math.max(maxTitleLength * 8 + 32, maxSubtitleLength * 6 + 32); + const preferredWidth = Math.min(Math.max(estimatedWidth, 300), MENU_WIDTH); + + return Math.min(preferredWidth, maxAllowedWidth); + }, + [options] + ); - // Positioning for text-triggered menus const recalcPosition = useCallback(() => { if (!range) return; + const win = range.startContainer.win; const rect = range.getBoundingClientRect(); - const containerWidth = calculateWidth(); + const containerWidth = calculateWidth(win); const top = rect.top - 4; const minLeft = 8; - const maxLeft = window.innerWidth - containerWidth - 8; + const maxLeft = win.innerWidth - containerWidth - 8; const left = Math.min(Math.max(rect.left, minLeft), maxLeft); setPosition({ @@ -98,14 +104,16 @@ export function TypeaheadMenuPortal({ }, [recalcPosition]); useEffect(() => { + if (!range) return; + const win = range.startContainer.win; const handler = () => recalcPosition(); - window.addEventListener("resize", handler); - document.addEventListener("scroll", handler, { passive: true }); + win.addEventListener("resize", handler); + win.document.addEventListener("scroll", handler, { passive: true }); return () => { - window.removeEventListener("resize", handler); - document.removeEventListener("scroll", handler); + win.removeEventListener("resize", handler); + win.document.removeEventListener("scroll", handler); }; - }, [recalcPosition]); + }, [recalcPosition, range]); if (!position || options.length === 0) { return null; @@ -135,7 +143,7 @@ export function TypeaheadMenuPortal({
); - return createPortal(container, document.body); + return createPortal(container, targetWin.document.body); } export { tryToPositionRange }; diff --git a/src/components/chat-components/pills/ActiveNotePillNode.tsx b/src/components/chat-components/pills/ActiveNotePillNode.tsx index c7e2a059..421416a0 100644 --- a/src/components/chat-components/pills/ActiveNotePillNode.tsx +++ b/src/components/chat-components/pills/ActiveNotePillNode.tsx @@ -4,11 +4,12 @@ import { DOMConversionOutput, DOMExportOutput, EditorConfig, + LexicalEditor, LexicalNode, NodeKey, $getRoot, } from "lexical"; -import { BasePillNode, SerializedBasePillNode } from "./BasePillNode"; +import { BasePillNode, getEditorDocument, SerializedBasePillNode } from "./BasePillNode"; import { TruncatedPillText } from "./TruncatedPillText"; import { PillBadge } from "./PillBadge"; import { useActiveFile } from "../context/ActiveFileContext"; @@ -43,8 +44,8 @@ export class ActiveNotePillNode extends BasePillNode { return "data-lexical-active-note-pill"; } - createDOM(_config: EditorConfig): HTMLElement { - const span = document.createElement("span"); + createDOM(_config: EditorConfig, editor: LexicalEditor): HTMLElement { + const span = getEditorDocument(editor).createElement("span"); span.className = "active-note-pill-wrapper"; return span; } @@ -75,8 +76,8 @@ export class ActiveNotePillNode extends BasePillNode { }; } - exportDOM(): DOMExportOutput { - const element = document.createElement("span"); + exportDOM(editor: LexicalEditor): DOMExportOutput { + const element = getEditorDocument(editor).createElement("span"); element.setAttribute("data-lexical-active-note-pill", "true"); element.textContent = "{activeNote}"; return { element }; diff --git a/src/components/chat-components/pills/ActiveWebTabPillNode.tsx b/src/components/chat-components/pills/ActiveWebTabPillNode.tsx index a7c06ff8..802019d2 100644 --- a/src/components/chat-components/pills/ActiveWebTabPillNode.tsx +++ b/src/components/chat-components/pills/ActiveWebTabPillNode.tsx @@ -4,6 +4,7 @@ import { DOMConversionOutput, DOMExportOutput, EditorConfig, + LexicalEditor, LexicalNode, NodeKey, $getRoot, @@ -11,7 +12,7 @@ import { import { Globe } from "lucide-react"; import { Platform } from "obsidian"; import { ACTIVE_WEB_TAB_MARKER } from "@/constants"; -import { BasePillNode, SerializedBasePillNode } from "./BasePillNode"; +import { BasePillNode, getEditorDocument, SerializedBasePillNode } from "./BasePillNode"; import { TruncatedPillText } from "./TruncatedPillText"; import { PillBadge } from "./PillBadge"; import { useActiveWebTabState } from "../hooks/useActiveWebTabState"; @@ -44,8 +45,8 @@ export class ActiveWebTabPillNode extends BasePillNode { return "data-lexical-active-web-tab-pill"; } - createDOM(_config: EditorConfig): HTMLElement { - const span = document.createElement("span"); + createDOM(_config: EditorConfig, editor: LexicalEditor): HTMLElement { + const span = getEditorDocument(editor).createElement("span"); span.className = "active-web-tab-pill-wrapper"; return span; } @@ -76,8 +77,8 @@ export class ActiveWebTabPillNode extends BasePillNode { }; } - exportDOM(): DOMExportOutput { - const element = document.createElement("span"); + exportDOM(editor: LexicalEditor): DOMExportOutput { + const element = getEditorDocument(editor).createElement("span"); element.setAttribute("data-lexical-active-web-tab-pill", "true"); element.textContent = ACTIVE_WEB_TAB_MARKER; return { element }; diff --git a/src/components/chat-components/pills/BasePillNode.tsx b/src/components/chat-components/pills/BasePillNode.tsx index 8a2dd5af..0e989103 100644 --- a/src/components/chat-components/pills/BasePillNode.tsx +++ b/src/components/chat-components/pills/BasePillNode.tsx @@ -3,12 +3,23 @@ import { DecoratorNode, DOMExportOutput, EditorConfig, + LexicalEditor, NodeKey, SerializedLexicalNode, } from "lexical"; import { IPillNode } from "../plugins/PillDeletionPlugin"; import { PillBadge } from "./PillBadge"; +/** + * Returns the Document that owns the given Lexical editor's root element. + * Pills must create DOM in the editor's window — using `activeDocument` + * (focused window) can produce nodes with the wrong owner when the chat is + * in an Obsidian popout but a different window is focused. + */ +export function getEditorDocument(editor: LexicalEditor): Document { + return editor.getRootElement()?.doc ?? activeDocument; +} + export interface SerializedBasePillNode extends SerializedLexicalNode { value: string; } @@ -106,8 +117,8 @@ export abstract class BasePillNode extends DecoratorNode implements /** * Default DOM creation - subclasses can override for custom elements. */ - createDOM(_config: EditorConfig): HTMLElement { - const span = document.createElement("span"); + createDOM(_config: EditorConfig, editor: LexicalEditor): HTMLElement { + const span = getEditorDocument(editor).createElement("span"); span.className = this.getClassName(); return span; } @@ -115,8 +126,8 @@ export abstract class BasePillNode extends DecoratorNode implements /** * Default DOM export - subclasses can override for custom attributes. */ - exportDOM(): DOMExportOutput { - const element = document.createElement("span"); + exportDOM(editor: LexicalEditor): DOMExportOutput { + const element = getEditorDocument(editor).createElement("span"); element.setAttribute(this.getDataAttribute(), ""); element.setAttribute("data-pill-value", this.__value); element.textContent = this.__value; diff --git a/src/components/chat-components/pills/FolderPillNode.tsx b/src/components/chat-components/pills/FolderPillNode.tsx index b90d7186..1c01605b 100644 --- a/src/components/chat-components/pills/FolderPillNode.tsx +++ b/src/components/chat-components/pills/FolderPillNode.tsx @@ -4,10 +4,11 @@ import { DOMConversionMap, DOMConversionOutput, DOMExportOutput, + LexicalEditor, LexicalNode, NodeKey, } from "lexical"; -import { BasePillNode, SerializedBasePillNode } from "./BasePillNode"; +import { BasePillNode, getEditorDocument, SerializedBasePillNode } from "./BasePillNode"; import { TruncatedPillText } from "./TruncatedPillText"; import { PillBadge } from "./PillBadge"; @@ -93,8 +94,8 @@ export class FolderPillNode extends BasePillNode { /** * Override to export DOM with curly braces */ - exportDOM(): DOMExportOutput { - const element = document.createElement("span"); + exportDOM(editor: LexicalEditor): DOMExportOutput { + const element = getEditorDocument(editor).createElement("span"); element.setAttribute(this.getDataAttribute(), ""); element.setAttribute("data-pill-value", this.__value); element.textContent = `{${this.getFolderPath()}}`; diff --git a/src/components/chat-components/pills/NotePillNode.tsx b/src/components/chat-components/pills/NotePillNode.tsx index a797ecf7..e98f3952 100644 --- a/src/components/chat-components/pills/NotePillNode.tsx +++ b/src/components/chat-components/pills/NotePillNode.tsx @@ -5,10 +5,11 @@ import { DOMConversionOutput, DOMExportOutput, EditorConfig, + LexicalEditor, LexicalNode, NodeKey, } from "lexical"; -import { BasePillNode, SerializedBasePillNode } from "./BasePillNode"; +import { BasePillNode, getEditorDocument, SerializedBasePillNode } from "./BasePillNode"; import { TruncatedPillText } from "./TruncatedPillText"; import { PillBadge } from "./PillBadge"; @@ -43,8 +44,8 @@ export class NotePillNode extends BasePillNode { return "data-lexical-note-pill"; } - createDOM(_config: EditorConfig): HTMLElement { - const span = document.createElement("span"); + createDOM(_config: EditorConfig, editor: LexicalEditor): HTMLElement { + const span = getEditorDocument(editor).createElement("span"); span.className = "note-pill-wrapper"; return span; } @@ -78,8 +79,8 @@ export class NotePillNode extends BasePillNode { }; } - exportDOM(): DOMExportOutput { - const element = document.createElement("span"); + exportDOM(editor: LexicalEditor): DOMExportOutput { + const element = getEditorDocument(editor).createElement("span"); element.setAttribute("data-lexical-note-pill", "true"); element.setAttribute("data-note-title", this.__noteTitle); element.setAttribute("data-note-path", this.__notePath); diff --git a/src/components/chat-components/pills/URLPillNode.tsx b/src/components/chat-components/pills/URLPillNode.tsx index d93189f7..6bf3e8e5 100644 --- a/src/components/chat-components/pills/URLPillNode.tsx +++ b/src/components/chat-components/pills/URLPillNode.tsx @@ -5,10 +5,11 @@ import { DOMConversionOutput, DOMExportOutput, EditorConfig, + LexicalEditor, LexicalNode, NodeKey, } from "lexical"; -import { BasePillNode, SerializedBasePillNode } from "./BasePillNode"; +import { BasePillNode, getEditorDocument, SerializedBasePillNode } from "./BasePillNode"; import { PillBadge } from "./PillBadge"; export interface SerializedURLPillNode extends SerializedBasePillNode { @@ -49,8 +50,8 @@ export class URLPillNode extends BasePillNode { return "data-lexical-url-pill"; } - createDOM(_config: EditorConfig): HTMLElement { - const span = document.createElement("span"); + createDOM(_config: EditorConfig, editor: LexicalEditor): HTMLElement { + const span = getEditorDocument(editor).createElement("span"); span.className = "url-pill-wrapper"; return span; } @@ -85,8 +86,8 @@ export class URLPillNode extends BasePillNode { }; } - exportDOM(): DOMExportOutput { - const element = document.createElement("span"); + exportDOM(editor: LexicalEditor): DOMExportOutput { + const element = getEditorDocument(editor).createElement("span"); element.setAttribute("data-lexical-url-pill", "true"); element.setAttribute("data-url", this.__url); if (this.__title) { diff --git a/src/components/chat-components/pills/WebTabPillNode.tsx b/src/components/chat-components/pills/WebTabPillNode.tsx index 95cceb0e..9a9f2ff2 100644 --- a/src/components/chat-components/pills/WebTabPillNode.tsx +++ b/src/components/chat-components/pills/WebTabPillNode.tsx @@ -5,12 +5,13 @@ import { DOMConversionOutput, DOMExportOutput, EditorConfig, + LexicalEditor, LexicalNode, NodeKey, } from "lexical"; import { Globe } from "lucide-react"; import { getDomainFromUrl } from "@/utils"; -import { BasePillNode, SerializedBasePillNode } from "./BasePillNode"; +import { BasePillNode, getEditorDocument, SerializedBasePillNode } from "./BasePillNode"; import { PillBadge } from "./PillBadge"; export interface SerializedWebTabPillNode extends SerializedBasePillNode { @@ -63,8 +64,8 @@ export class WebTabPillNode extends BasePillNode { return "data-lexical-web-tab-pill"; } - createDOM(_config: EditorConfig): HTMLElement { - const span = document.createElement("span"); + createDOM(_config: EditorConfig, editor: LexicalEditor): HTMLElement { + const span = getEditorDocument(editor).createElement("span"); span.className = "web-tab-pill-wrapper"; return span; } @@ -99,8 +100,8 @@ export class WebTabPillNode extends BasePillNode { }; } - exportDOM(): DOMExportOutput { - const element = document.createElement("span"); + exportDOM(editor: LexicalEditor): DOMExportOutput { + const element = getEditorDocument(editor).createElement("span"); element.setAttribute("data-lexical-web-tab-pill", "true"); element.setAttribute("data-url", this.__url); if (this.__title) { diff --git a/src/components/command-ui/draggable-modal.tsx b/src/components/command-ui/draggable-modal.tsx index 012b55bc..82372066 100644 --- a/src/components/command-ui/draggable-modal.tsx +++ b/src/components/command-ui/draggable-modal.tsx @@ -80,7 +80,7 @@ export function DraggableModal({ // so a click on the drag handle (without dragging) preserves anchorBottom behavior. const handleMouseDown = useCallback( (e: React.MouseEvent) => { - const ownerDoc = e.currentTarget.ownerDocument; + const ownerDoc = (e.currentTarget as HTMLElement).doc; const startX = e.clientX; const startY = e.clientY; @@ -189,7 +189,7 @@ export function DraggableModal({ if (anchorBottom !== undefined || isManualPositionRef.current) return; if (heightPx === null) return; - const ownerWindow = dragRef.current?.ownerDocument?.defaultView ?? window; + const ownerWindow = dragRef.current?.win ?? window; const maxY = ownerWindow.innerHeight - 12 - heightPx; const newY = Math.max(12, Math.min(position.y, maxY)); if (Math.abs(position.y - newY) < 1) return; @@ -233,7 +233,7 @@ export function DraggableModal({ useEffect(() => { if (!open) return; - const ownerDocument = dragRef.current?.ownerDocument ?? document; + const ownerDocument = dragRef.current?.doc ?? activeDocument; const handleKeyDown = (e: KeyboardEvent) => { // Respect defaultPrevented to let internal components (e.g., Lexical typeahead, Radix menus) consume Escape first diff --git a/src/components/command-ui/markdown-preview.tsx b/src/components/command-ui/markdown-preview.tsx index 10f8a962..9e9fa329 100644 --- a/src/components/command-ui/markdown-preview.tsx +++ b/src/components/command-ui/markdown-preview.tsx @@ -24,8 +24,8 @@ export function MarkdownPreview({ content, renderMarkdown, className }: Markdown const currentGen = ++renderGenRef.current; // Reason: Render into a detached element so that if content changes // mid-render, the stale result never touches the live DOM. - // Reason: Use ownerDocument for popout-window safety in Obsidian - const scratchEl = targetEl.ownerDocument.createElement("div"); + // Reason: Use the target's doc for popout-window safety in Obsidian + const scratchEl = targetEl.doc.createElement("div"); targetEl.innerHTML = ""; renderMarkdown(content, scratchEl) diff --git a/src/components/command-ui/menu-command-modal.tsx b/src/components/command-ui/menu-command-modal.tsx index 358edb51..98c93df8 100644 --- a/src/components/command-ui/menu-command-modal.tsx +++ b/src/components/command-ui/menu-command-modal.tsx @@ -117,7 +117,7 @@ export function MenuCommandModal({ if (!open) return; const modalEl = innerRef.current?.closest('[data-copilot-draggable-modal="true"]'); - const ownerDocument = modalEl?.ownerDocument ?? document; + const ownerDocument = modalEl?.doc ?? activeDocument; const handleKeyDown = (e: KeyboardEvent) => { // Only handle when result is ready and not busy diff --git a/src/components/modals/AddImageModal.tsx b/src/components/modals/AddImageModal.tsx deleted file mode 100644 index 4c3b6224..00000000 --- a/src/components/modals/AddImageModal.tsx +++ /dev/null @@ -1,29 +0,0 @@ -import { App } from "obsidian"; - -export class AddImageModal { - private app: App; - private onImagesSelected: (files: File[]) => void; - - constructor(app: App, onImagesSelected: (files: File[]) => void) { - this.app = app; - this.onImagesSelected = onImagesSelected; - } - - open() { - const input = document.createElement("input"); - input.type = "file"; - input.accept = "image/*"; - input.multiple = true; - input.style.display = "none"; - - input.addEventListener("change", () => { - const files = Array.from(input.files || []); - this.onImagesSelected(files); - // Clean up - document.body.removeChild(input); - }); - - document.body.appendChild(input); - input.click(); - } -} diff --git a/src/components/modals/SourcesModal.tsx b/src/components/modals/SourcesModal.tsx index 4087bb88..1792dad8 100644 --- a/src/components/modals/SourcesModal.tsx +++ b/src/components/modals/SourcesModal.tsx @@ -79,7 +79,7 @@ export class SourcesModal extends Modal { // Display with 4 decimals to match SearchCore logs and avoid apparent ties if (typeof source.score === "number") { itemContainer.appendChild( - document.createTextNode(` - Relevance score: ${source.score.toFixed(4)}`) + this.contentEl.doc.createTextNode(` - Relevance score: ${source.score.toFixed(4)}`) ); } diff --git a/src/components/quick-ask/QuickAskOverlay.tsx b/src/components/quick-ask/QuickAskOverlay.tsx index 6ba388b8..c21f058a 100644 --- a/src/components/quick-ask/QuickAskOverlay.tsx +++ b/src/components/quick-ask/QuickAskOverlay.tsx @@ -314,7 +314,7 @@ export class QuickAskOverlay { if (QuickAskOverlay.overlayRoot) return QuickAskOverlay.overlayRoot; - const doc = host.ownerDocument ?? document; + const doc = host.doc; const root = doc.createElement("div"); root.className = "copilot-quick-ask-overlay-root"; host.appendChild(root); @@ -325,11 +325,11 @@ export class QuickAskOverlay { private mountOverlay(): void { // Mount overlay inside editor DOM for proper layering - const overlayHost = this.options.view.dom ?? document.body; + const overlayHost = this.options.view.dom ?? activeDocument.body; this.overlayHost = overlayHost; // Capture owner document/window for popout window compatibility - const doc = overlayHost.ownerDocument ?? document; + const doc = overlayHost.doc; const win = doc.defaultView ?? window; this.ownerDocument = doc; this.ownerWindow = win; @@ -384,7 +384,7 @@ export class QuickAskOverlay { if (event.key !== "Escape") return; if (event.defaultPrevented) return; - const doc = this.ownerDocument ?? document; + const doc = this.ownerDocument ?? activeDocument; const activeEl = doc.activeElement; const isFocusInsidePanel = !!(activeEl && this.overlayContainer?.contains(activeEl)); @@ -532,7 +532,7 @@ export class QuickAskOverlay { return; } - const doc = this.ownerDocument ?? document; + const doc = this.ownerDocument ?? activeDocument; const hostRect = this.overlayHost?.getBoundingClientRect() ?? doc.body.getBoundingClientRect(); const viewportWidth = hostRect.width; @@ -665,7 +665,7 @@ export class QuickAskOverlay { this.resizeStartRect = rect; this.resizeStartMouse = start; - const doc = this.ownerDocument ?? document; + const doc = this.ownerDocument ?? activeDocument; const body = doc.body; // Prevent text selection and set resize cursor on body during drag @@ -721,7 +721,7 @@ export class QuickAskOverlay { // Only restore body styles if we actually started resizing // This prevents polluting body styles when destroy() is called without resize if (this.isResizing) { - const doc = this.ownerDocument ?? document; + const doc = this.ownerDocument ?? activeDocument; const body = doc.body; doc.removeEventListener("mousemove", this.handleResizeMove, true); doc.removeEventListener("mouseup", this.handleResizeEnd, true); @@ -740,7 +740,7 @@ export class QuickAskOverlay { private applyResize(clientX: number, clientY: number): void { if (!this.resizeStartRect || !this.resizeStartMouse || !this.resizeDirection) return; - const doc = this.ownerDocument ?? document; + const doc = this.ownerDocument ?? activeDocument; const hostRect = this.overlayHost?.getBoundingClientRect() ?? doc.body.getBoundingClientRect(); const deltaX = clientX - this.resizeStartMouse.x; @@ -856,7 +856,7 @@ export class QuickAskOverlay { private updateDragPosition(): void { if (!this.overlayContainer || !this.dragPosition) return; - const doc = this.ownerDocument ?? document; + const doc = this.ownerDocument ?? activeDocument; const hostRect = this.overlayHost?.getBoundingClientRect() ?? doc.body.getBoundingClientRect(); const viewportWidth = hostRect.width; diff --git a/src/contexts/TabContext.tsx b/src/contexts/TabContext.tsx index 1f0563d9..7527b5c2 100644 --- a/src/contexts/TabContext.tsx +++ b/src/contexts/TabContext.tsx @@ -15,7 +15,7 @@ export const TabProvider: React.FC<{ children: React.ReactNode }> = ({ children useEffect(() => { if (!hasInitialized.current) { - const modal = document.querySelector(".modal-container") as HTMLElement; + const modal = activeDocument.querySelector(".modal-container") as HTMLElement; setModalContainer(modal); hasInitialized.current = true; } diff --git a/src/hooks/use-draggable.ts b/src/hooks/use-draggable.ts index 85344e91..73e8ec0a 100644 --- a/src/hooks/use-draggable.ts +++ b/src/hooks/use-draggable.ts @@ -106,7 +106,7 @@ export function useDraggable(options: UseDraggableOptions = {}) { const el = dragRef.current; if (bounds === "window" && el) { - const ownerWindow = el.ownerDocument?.defaultView ?? window; + const ownerWindow = el.win; const rect = el.getBoundingClientRect(); const maxX = ownerWindow.innerWidth - rect.width; const maxY = ownerWindow.innerHeight - rect.height; @@ -141,7 +141,7 @@ export function useDraggable(options: UseDraggableOptions = {}) { const scheduleApply = useCallback((): void => { if (rafIdRef.current != null) return; - const ownerWindow = dragRef.current?.ownerDocument?.defaultView ?? window; + const ownerWindow = dragRef.current?.win ?? window; rafIdRef.current = ownerWindow.requestAnimationFrame(() => { rafIdRef.current = null; @@ -192,7 +192,7 @@ export function useDraggable(options: UseDraggableOptions = {}) { y: e.clientY - current.y, }; - const ownerDocument = dragRef.current?.ownerDocument ?? document; + const ownerDocument = dragRef.current?.doc ?? activeDocument; const ownerWindow = ownerDocument.defaultView ?? window; const body = ownerDocument.body; previousBodyStyleRef.current = { diff --git a/src/hooks/use-resizable.ts b/src/hooks/use-resizable.ts index 6d58f459..c06c0206 100644 --- a/src/hooks/use-resizable.ts +++ b/src/hooks/use-resizable.ts @@ -95,7 +95,7 @@ export function useRafResizable(options: UseRafResizableOptions): UseRafResizabl // Capture owner document/window from the event target const targetElement = e.currentTarget as HTMLElement | null; - const ownerDoc = targetElement?.ownerDocument ?? document; + const ownerDoc = targetElement?.doc ?? activeDocument; ownerDocumentRef.current = ownerDoc; ownerWindowRef.current = ownerDoc.defaultView ?? window; @@ -133,7 +133,7 @@ export function useRafResizable(options: UseRafResizableOptions): UseRafResizabl ? "nesw-resize" : "nwse-resize"; - const ownerDocument = ownerDocumentRef.current ?? document; + const ownerDocument = ownerDocumentRef.current ?? activeDocument; const ownerWindow = ownerWindowRef.current ?? window; const body = ownerDocument.body; const previousCursor = body.style.cursor; diff --git a/src/integration_tests/AgentPrompt.test.ts b/src/integration_tests/AgentPrompt.test.ts index 649d5391..fd0359e2 100644 --- a/src/integration_tests/AgentPrompt.test.ts +++ b/src/integration_tests/AgentPrompt.test.ts @@ -41,7 +41,7 @@ jest.mock("obsidian", () => ({ })), Notice: jest.fn().mockImplementation(function (message) { this.message = message; - this.noticeEl = document.createElement("div"); + this.noticeEl = window.document.createElement("div"); this.hide = jest.fn(); }), Platform: { diff --git a/src/main.ts b/src/main.ts index 0b7ab42e..b7c19aa2 100644 --- a/src/main.ts +++ b/src/main.ts @@ -91,6 +91,7 @@ export default class CopilotPlugin extends Plugin { chatSelectionHighlightController: ChatSelectionHighlightController; private selectionDebounceTimer?: number; private selectionChangeHandler?: () => void; + private selectionListenerDocument?: Document; private lastSelectionSignature?: string; private webSelectionTracker?: WebSelectionTracker; private readonly chatHistoryLastAccessedAtManager = new RecentUsageManager(); @@ -378,8 +379,10 @@ export default class CopilotPlugin extends Plugin { }, 500); }; - // Register the DOM selection change event - document.addEventListener("selectionchange", this.selectionChangeHandler); + // Capture the document at registration so removal targets the same one + // (activeDocument can change if the user focuses a popout window). + this.selectionListenerDocument = activeDocument; + this.selectionListenerDocument.addEventListener("selectionchange", this.selectionChangeHandler); } /** @@ -389,9 +392,13 @@ export default class CopilotPlugin extends Plugin { if (this.selectionDebounceTimer) { window.clearTimeout(this.selectionDebounceTimer); } - if (this.selectionChangeHandler) { - document.removeEventListener("selectionchange", this.selectionChangeHandler); + if (this.selectionChangeHandler && this.selectionListenerDocument) { + this.selectionListenerDocument.removeEventListener( + "selectionchange", + this.selectionChangeHandler + ); } + this.selectionListenerDocument = undefined; } /**