Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions electron/ipc/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
51 changes: 51 additions & 0 deletions electron/native-bridge/screen/macPickerSession.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({});
Expand Down
26 changes: 26 additions & 0 deletions electron/native-bridge/screen/macPickerSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ export class MacPickerSession {
private proc: ChildProcessWithoutNullStreams | null = null;
private starting: Promise<boolean> | 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 = "";
Expand All @@ -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<boolean> {
if (this.proc) {
Expand Down Expand Up @@ -286,6 +310,7 @@ export class MacPickerSession {
resolve(selection);
};
});
this.presentedExclusions = [...excludedWindowIds];
this.send({
command: "present",
excludedWindowIds,
Expand Down Expand Up @@ -335,6 +360,7 @@ export class MacPickerSession {
const selection = parsePickerSelection(event);
if (selection) {
this.selection = selection;
this.selectionExclusions = this.presentedExclusions;
}
this.resolvePick(selection);
return;
Expand Down
Loading