fix(ui): open media URLs directly in new tabs

[DustyShoe]

Summary

Fixes a UI regression where middle-clicking gallery media could ask Windows to open an external application instead of opening a browser tab.

The regression was introduced by PR #9163 in commit eb9a951248775225fdea689caf9044209d6f0829, which changed the middle-click path to open about:blank before navigating to the media URL.

This change opens the media URL directly. If media-cookie self-heal is pending, the URL is reloaded after self-heal settles so protected media remains accessible without using an about:blank intermediary.

A focused regression test verifies that the media URL is passed directly to window.open() when self-heal is not pending.

Related Issues / Discussions

Closes #9522

QA Instructions

1. Enable "Use Middle Click to Open Images/Videos in New Tab".
2. Middle-click an image or video in the gallery.
3. Confirm that the media opens in a browser tab without a Windows application prompt.
4. Repeat after restoring an authenticated session.

Automated verification:

• pnpm exec vitest run src/features/auth/hooks/useMediaCookieRefresh.test.ts
• pnpm build
• Full frontend suite: 150 test files and 1893 tests passed.

Merge Plan

Normal merge. No API, database, schema, Redux, or dependency changes.

Checklist

• The PR has a short but descriptive title, suitable for a changelog
• Tests added / updated (if applicable)
• Changes to a redux slice have a corresponding migration
• Documentation added / updated (if applicable)
• Updated What's New copy (if doing a release after this PR)

[lstein]

Adversarial review. The fix is correct and does what it claims — I confirmed the premise: before #9163, useMiddleClickOpenInNewTab called openImageInNewTab(imageUrl) → window.open(url, '_blank', 'noopener,noreferrer'), which is exactly what the non-pending branch restores.

But the about:blank-replacement branch introduces a new, avoidable regression of its own. One blocking finding, two smaller ones.

Verification: applied the diff locally, ran vitest run src/features/auth src/features/gallery/videoReviewRegressions.test.ts (41 passed, no type errors) and prettier/eslint on both files (clean). The new test is load-bearing — reverting only the source change makes it fail.

---

1. The pending branch navigates the new tab twice, unconditionally

const tab = window.open(url, '_blank');            // navigation 1
void waitForMediaCookieSelfHeal().then(() => {
  tab?.location.replace(url);                      // navigation 2 — never conditional
});

Triggering sequence:

1. Multiuser mode. User logs in — the login response already sets invokeai_media_token (invokeai/app/api/routers/auth.py:245), so media works immediately.
2. useMediaCookieRefresh mounts, setSelfHealPending(true), and refreshMediaCookie() hits a transient network error → RETRY_DELAYS_MS[0] = 2000, so selfHealPending stays true for 2s, and up to ~12s across both retries.
3. Inside that window the user middle-clicks a video in the gallery (GalleryVideoItem.tsx:130 → useMiddleClickOpenInNewTab → openMediaInNewTab).
4. Pending branch taken. The tab opens, the cookie is valid, the video loads and starts playing.
5. The refresh settles — on success, on 401, or on retries exhausted; all three call setSelfHealPending(false) — and tab.location.replace(url) fires. The tab reloads: playback restarts at 0:00 and the whole file is re-fetched.

Under the old code the tab sat on about:blank, so there was exactly one media load. The replace is only useful if the first load actually 401'd, and nothing checks that — it can't, cross-document. On the 401 and retries-exhausted paths the reload is guaranteed to be a second 401.

Suggested fix: drop the pending branch entirely and always take the direct path. That is what middle-click did before #9163, what ContextMenuItemOpenInNewTab still does for images today via openImageInNewTab, and what this PR's own non-pending branch already accepts. The residual exposure is a broken tab during a sub-second startup window, which the code already tolerates in three other places. If the retry is worth keeping, at minimum skip the replace when the self-heal ended in failure.

2. The non-pending branch duplicates an existing helper

src/common/util/openImageInNewTab.ts is, in its entirety:

export const openImageInNewTab = (imageUrl: string) => {
  window.open(imageUrl, '_blank', 'noopener,noreferrer');
};

That is character-for-character the line this PR adds, and it is still live (ContextMenuItemOpenInNewTab.tsx:12). Worth collapsing to one definition — especially since resolving finding 1 makes openMediaInNewTab equal to it.

3. No guard against about:blank returning; the pending branch is untested

This PR exists because #9163 quietly swapped in about:blank. The repo already uses source-text regression guards for exactly this class of thing (protectedMediaConsumers.test.ts, and videoReviewRegressions.test.ts, which asserts not.toContain('window.open(videoDTO.video_url')). A one-liner alongside them —

expect(readSource('.../useMediaCookieRefresh.ts')).not.toContain('about:blank');

— would actually lock the fix in. As it stands the new test covers only the branch that has no logic; the two-step branch has zero coverage.

---

Attacks that failed, for the record

