fix: cancel observed focus delivery before it runs

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
@@ -89,6 +92,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)

src/State/ObservedElement.ts

@@ -41,7 +41,7 @@ export class ObservedElementAPI
         [name: string]: { [uid: string]: ObservedElementInfo };
     } = {};
     private _currentRequest:
-        Types.ObservedElementAsyncRequest<HTMLElement | null> | undefined;
+        Types.ObservedElementAsyncRequest<boolean> | 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)) {
@@ -374,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: {},
@@ -412,17 +416,28 @@ export class ObservedElementAPI
             ObservedElementAccessibilities.Focusable
         );
 
-        this._currentRequest = request;
         this._currentRequestTimestamp = Date.now();
+        let isCanceled = false;
+        let isCompleted = false;
 
         const ret: Types.ObservedElementAsyncRequest<boolean> = {
             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 +464,27 @@ export class ObservedElementAPI
                 return focusResult;
             }),
             cancel: () => {
-                request.cancel();
+                if (!isCompleted && !isCanceled) {
+                    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;

tests/ObservedElement.test.tsx

@@ -865,6 +865,193 @@ 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(
+                    <div id="root" {...getTabsterAttribute({ root: {} })}>
+                        <button id="start">Start</button>
+                    </div>
+                )
+                    .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) => {
+                await new BroTest.BroTest(
+                    <div {...getTabsterAttribute({ root: {} })}>
+                        <button
+                            id="start"
+                            {...getTabsterAttribute({
+                                observed: { names: ["start"] },
+                            })}
+                        >
+                            Start
+                        </button>
+                        <button
+                            id="target"
+                            {...getTabsterAttribute({
+                                observed: { names: ["target"] },
+                            })}
+                        >
+                            Target
+                        </button>
+                    </div>
+                )
+                    .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<boolean>
+                            | 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(
+                <button
+                    id="target"
+                    {...getTabsterAttribute({
+                        observed: { names: ["target"] },
+                    })}
+                >
+                    Target
+                </button>
+            )
+                .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";