From e478749dd650e64666e2ebdad574b8648b69195b Mon Sep 17 00:00:00 2001 From: YuanboXue-Amber Date: Thu, 12 Nov 2026 13:06:18 +0100 Subject: [PATCH 1/2] fix: cancel observed focus delivery before it runs Retain the cancellable focus operation rather than only its element wait. Stop ready-target delivery on cancellation, supersession, and disposal, while preserving already-completed focus. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/docs/observed.md | 5 ++ src/State/ObservedElement.ts | 32 +++++++-- tests/ObservedElement.test.tsx | 120 +++++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 5 deletions(-) diff --git a/docs/docs/observed.md b/docs/docs/observed.md index 076e5bff..e89c0d8a 100644 --- a/docs/docs/observed.md +++ b/docs/docs/observed.md @@ -89,6 +89,11 @@ focus.result.then((focused: boolean) => { focus.cancel(); ``` +Cancellation prevents focus delivery even when the element has already been +found but the request's promise has not delivered focus yet. Once focus has +been delivered, canceling the completed request does not undo it. Disposing +the observed-element API also cancels its pending focus request. + `waitElement()`/`requestFocus()` both return an `ObservedElementAsyncRequest`, whose `status` is one of the [`ObservedElementRequestStatuses`](api-reference.md#constants) diff --git a/src/State/ObservedElement.ts b/src/State/ObservedElement.ts index 943bd66d..a1f36238 100644 --- a/src/State/ObservedElement.ts +++ b/src/State/ObservedElement.ts @@ -41,7 +41,7 @@ export class ObservedElementAPI [name: string]: { [uid: string]: ObservedElementInfo }; } = {}; private _currentRequest: - Types.ObservedElementAsyncRequest | undefined; + Types.ObservedElementAsyncRequest | undefined; private _currentRequestTimestamp = 0; onObservedElementChange?: (change: Types.ObservedElementChange) => void; @@ -56,6 +56,8 @@ export class ObservedElementAPI } dispose(): void { + this._currentRequest?.cancel(); + this._currentRequest = undefined; this._tabster.focusedElement.unsubscribe(this._onFocus); for (const key of Object.keys(this._waiting)) { @@ -412,17 +414,28 @@ export class ObservedElementAPI ObservedElementAccessibilities.Focusable ); - this._currentRequest = request; this._currentRequestTimestamp = Date.now(); + let isCanceled = false; + let isCompleted = false; const ret: Types.ObservedElementAsyncRequest = { result: request.result.then((element) => { + if (isCanceled) { + return false; + } + if (this._lastRequestFocusId !== requestId) { + isCanceled = true; request.diagnostics.reason = ObservedElementFailureReasons.SupersededByNewRequest; return false; } + isCompleted = true; + if (this._currentRequest === ret) { + delete this._currentRequest; + } + if (!element) { // Element was not found - reason should already be set by timeout or cancellation // If not set, default to timeout reason @@ -449,18 +462,27 @@ export class ObservedElementAPI return focusResult; }), cancel: () => { - request.cancel(); + if (!isCompleted) { + isCanceled = true; + request.cancel(); + ret.status = ObservedElementRequestStatuses.Canceled; + } }, status: request.status, diagnostics: request.diagnostics, }; + this._currentRequest = ret; + request.result.finally(() => { - if (this._currentRequest === request) { + isCompleted = true; + if (this._currentRequest === ret) { delete this._currentRequest; } - ret.status = request.status; + ret.status = isCanceled + ? ObservedElementRequestStatuses.Canceled + : request.status; }); return ret; diff --git a/tests/ObservedElement.test.tsx b/tests/ObservedElement.test.tsx index 1a766dff..791943e6 100644 --- a/tests/ObservedElement.test.tsx +++ b/tests/ObservedElement.test.tsx @@ -865,6 +865,126 @@ describe("Focusable", () => { ); }); + describe("Cancellation before delivery", () => { + it.each(["manual", "superseded", "disposed"])( + "should not focus an already found target after cancellation (%s)", + async (kind) => { + await new BroTest.BroTest( +
+ + +
+ ) + .eval(async (kind: string) => { + const { observedElement, dom } = + getTabsterTestVariables(); + const start = dom?.getElementById(document, "start"); + const target = dom?.getElementById(document, "target"); + if (!observedElement || !start || !target) { + throw new Error("Missing observed test setup"); + } + + start.focus(); + let targetFocusCount = 0; + target.addEventListener("focus", () => { + targetFocusCount++; + }); + const request = observedElement.requestFocus( + "target", + 1000 + ); + let newerRequest: + | Types.ObservedElementAsyncRequest + | undefined; + + if (kind === "manual") { + request.cancel(); + } else if (kind === "superseded") { + newerRequest = observedElement.requestFocus( + "start", + 1000 + ); + } else { + observedElement.dispose(); + } + + const result = await request.result; + const newerResult = await newerRequest?.result; + return { + result, + status: request.status, + targetFocusCount, + newerResult, + }; + }, kind) + .check( + ({ result, status, targetFocusCount, newerResult }) => { + expect(result).toBe(false); + expect(status).toBe( + ObservedElementRequestStatuses.Canceled + ); + expect(targetFocusCount).toBe(0); + if (kind === "superseded") { + expect(newerResult).toBe(true); + } + } + ) + .activeElement((el) => { + expect(el?.attributes.id).toBe("start"); + }); + } + ); + + it("should preserve a completed focus request when cancel is called later", async () => { + await new BroTest.BroTest( + + ) + .eval(async () => { + const request = + getTabsterTestVariables().observedElement?.requestFocus( + "target", + 1000 + ); + if (!request) { + throw new Error("Missing observed test setup"); + } + const result = await request.result; + request.cancel(); + return { result, status: request.status }; + }) + .check(({ result, status }) => { + expect(result).toBe(true); + expect(status).toBe( + ObservedElementRequestStatuses.Succeeded + ); + }) + .activeElement((el) => { + expect(el?.attributes.id).toBe("target"); + }); + }); + }); + describe("Diagnostics", () => { it("should provide diagnostics with waitForElementDuration when element is found immediately", async () => { const name = "test"; From afbfe667ec11112cec8bb141a617c83901c3a173 Mon Sep 17 00:00:00 2001 From: YuanboXue-Amber Date: Thu, 12 Nov 2026 13:34:07 +0100 Subject: [PATCH 2/2] fix: keep stale cancellation from clearing newer waits Make focus cancellation idempotent and reject a pending element wait only while it still owns its registry entry. Add browser regressions for repeated cancellation of focus requests and element waits with the same name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/docs/observed.md | 3 ++ src/State/ObservedElement.ts | 6 ++- tests/ObservedElement.test.tsx | 67 ++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/docs/docs/observed.md b/docs/docs/observed.md index e89c0d8a..e43252e9 100644 --- a/docs/docs/observed.md +++ b/docs/docs/observed.md @@ -73,6 +73,9 @@ wait.result.then((element) => { wait.cancel(); ``` +Canceling a wait again after it has ended does not affect a newer wait for the +same observed name. + ### `requestFocus()` Waits for the observed element to appear, then focuses it. A subsequent diff --git a/src/State/ObservedElement.ts b/src/State/ObservedElement.ts index a1f36238..4d72b2ad 100644 --- a/src/State/ObservedElement.ts +++ b/src/State/ObservedElement.ts @@ -376,8 +376,10 @@ export class ObservedElementAPI request.status = ObservedElementRequestStatuses.Canceled; request.diagnostics.waitForElementDuration = Date.now() - startTime; + if (this._waiting[key] === w) { + this._rejectWaiting(key, true); + } } - this._rejectWaiting(key, true); }, status: ObservedElementRequestStatuses.Waiting, diagnostics: {}, @@ -462,7 +464,7 @@ export class ObservedElementAPI return focusResult; }), cancel: () => { - if (!isCompleted) { + if (!isCompleted && !isCanceled) { isCanceled = true; request.cancel(); ret.status = ObservedElementRequestStatuses.Canceled; diff --git a/tests/ObservedElement.test.tsx b/tests/ObservedElement.test.tsx index 791943e6..f45bf32e 100644 --- a/tests/ObservedElement.test.tsx +++ b/tests/ObservedElement.test.tsx @@ -866,6 +866,73 @@ describe("Focusable", () => { }); describe("Cancellation before delivery", () => { + it.each(["focus", "wait"])( + "should not cancel a newer same-name request through repeated cancellation (%s)", + async (kind) => { + await new BroTest.BroTest( +
+ +
+ ) + .eval(async (kind: string) => { + const { observedElement, dom } = + getTabsterTestVariables(); + const root = dom?.getElementById(document, "root"); + if (!observedElement || !root) { + throw new Error("Missing observed test setup"); + } + + dom?.getElementById(document, "start")?.focus(); + const oldRequest = + kind === "focus" + ? observedElement.requestFocus("target", 1000) + : observedElement.waitElement("target", 1000); + oldRequest.cancel(); + const newerRequest = + kind === "focus" + ? observedElement.requestFocus("target", 1000) + : observedElement.waitElement("target", 1000); + oldRequest.cancel(); + + const target = document.createElement("button"); + target.id = "target"; + target.setAttribute( + "data-tabster", + JSON.stringify({ + observed: { names: ["target"] }, + }) + ); + root.appendChild(target); + + const result = await newerRequest.result; + return { + result: + typeof result === "boolean" + ? result + : result?.id, + status: newerRequest.status, + oldResult: await oldRequest.result, + oldStatus: oldRequest.status, + }; + }, kind) + .check(({ result, status, oldResult, oldStatus }) => { + expect(result).toBe(kind === "focus" ? true : "target"); + expect(status).toBe( + ObservedElementRequestStatuses.Succeeded + ); + expect(oldResult).toBe(kind === "focus" ? false : null); + expect(oldStatus).toBe( + ObservedElementRequestStatuses.Canceled + ); + }) + .activeElement((el) => { + expect(el?.attributes.id).toBe( + kind === "focus" ? "target" : "start" + ); + }); + } + ); + it.each(["manual", "superseded", "disposed"])( "should not focus an already found target after cancellation (%s)", async (kind) => {