• noopener,noreferrer forces a popup window instead of a tab — no. MDN carves out exactly these two tokens, and openImageInNewTab has shipped this string in production.
• tab.opener = null throws cross-origin — no. Navigation is async, so the handle still points at the same-origin initial about:blank document at assignment time. Moot anyway: media URLs are relative.
• tab.location.replace blocked cross-origin — no. replace is on the cross-origin-accessible property list, and again the URLs are same-origin.
• The first 401 pops a browser auth dialog — no. Media routes send WWW-Authenticate: Bearer (auth_dependencies.py:127-147); browsers only prompt for Basic/Digest/NTLM/Negotiate.
• The second navigation triggers a duplicate download — no. Content-Disposition: inline on both media routes (images.py:388, videos.py:608), so the tab renders rather than downloads.
• SameSite=lax cookie dropped because of noreferrer — no. SameSite is computed from the initiator site, not the Referer header; a top-level same-site GET still sends it.
• Startup hole where selfHealPending is still false before the effect runs — real, but identical under the old code (waitForMediaCookieSelfHeal() resolved immediately → same instant 401). Not a regression.
• Popup blocked → tab === null — guarded by tab?., unchanged.
• Test global-stub leakage — afterEach is scoped to the new describe and unstubAllGlobals restores it. The vitest environment is node (no environment in vite.config.mts), so window genuinely comes from the stub.
• Another about:blank opener survives the fix — konva/util.ts:460 (window.open('') in previewBlob) would hit the same OS-delegation issue, but both call sites are gated on manager._isDebugging. Dev-only, out of scope.

[DustyShoe]

Thanks for the thorough review. I agree with the findings and addressed all three in 1d3379811c.

1. Removed the pending self-heal navigation branch entirely. Media URLs now always open directly, with no unconditional reload or duplicate fetch.
2. Replaced the separate image/video openers with a single shared openMediaInNewTab helper used by all image and video consumers.
3. Added regression coverage that:
   • verifies the URL is passed directly to window.open();
   • rejects any about:blank intermediary;
   • ensures the middle-click hook remains wired to the shared direct opener instead of the auth helper.

[lstein]

Round 2 at 7e5f751e7a — all three round-1 findings are fixed, and an adversarial pass over the new shape found no blocking defects. Approving.

What I verified

Applied the PR diff on top of main (0b6a024f2f) and ran the full gate:

• pnpm lint:prettier, lint:eslint, lint:knip, lint:dpdm, lint:tsc — all pass.
• Full suite: 168 files / 2204 tests green, no type errors.
• Test sensitivity: reverting both source changes (hook import back to the auth hook, and an about:blank hop back into the util) fails all three new tests with the expected assertions. They are load-bearing, not decorative.

Attacks attempted, and why each failed

• Orphaned state from the removed opener. waitForMediaCookieSelfHeal still serves useDownloadImage; selfHealWaiters is still resolved and cleared on settle, so no waiter can leak. isMediaCookieSelfHealPending is still live at CurrentVideoPreview.tsx:169 for transient-401 toast suppression — the import was correctly split rather than dropped, so there's no dead import either.
• Stale references to a removed export. openImageInNewTab has zero references repo-wide; all five openMediaInNewTab consumers resolve to the new module.
• noopener,noreferrer forces a popup window instead of a tab. No — MDN carves out feature strings containing only those two tokens, and the image path has shipped this exact call for a long time.
• noreferrer drops the media cookie. No — the cookie is SameSite=Lax, and SameSite keys off the initiator site, not Referer. Same-origin top-level GET either way.
• A direct top-level navigation downloads instead of rendering. No — both images.py:413 and videos.py:625 send Content-Disposition: inline.
• window.open(undefined) from some caller. No — all three useMiddleClickOpenInNewTab call sites pass required DTO fields (image_url / video_url).
• Layering / import cycles. common/ no longer imports features/auth — that direction is now correct, and dpdm is clean.

One behavior loss, which I think is the right call

Middle-clicking a video while the media-cookie self-heal is still pending now navigates straight to a URL that may 401, instead of waiting — the user lands on a JSON error page and has to click again. The window is normally sub-second, up to ~12s if the initial refresh fails and retries.

I don't think that should be traded back, and the reason is worth writing down: selfHealPending === true does not mean the cookie is missing. The hook also runs on fresh logins, where the login response already set the cookie and the refresh is (per its own comment) "a harmless no-op". So the old wait was an unreliable proxy whose cost — reloading a playing video — landed in the common case, while its benefit covers only a narrow, self-recoverable one.

Non-blocking nits

1. The PR body is now stale, and it becomes the squash-merge message: "If media-cookie self-heal is pending, the URL is reloaded after self-heal settles" describes the branch that 1d3379811c removed, and "Automated verification: useMediaCookieRefresh.test.ts" no longer covers this change (that file is untouched; the coverage is openMediaInNewTab.test.ts).
2. Consider a one-line comment in openMediaInNewTab.ts — e.g. // Navigate directly; an about:blank hop makes some Windows shells offer an external app (#9522) — so the hop doesn't get re-added by someone re-reading the #9163 rationale. The guard test enforces it, but the why isn't in the file.