Fix Safari pinned tabs causing autofill to be wrong (#13755)

Co-authored-by: Jonathan Prusik <jprusik@users.noreply.github.com>

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<chrome.tabs.Tab>({
+          title: "tab[0] is a pinned tab from another window",
+          pinned: true,
+          windowId: wrongWindowId,
+        }),
+        mock<chrome.tabs.Tab>({
+          title: "tab[1] is the tab with the correct foreground window",
+          windowId: expectedWindowId,
+        }),
+      ];
+
+      function mockCurrentWindowId(id: number | null) {
+        jest
+          .spyOn(BrowserApi, "getCurrentWindow")
+          .mockResolvedValue(mock<chrome.windows.Window>({ 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]);
+            });
+          }
+        },
+      );
+    },
+  );
 });

apps/browser/src/platform/browser/browser-api.ts

@@ -125,7 +125,7 @@ export class BrowserApi {
   }
 
   static async getTabFromCurrentWindowId(): Promise<chrome.tabs.Tab> | 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<chrome.tabs.Tab> | 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<chrome.tabs.Tab> | 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,