diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 1d9f5ef9d..81e706c08 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -2096,13 +2096,19 @@ export function registerIpcHandlers( }, ); - async function presentMacSystemPicker(session: MacPickerSession) { - // The HUD and the notes window belong to this process, not to the helper, so a display - // pick would record them unless the picker is told to leave them out. + /** + * The HUD and the notes window belong to this process, not to the helper, so a display + * pick would record them unless the picker is told to leave them out. + */ + function macPickerExcludedWindowIds() { const appWindowSourceIds = [getMainWindow(), getNotesWindow()] .filter((window): window is BrowserWindow => !!window && !window.isDestroyed()) .map((window) => window.getMediaSourceId()); - const excludedWindowIds = collectMacCaptureExcludedWindowIds(appWindowSourceIds); + return collectMacCaptureExcludedWindowIds(appWindowSourceIds); + } + + async function presentMacSystemPicker(session: MacPickerSession) { + const excludedWindowIds = macPickerExcludedWindowIds(); // Out of the way while the picker is up. The HUD window is far larger than the bar // it draws (a transparent reserve above it), and Apple's picker targets windows by // their frame, not by where clicks land -- so that invisible rectangle hid every @@ -2179,6 +2185,9 @@ export function registerIpcHandlers( if (!isMacPickerSourceId(selectedSource?.id)) { return selectedDesktopSource ? selectedSource : null; } + // A screen pick that would record the current HUD or notes window is no pick at all: + // answering null makes the next Record present the picker again (#965). + macPickerSession?.forgetSelectionUnlessExcluding(macPickerExcludedWindowIds()); if (!macPickerSession?.getSelection()) { selectedSource = null; selectedDesktopSource = null; @@ -3018,6 +3027,8 @@ export function registerIpcHandlers( // A source from Apple's picker records through the session that holds the pick, // and needs no Screen Recording grant -- so nothing here may go near one. const pickerSession = isMacPickerSourceId(request.source.sourceId) ? macPickerSession : null; + // Normally caught by `get-selected-source` first; this is the last word (#965). + pickerSession?.forgetSelectionUnlessExcluding(macPickerExcludedWindowIds()); const pick = pickerSession?.getSelection() ?? null; if (isMacPickerSourceId(request.source.sourceId) && !pick) { selectedSource = null; diff --git a/electron/native-bridge/screen/macPickerSession.test.ts b/electron/native-bridge/screen/macPickerSession.test.ts index c5945bedf..adac0495b 100644 --- a/electron/native-bridge/screen/macPickerSession.test.ts +++ b/electron/native-bridge/screen/macPickerSession.test.ts @@ -199,6 +199,57 @@ describe("MacPickerSession", () => { expect(helper.commands.at(-1)).toBe("stop"); }); + it("forgets a screen pick once an app window it does not leave out must be (#965)", async () => { + const { session } = await pickDisplay(); + + // The windows the picker was told about, in any order, or fewer of them: still good. + session.forgetSelectionUnlessExcluding([8, 7]); + session.forgetSelectionUnlessExcluding([7]); + expect(session.getSelection()).not.toBeNull(); + + // The HUD came back from the editor as a new window: the pick would record it. + session.forgetSelectionUnlessExcluding([9]); + expect(session.getSelection()).toBeNull(); + }); + + it("keeps the exclusions of the pick, not of a picker that was cancelled", async () => { + const { helper, session } = await pickDisplay(); + const cancelled = session.present([9]); + await flush(); + helper.say({ event: "picker-cancelled" }); + expect(await cancelled).toBeNull(); + + session.forgetSelectionUnlessExcluding([7, 8]); + expect(session.getSelection()).not.toBeNull(); + session.forgetSelectionUnlessExcluding([9]); + expect(session.getSelection()).toBeNull(); + }); + + it("keeps a window pick whatever the app's windows are", async () => { + const { helper, session } = await readySession(); + const pick = session.present([7]); + await flush(); + helper.say({ ...DISPLAY_PICK, kind: "window", windowId: 42 }); + expect(await pick).not.toBeNull(); + + session.forgetSelectionUnlessExcluding([9]); + expect(session.getSelection()).toMatchObject({ kind: "window" }); + }); + + it("keeps the pick while a take is running from it", async () => { + const { helper, session } = await pickDisplay(); + session.startTake({}); + + // The notes window opened mid-take: the running take still reads the pick's frame. + session.forgetSelectionUnlessExcluding([7, 8, 9]); + expect(session.getSelection()).not.toBeNull(); + + helper.say({ event: "take-ended" }); + await flush(); + session.forgetSelectionUnlessExcluding([7, 8, 9]); + expect(session.getSelection()).toBeNull(); + }); + it("ends a running take and forgets the pick when the session dies", async () => { const { helper, session } = await pickDisplay(); const take = session.startTake({}); diff --git a/electron/native-bridge/screen/macPickerSession.ts b/electron/native-bridge/screen/macPickerSession.ts index 0f0e5b186..7b7aea758 100644 --- a/electron/native-bridge/screen/macPickerSession.ts +++ b/electron/native-bridge/screen/macPickerSession.ts @@ -186,6 +186,10 @@ export class MacPickerSession { private proc: ChildProcessWithoutNullStreams | null = null; private starting: Promise | null = null; private selection: MacPickerSelection | null = null; + /** The windows the picker was told to leave out, as of the last `present`. */ + private presentedExclusions: readonly number[] = []; + /** The windows the retained pick leaves out: baked into its filter, fixed until the next pick. */ + private selectionExclusions: readonly number[] = []; private take: TakeProcess | null = null; private pendingPick: ((selection: MacPickerSelection | null) => void) | null = null; private lineBuffer = ""; @@ -199,6 +203,26 @@ export class MacPickerSession { return this.selection; } + /** + * Forgets a screen pick that does not leave out every one of `windowIds`. + * + * The windows a screen pick leaves out are the ones the picker was told about when it + * was shown, baked into the pick's filter. A take cannot add one: a fresh filter needs + * the Screen Recording grant this picker exists to avoid. So once the HUD or the notes + * window is not one the pick knows -- the HUD is a new window after every trip through + * the editor -- the pick would record it, and only picking again leaves it out (#965). + * A window pick records that one window and is never affected, and neither is a take + * already running: its filter is set, and its cursor telemetry still reads the pick. + */ + forgetSelectionUnlessExcluding(windowIds: readonly number[]) { + if (this.selection?.kind !== "display" || (this.take && !this.take.ended)) { + return; + } + if (!windowIds.every((windowId) => this.selectionExclusions.includes(windowId))) { + this.selection = null; + } + } + /** Starts the helper if it is not running. Resolves false when it cannot. */ start(): Promise { if (this.proc) { @@ -286,6 +310,7 @@ export class MacPickerSession { resolve(selection); }; }); + this.presentedExclusions = [...excludedWindowIds]; this.send({ command: "present", excludedWindowIds, @@ -335,6 +360,7 @@ export class MacPickerSession { const selection = parsePickerSelection(event); if (selection) { this.selection = selection; + this.selectionExclusions = this.presentedExclusions; } this.resolvePick(selection); return;