diff --git a/apps/browser/src/platform/browser/browser-api.spec.ts b/apps/browser/src/platform/browser/browser-api.spec.ts index f6bf7d4069b..49d3e8e1cec 100644 --- a/apps/browser/src/platform/browser/browser-api.spec.ts +++ b/apps/browser/src/platform/browser/browser-api.spec.ts @@ -589,4 +589,113 @@ describe("BrowserApi", () => { ); }); }); + + /* + * Safari sometimes returns >1 tabs unexpectedly even when + * specificing a `windowId` or `currentWindow: true` query option. + * + * For example, when there are >=2 windows with an active pinned tab, + * the pinned tab will always be included as the first entry in the array, + * while the correct tab is included as the second entry. + * + * These tests can remain as verification when Safari fixes this bug. + */ + describe.each([{ isSafariApi: true }, { isSafariApi: false }])( + "SafariTabsQuery %p", + ({ isSafariApi }) => { + let originalIsSafariApi = BrowserApi.isSafariApi; + const expectedWindowId = 10; + const wrongWindowId = expectedWindowId + 1; + const raceConditionWindowId = expectedWindowId + 2; + const mismatchedWindowId = expectedWindowId + 3; + + const resolvedTabsQueryResult = [ + mock({ + title: "tab[0] is a pinned tab from another window", + pinned: true, + windowId: wrongWindowId, + }), + mock({ + title: "tab[1] is the tab with the correct foreground window", + windowId: expectedWindowId, + }), + ]; + + function mockCurrentWindowId(id: number | null) { + jest + .spyOn(BrowserApi, "getCurrentWindow") + .mockResolvedValue(mock({ id })); + } + + beforeEach(() => { + originalIsSafariApi = BrowserApi.isSafariApi; + BrowserApi.isSafariApi = isSafariApi; + mockCurrentWindowId(expectedWindowId); + jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue(resolvedTabsQueryResult); + }); + + afterEach(() => { + BrowserApi.isSafariApi = originalIsSafariApi; + jest.restoreAllMocks(); + }); + + describe.each([BrowserApi.getTabFromCurrentWindow, BrowserApi.getTabFromCurrentWindowId])( + "%p", + (getCurrTabFn) => { + it("returns the first tab when the query result has one tab", async () => { + const expectedSingleTab = resolvedTabsQueryResult[0]; + jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue([expectedSingleTab]); + const actualTab = await getCurrTabFn(); + expect(actualTab).toBe(expectedSingleTab); + }); + + it("returns the first tab when the current window ID is mismatched", async () => { + mockCurrentWindowId(mismatchedWindowId); + const actualTab = await getCurrTabFn(); + expect(actualTab).toBe(resolvedTabsQueryResult[0]); + }); + + it("returns the first tab when the current window ID is unavailable", async () => { + mockCurrentWindowId(null); + const actualTab = await getCurrTabFn(); + expect(actualTab).toBe(resolvedTabsQueryResult[0]); + }); + + if (isSafariApi) { + it("returns the tab with the current window ID", async () => { + const actualTab = await getCurrTabFn(); + expect(actualTab.windowId).toBe(expectedWindowId); + }); + + it(`returns the tab with the current window ID at the time of calling [Function ${getCurrTabFn.name}]`, async () => { + jest.spyOn(BrowserApi, "tabsQuery").mockImplementation(() => { + /* + * Simulate rapid clicking/switching between windows, e.g. + * 1. From Window A, call `getCurrTabFn()` + * 2. getCurrTabFn() calls `await BrowserApi.tabsQuery()` + * 3. Users switches to Window B before the `await` returns + * 4. getCurrTabFn() calls `await BrowserApi.getCurrentWindow()` + * ^ This now returns Window B and filters the results erroneously + */ + mockCurrentWindowId(raceConditionWindowId); + + return Promise.resolve(resolvedTabsQueryResult); + }); + + const actualTab = await getCurrTabFn(); + expect(actualTab.windowId).toBe(expectedWindowId); + }); + } /* !isSafariApi */ else { + it("falls back to tabsQueryFirst", async () => { + const tabsQueryFirstSpy = jest.spyOn(BrowserApi, "tabsQueryFirst"); + const actualTab = await getCurrTabFn(); + + expect(tabsQueryFirstSpy).toHaveBeenCalled(); + expect(actualTab).toBe(resolvedTabsQueryResult[0]); + }); + } + }, + ); + }, + ); }); diff --git a/apps/browser/src/platform/browser/browser-api.ts b/apps/browser/src/platform/browser/browser-api.ts index ec16c883bfb..4b4cec7e7da 100644 --- a/apps/browser/src/platform/browser/browser-api.ts +++ b/apps/browser/src/platform/browser/browser-api.ts @@ -125,7 +125,7 @@ export class BrowserApi { } static async getTabFromCurrentWindowId(): Promise | null { - return await BrowserApi.tabsQueryFirst({ + return await BrowserApi.tabsQueryFirstCurrentWindowForSafari({ active: true, windowId: chrome.windows.WINDOW_ID_CURRENT, }); @@ -153,7 +153,7 @@ export class BrowserApi { } static async getTabFromCurrentWindow(): Promise | null { - return await BrowserApi.tabsQueryFirst({ + return await BrowserApi.tabsQueryFirstCurrentWindowForSafari({ active: true, currentWindow: true, }); @@ -197,6 +197,51 @@ export class BrowserApi { return null; } + /** + * Drop-in replacement for {@link BrowserApi.tabsQueryFirst}. + * + * Safari sometimes returns >1 tabs unexpectedly even when + * specificing a `windowId` or `currentWindow: true` query option. + * + * For all of these calls, + * ``` + * await chrome.tabs.query({active: true, currentWindow: true}) + * await chrome.tabs.query({active: true, windowId: chrome.windows.WINDOW_ID_CURRENT}) + * await chrome.tabs.query({active: true, windowId: 10}) + * ``` + * + * Safari could return: + * ``` + * [ + * {windowId: 2, pinned: true, title: "Incorrect tab in another window", …}, + * {windowId: 10, title: "Correct tab in foreground", …}, + * ] + * ``` + * + * This function captures the current window ID manually before running the query, + * then finds and returns the tab with the matching window ID. + * + * See the `SafariTabsQuery` tests in `browser-api.spec.ts`. + * + * This workaround can be removed when Safari fixes this bug. + */ + static async tabsQueryFirstCurrentWindowForSafari( + options: chrome.tabs.QueryInfo, + ): Promise | null { + if (!BrowserApi.isSafariApi) { + return await BrowserApi.tabsQueryFirst(options); + } + + const currentWindowId = (await BrowserApi.getCurrentWindow()).id; + const tabs = await BrowserApi.tabsQuery(options); + + if (tabs.length <= 1 || currentWindowId == null) { + return tabs[0]; + } + + return tabs.find((t) => t.windowId === currentWindowId) ?? tabs[0]; + } + static tabSendMessageData( tab: chrome.tabs.Tab, command: string,