mirror of
https://github.com/Tencent/BrowserSkill.git
synced 2026-09-10 13:01:23 +08:00
fix(extension): dismiss overlay on borrow timeout
Send borrow-cancel when the confirmation timeout denies, and add a timer fallback so auto-deny does not depend only on CSS transitionend. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -62,6 +62,9 @@ function BorrowToastStack({ children }: { children: React.ReactNode }) {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Matches BorrowProgressBar / CountdownRing transition duration (duration-1000). */
|
||||||
|
const PROGRESS_TRANSITION_MS = 1000;
|
||||||
|
|
||||||
function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; isModal: boolean }) {
|
function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; isModal: boolean }) {
|
||||||
const { t } = useTranslation("extension");
|
const { t } = useTranslation("extension");
|
||||||
const totalSeconds = Math.ceil(request.timeoutMs / 1000);
|
const totalSeconds = Math.ceil(request.timeoutMs / 1000);
|
||||||
@@ -74,17 +77,6 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i
|
|||||||
onAllowRef.current = request.onAllow;
|
onAllowRef.current = request.onAllow;
|
||||||
onDenyRef.current = request.onDeny;
|
onDenyRef.current = request.onDeny;
|
||||||
|
|
||||||
useEffect(() => {
|
|
||||||
if (secondsLeft <= 0) {
|
|
||||||
if (!settledRef.current) {
|
|
||||||
setAwaitingAutoDeny(true);
|
|
||||||
}
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
const id = setTimeout(() => setSecondsLeft((s) => s - 1), 1000);
|
|
||||||
return () => clearTimeout(id);
|
|
||||||
}, [secondsLeft]);
|
|
||||||
|
|
||||||
function triggerAllow() {
|
function triggerAllow() {
|
||||||
if (settledRef.current) return;
|
if (settledRef.current) return;
|
||||||
settledRef.current = true;
|
settledRef.current = true;
|
||||||
@@ -105,6 +97,27 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i
|
|||||||
setTimeout(() => onDenyRef.current(), EXIT_ANIMATION_MS);
|
setTimeout(() => onDenyRef.current(), EXIT_ANIMATION_MS);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
useEffect(() => {
|
||||||
|
if (secondsLeft <= 0) {
|
||||||
|
if (!settledRef.current) {
|
||||||
|
setAwaitingAutoDeny(true);
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const id = setTimeout(() => setSecondsLeft((s) => s - 1), 1000);
|
||||||
|
return () => clearTimeout(id);
|
||||||
|
}, [secondsLeft]);
|
||||||
|
|
||||||
|
// Prefer transitionend for visual sync, but fall back to a timer so
|
||||||
|
// background tabs / suppressed CSS transitions still auto-deny.
|
||||||
|
useEffect(() => {
|
||||||
|
if (!awaitingAutoDeny || settledRef.current) return;
|
||||||
|
const id = setTimeout(() => {
|
||||||
|
handleDeny();
|
||||||
|
}, PROGRESS_TRANSITION_MS);
|
||||||
|
return () => clearTimeout(id);
|
||||||
|
}, [awaitingAutoDeny]);
|
||||||
|
|
||||||
const progress = secondsLeft / totalSeconds;
|
const progress = secondsLeft / totalSeconds;
|
||||||
const truncatedTitle =
|
const truncatedTitle =
|
||||||
request.tabTitle.length > 30 ? `${request.tabTitle.slice(0, 30)}…` : request.tabTitle;
|
request.tabTitle.length > 30 ? `${request.tabTitle.slice(0, 30)}…` : request.tabTitle;
|
||||||
|
|||||||
@@ -38,9 +38,6 @@ describe("BorrowConfirmationOverlay", () => {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
expect(screen.getByText("0 秒后自动拒绝")).toBeTruthy();
|
expect(screen.getByText("0 秒后自动拒绝")).toBeTruthy();
|
||||||
await act(async () => {
|
|
||||||
await vi.advanceTimersByTimeAsync(1000);
|
|
||||||
});
|
|
||||||
const progressCircle = container.querySelectorAll("svg circle")[1];
|
const progressCircle = container.querySelectorAll("svg circle")[1];
|
||||||
expect(progressCircle).toBeTruthy();
|
expect(progressCircle).toBeTruthy();
|
||||||
await act(() => {
|
await act(() => {
|
||||||
@@ -58,6 +55,42 @@ describe("BorrowConfirmationOverlay", () => {
|
|||||||
expect(onAllow).not.toHaveBeenCalled();
|
expect(onAllow).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("auto-denies via timer fallback when progress transitionend never fires", async () => {
|
||||||
|
const onAllow = vi.fn();
|
||||||
|
const onDeny = vi.fn();
|
||||||
|
|
||||||
|
render(
|
||||||
|
<BorrowConfirmationOverlay
|
||||||
|
requests={[
|
||||||
|
{
|
||||||
|
id: "req-fallback",
|
||||||
|
isActiveTab: true,
|
||||||
|
tabTitle: "Example",
|
||||||
|
timeoutMs: 5000,
|
||||||
|
onAllow,
|
||||||
|
onDeny,
|
||||||
|
},
|
||||||
|
]}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
|
||||||
|
for (let i = 0; i < 5; i++) {
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(1000);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
expect(screen.getByText("0 秒后自动拒绝")).toBeTruthy();
|
||||||
|
// No transitionend — the PROGRESS_TRANSITION_MS fallback must still deny.
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(1000);
|
||||||
|
});
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(150);
|
||||||
|
});
|
||||||
|
expect(onDeny).toHaveBeenCalledTimes(1);
|
||||||
|
expect(onAllow).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it("only invokes onDeny once when deny is clicked repeatedly", async () => {
|
it("only invokes onDeny once when deny is clicked repeatedly", async () => {
|
||||||
const onAllow = vi.fn();
|
const onAllow = vi.fn();
|
||||||
const onDeny = vi.fn();
|
const onDeny = vi.fn();
|
||||||
|
|||||||
@@ -54,8 +54,7 @@ export default defineBackground(() => {
|
|||||||
deps: {
|
deps: {
|
||||||
// Skip every Agent Window when choosing where to render the
|
// Skip every Agent Window when choosing where to render the
|
||||||
// overlay — Agent Windows boot on about:blank, which has no
|
// overlay — Agent Windows boot on about:blank, which has no
|
||||||
// content script, so sendMessage would fail-open and silently
|
// content script, so they cannot surface an authorization decision.
|
||||||
// allow the borrow without any UI shown.
|
|
||||||
isAgentWindowId: (windowId) => sessions.findByWindowId(windowId) !== null,
|
isAgentWindowId: (windowId) => sessions.findByWindowId(windowId) !== null,
|
||||||
// Resolve i18n strings per-borrow so language switches take effect
|
// Resolve i18n strings per-borrow so language switches take effect
|
||||||
// without re-creating the dispatcher.
|
// without re-creating the dispatcher.
|
||||||
@@ -82,8 +81,8 @@ export default defineBackground(() => {
|
|||||||
// The Allow / Deny buttons on the OS notification are the *explicit*
|
// The Allow / Deny buttons on the OS notification are the *explicit*
|
||||||
// authorization fallback when every candidate user window's content
|
// authorization fallback when every candidate user window's content
|
||||||
// script was missing (extension just reloaded, page in BFCache, etc.).
|
// script was missing (extension just reloaded, page in BFCache, etc.).
|
||||||
// Without this listener those button clicks would land nowhere and we'd
|
// Without this listener those button clicks would land nowhere and the
|
||||||
// be back to fail-open-after-sendMessage-failure.
|
// request would only resolve via the fail-closed background timeout.
|
||||||
if (typeof chrome.notifications?.onButtonClicked?.addListener === "function") {
|
if (typeof chrome.notifications?.onButtonClicked?.addListener === "function") {
|
||||||
attachBorrowNotificationButtonHandler({
|
attachBorrowNotificationButtonHandler({
|
||||||
onButtonClicked: chrome.notifications.onButtonClicked,
|
onButtonClicked: chrome.notifications.onButtonClicked,
|
||||||
|
|||||||
@@ -437,6 +437,7 @@ describe("requestBorrowConfirmation", () => {
|
|||||||
expect(notificationId.startsWith(BORROW_NOTIFICATION_PREFIX)).toBe(true);
|
expect(notificationId.startsWith(BORROW_NOTIFICATION_PREFIX)).toBe(true);
|
||||||
expect(options.type).toBe("basic");
|
expect(options.type).toBe("basic");
|
||||||
expect(options.title).toMatch(/borrow/i);
|
expect(options.title).toMatch(/borrow/i);
|
||||||
|
expect(options.requireInteraction).toBe(true);
|
||||||
expect(notifications.clear).toHaveBeenCalledWith(notificationId);
|
expect(notifications.clear).toHaveBeenCalledWith(notificationId);
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -463,10 +464,12 @@ describe("requestBorrowConfirmation", () => {
|
|||||||
userWindowWithActiveTab({ windowId: 77, tabId: 4242, url: "https://docs.example/" }),
|
userWindowWithActiveTab({ windowId: 77, tabId: 4242, url: "https://docs.example/" }),
|
||||||
);
|
);
|
||||||
// Keep sendMessage pending so the request stays live while we click.
|
// Keep sendMessage pending so the request stays live while we click.
|
||||||
|
// A second call may arrive if the background timeout dismisses the overlay.
|
||||||
let resolveSend: (v: { type: string; allowed: boolean }) => void = () => undefined;
|
let resolveSend: (v: { type: string; allowed: boolean }) => void = () => undefined;
|
||||||
tabs.sendMessage.mockImplementationOnce(
|
tabs.sendMessage.mockImplementationOnce(
|
||||||
() => new Promise((res) => (resolveSend = res as never)),
|
() => new Promise((res) => (resolveSend = res as never)),
|
||||||
);
|
);
|
||||||
|
tabs.sendMessage.mockResolvedValue(undefined);
|
||||||
|
|
||||||
const pending = requestBorrowConfirmation(42, {
|
const pending = requestBorrowConfirmation(42, {
|
||||||
deps: { tabs, windows, notifications },
|
deps: { tabs, windows, notifications },
|
||||||
@@ -476,7 +479,7 @@ describe("requestBorrowConfirmation", () => {
|
|||||||
|
|
||||||
// Simulate user clicking the OS notification.
|
// Simulate user clicking the OS notification.
|
||||||
for (const l of listeners) l(notificationId);
|
for (const l of listeners) l(notificationId);
|
||||||
await vi.runAllTimersAsync();
|
await Promise.resolve();
|
||||||
expect(windows.update).toHaveBeenCalledWith(77, { focused: true });
|
expect(windows.update).toHaveBeenCalledWith(77, { focused: true });
|
||||||
|
|
||||||
// Finish the borrow so the test resolves cleanly.
|
// Finish the borrow so the test resolves cleanly.
|
||||||
@@ -491,12 +494,20 @@ describe("requestBorrowConfirmation", () => {
|
|||||||
userWindowWithActiveTab({ windowId: 11, tabId: 42, url: "https://app.example/" }),
|
userWindowWithActiveTab({ windowId: 11, tabId: 42, url: "https://app.example/" }),
|
||||||
);
|
);
|
||||||
tabs.sendMessage.mockImplementationOnce(() => new Promise(() => undefined));
|
tabs.sendMessage.mockImplementationOnce(() => new Promise(() => undefined));
|
||||||
|
tabs.sendMessage.mockResolvedValueOnce(undefined);
|
||||||
|
|
||||||
const pending = requestBorrowConfirmation(42, {
|
const pending = requestBorrowConfirmation(42, {
|
||||||
deps: { tabs, windows, notifications },
|
deps: { tabs, windows, notifications },
|
||||||
});
|
});
|
||||||
await vi.advanceTimersByTimeAsync(BACKGROUND_TIMEOUT_MS);
|
await vi.advanceTimersByTimeAsync(BACKGROUND_TIMEOUT_MS);
|
||||||
expect(await pending).toBe(false);
|
expect(await pending).toBe(false);
|
||||||
|
// Timeout must dismiss any in-flight overlay so Allow cannot linger.
|
||||||
|
expect(tabs.sendMessage).toHaveBeenCalledTimes(2);
|
||||||
|
const cancelMessage = tabs.sendMessage.mock.calls[1][1] as {
|
||||||
|
type: string;
|
||||||
|
requestId: string;
|
||||||
|
};
|
||||||
|
expect(cancelMessage.type).toBe("borrow-cancel");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("dismisses the pending overlay and denies when aborted", async () => {
|
it("dismisses the pending overlay and denies when aborted", async () => {
|
||||||
|
|||||||
@@ -28,8 +28,9 @@ export const CONFIRMATION_TIMEOUT_MS = 5000;
|
|||||||
const EXIT_ANIMATION_MS = 150;
|
const EXIT_ANIMATION_MS = 150;
|
||||||
/** Matches BorrowConfirmationOverlay progress ring/bar transition (duration-1000). */
|
/** Matches BorrowConfirmationOverlay progress ring/bar transition (duration-1000). */
|
||||||
export const PROGRESS_TRANSITION_MS = 1000;
|
export const PROGRESS_TRANSITION_MS = 1000;
|
||||||
// UI auto-allows after countdown + progress transition + exit fade. Background
|
// UI auto-denies after countdown + progress transition + exit fade. Background
|
||||||
// timeout must not fire before that chain completes or deny is ignored.
|
// timeout must not fire before that chain completes or the UI decision races
|
||||||
|
// with an already-settled deny.
|
||||||
export const BACKGROUND_TIMEOUT_MS =
|
export const BACKGROUND_TIMEOUT_MS =
|
||||||
CONFIRMATION_TIMEOUT_MS + PROGRESS_TRANSITION_MS + EXIT_ANIMATION_MS + 500;
|
CONFIRMATION_TIMEOUT_MS + PROGRESS_TRANSITION_MS + EXIT_ANIMATION_MS + 500;
|
||||||
|
|
||||||
@@ -441,21 +442,24 @@ export async function requestBorrowConfirmation(
|
|||||||
resolve(allowed);
|
resolve(allowed);
|
||||||
};
|
};
|
||||||
|
|
||||||
const onAbort = () => {
|
const dismissPendingOverlay = () => {
|
||||||
if (activeMessageTabId !== null) {
|
if (activeMessageTabId === null) return;
|
||||||
const cancelMessage: BorrowCancelMessage = {
|
const cancelMessage: BorrowCancelMessage = {
|
||||||
type: "borrow-cancel",
|
type: "borrow-cancel",
|
||||||
|
requestId,
|
||||||
|
};
|
||||||
|
void Promise.resolve(tabsApi.sendMessage(activeMessageTabId, cancelMessage)).catch((err) => {
|
||||||
|
console.debug("[bsk borrow] cancel message failed", {
|
||||||
|
tabId,
|
||||||
|
messageTabId: activeMessageTabId,
|
||||||
requestId,
|
requestId,
|
||||||
};
|
error: err instanceof Error ? err.message : String(err),
|
||||||
void tabsApi.sendMessage(activeMessageTabId, cancelMessage).catch((err) => {
|
|
||||||
console.debug("[bsk borrow] cancel message failed", {
|
|
||||||
tabId,
|
|
||||||
messageTabId: activeMessageTabId,
|
|
||||||
requestId,
|
|
||||||
error: err instanceof Error ? err.message : String(err),
|
|
||||||
});
|
|
||||||
});
|
});
|
||||||
}
|
});
|
||||||
|
};
|
||||||
|
|
||||||
|
const onAbort = () => {
|
||||||
|
dismissPendingOverlay();
|
||||||
settle(false);
|
settle(false);
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -470,6 +474,7 @@ export async function requestBorrowConfirmation(
|
|||||||
tabId,
|
tabId,
|
||||||
messageTabId: activeMessageTabId,
|
messageTabId: activeMessageTabId,
|
||||||
});
|
});
|
||||||
|
dismissPendingOverlay();
|
||||||
settle(false);
|
settle(false);
|
||||||
}, BACKGROUND_TIMEOUT_MS);
|
}, BACKGROUND_TIMEOUT_MS);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user