From 5f09a02f2f3384abd74a28856c76a56e4cc7b687 Mon Sep 17 00:00:00 2001
From: Alexander Ebert <ebert@woltlab.com>
Date: Sun, 21 Apr 2024 15:33:14 +0200
Subject: [PATCH] Reliably cancel an in-flight request

The `_previousXhr` property was not reliably set previously and the current in-flight was not visible unless a new request was dispatched.

The updated control flow in ca409b33e7550595d3ac2b50bb14b179237c13c9 effectively allowed for the `_previousXhr` property to be removed entirely. Working with `_xhr` directly allows to abort an in-flight request without dispatching a new request. This also fixes a third bug hidden in `_finalize()` that could have erased a later in-flight request in some cases.

See https://www.woltlab.com/community/thread/305578-halb-fehler-halb-wunsch/
---
 ts/WoltLabSuite/Core/Ajax/Request.ts          | 25 ++++++++-----------
 .../js/WoltLabSuite/Core/Ajax/Request.js      | 23 ++++++++---------
 2 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/ts/WoltLabSuite/Core/Ajax/Request.ts b/ts/WoltLabSuite/Core/Ajax/Request.ts
index 9eef6c8c28..9ac6809ef1 100644
--- a/ts/WoltLabSuite/Core/Ajax/Request.ts
+++ b/ts/WoltLabSuite/Core/Ajax/Request.ts
@@ -26,7 +26,6 @@ let _ignoreAllErrors = false;
 class AjaxRequest {
   private readonly _options: RequestOptions;
   private readonly _data: RequestData;
-  private _previousXhr?: XMLHttpRequest;
   private _xhr?: XMLHttpRequest;
 
   constructor(options: RequestOptions) {
@@ -108,10 +107,6 @@ class AjaxRequest {
    * Dispatches a request, optionally aborting a currently active request.
    */
   sendRequest(abortPrevious?: boolean): void {
-    if (this._xhr instanceof XMLHttpRequest) {
-      this._previousXhr = this._xhr;
-    }
-
     if (abortPrevious || this._options.autoAbort) {
       this.abortPrevious();
     }
@@ -183,12 +178,12 @@ class AjaxRequest {
    * Aborts a previous request.
    */
   abortPrevious(): void {
-    if (!this._previousXhr) {
+    if (this._xhr === undefined) {
       return;
     }
 
-    this._previousXhr.abort();
-    this._previousXhr = undefined;
+    this._xhr.abort();
+    this._xhr = undefined;
 
     if (!this._options.silent) {
       AjaxStatus.hide();
@@ -259,7 +254,7 @@ class AjaxRequest {
       options.success(data || {}, xhr.responseText, xhr, options.data!);
     }
 
-    this._finalize(options);
+    this._finalize(xhr, options);
   }
 
   /**
@@ -301,7 +296,7 @@ class AjaxRequest {
       }
     }
 
-    this._finalize(options);
+    this._finalize(xhr, options);
   }
 
   /**
@@ -349,15 +344,15 @@ class AjaxRequest {
 
   /**
    * Finalizes a request.
-   *
-   * @param  {Object}  options    request options
    */
-  _finalize(options: RequestOptions): void {
+  _finalize(xhr: XMLHttpRequest, options: RequestOptions): void {
     if (typeof options.finalize === "function") {
-      options.finalize(this._xhr!);
+      options.finalize(xhr);
     }
 
-    this._previousXhr = undefined;
+    if (this._xhr === xhr) {
+      this._xhr = undefined;
+    }
 
     DomChangeListener.trigger();
 
diff --git a/wcfsetup/install/files/js/WoltLabSuite/Core/Ajax/Request.js b/wcfsetup/install/files/js/WoltLabSuite/Core/Ajax/Request.js
index dd234cdcaf..ba0ad67dc4 100644
--- a/wcfsetup/install/files/js/WoltLabSuite/Core/Ajax/Request.js
+++ b/wcfsetup/install/files/js/WoltLabSuite/Core/Ajax/Request.js
@@ -85,9 +85,6 @@ define(["require", "exports", "tslib", "./Status", "../Core", "../Dom/Change/Lis
          * Dispatches a request, optionally aborting a currently active request.
          */
         sendRequest(abortPrevious) {
-            if (this._xhr instanceof XMLHttpRequest) {
-                this._previousXhr = this._xhr;
-            }
             if (abortPrevious || this._options.autoAbort) {
                 this.abortPrevious();
             }
@@ -155,11 +152,11 @@ define(["require", "exports", "tslib", "./Status", "../Core", "../Dom/Change/Lis
          * Aborts a previous request.
          */
         abortPrevious() {
-            if (!this._previousXhr) {
+            if (this._xhr === undefined) {
                 return;
             }
-            this._previousXhr.abort();
-            this._previousXhr = undefined;
+            this._xhr.abort();
+            this._xhr = undefined;
             if (!this._options.silent) {
                 AjaxStatus.hide();
             }
@@ -218,7 +215,7 @@ define(["require", "exports", "tslib", "./Status", "../Core", "../Dom/Change/Lis
                 }
                 options.success(data || {}, xhr.responseText, xhr, options.data);
             }
-            this._finalize(options);
+            this._finalize(xhr, options);
         }
         /**
          * Handles failed requests, this can be both a successful request with
@@ -254,7 +251,7 @@ define(["require", "exports", "tslib", "./Status", "../Core", "../Dom/Change/Lis
                     });
                 }
             }
-            this._finalize(options);
+            this._finalize(xhr, options);
         }
         /**
          * Returns the inner HTML for an error/exception display.
@@ -294,14 +291,14 @@ define(["require", "exports", "tslib", "./Status", "../Core", "../Dom/Change/Lis
         }
         /**
          * Finalizes a request.
-         *
-         * @param  {Object}  options    request options
          */
-        _finalize(options) {
+        _finalize(xhr, options) {
             if (typeof options.finalize === "function") {
-                options.finalize(this._xhr);
+                options.finalize(xhr);
+            }
+            if (this._xhr === xhr) {
+                this._xhr = undefined;
             }
-            this._previousXhr = undefined;
             Listener_1.default.trigger();
             // fix anchor tags generated through WCF::getAnchor()
             document.querySelectorAll('a[href*="#"]').forEach((link) => {
-- 
2.20.1