Bug 1901846 - "Done compacting (approx. XX KB saved)" is overwritten by "Loading message..." when message pane is visible. r=gds,tobyp
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Sat, 28 Dec 2024 11:36:08 +0200 (6 months ago)
changeset 43457 fa15d6b1fc22de16c37a780b117e22d4139af0da
parent 43456 be6215c3b21d60f97588fc237ac557bd5568406b
child 43458 5151a411e76f5ca9ccc3094057c774accf72f6a0
push id23074
push usermkmelin@iki.fi
push dateSat, 28 Dec 2024 09:50:23 +0000 (6 months ago)
treeherdercomm-central@fa15d6b1fc22 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgds, tobyp
bugs1901846, 1864776
Bug 1901846 - "Done compacting (approx. XX KB saved)" is overwritten by "Loading message..." when message pane is visible. r=gds,tobyp This changes the status bar output to have FIFO queue (max size 10) of status messages. If there's a queue, next message will be shown in 750ms, and then moving on the the next one until the queue is empty. This allows the status messages to be seen before next message overwrites the previous one. There are situations where there may be a whole lot of updates, in which case it seems ok to just drop those messages (see bug 1864776). It just gets confusing in case the status updates are way late. Differential Revision: https://phabricator.services.mozilla.com/D232320
mail/base/content/mailWindow.js
mail/base/test/browser/browser_statusFeedback.js
mailnews/base/content/subscribe.js
mailnews/base/public/nsIMsgStatusFeedback.idl
mailnews/base/src/FolderCompactor.cpp
mailnews/base/src/nsMsgProgress.cpp
mailnews/base/src/nsMsgStatusFeedback.cpp
--- a/mail/base/content/mailWindow.js
+++ b/mail/base/content/mailWindow.js
@@ -309,16 +309,17 @@ function nsMsgStatusFeedback() {
   this._statusText = document.getElementById("statusText");
   this._statusPanel = document.getElementById("statusbar-display");
   this._progressBar = document.getElementById("statusbar-icon");
   this._progressBarContainer = document.getElementById(
     "statusbar-progresspanel"
   );
   this._throbber = document.getElementById("throbber-box");
   this._activeProcesses = [];
+  this._statusQueue = [];
 
   // make sure the stop button is accurate from the get-go
   goUpdateCommand("cmd_stop");
 }
 
 /**
  * @implements {nsIMsgStatusFeedback}
  * @implements {nsIXULBrowserWindow}
@@ -335,22 +336,21 @@ nsMsgStatusFeedback.prototype = {
   _throbber: null,
 
   // Member variables.
   _startTimeoutID: null,
   _stopTimeoutID: null,
   // How many start meteors have been requested.
   _startRequests: 0,
   _meteorsSpinning: false,
-  _defaultStatusText: "",
   _progressBarVisible: false,
   _activeProcesses: null,
   _statusFeedbackProgress: -1,
   _statusLastShown: 0,
-  _lastStatusText: null,
+  _statusQueue: null,
   _timeoutDelay: ChromeUtils.isInAutomation ? 50 : 500,
 
   // unload - call to remove links to listeners etc.
   unload() {
     // Remove listeners for any active processes we have hooked ourselves into.
     this._activeProcesses.forEach(function (element) {
       element.removeListener(this);
     }, this);
@@ -430,60 +430,62 @@ nsMsgStatusFeedback.prototype = {
   QueryInterface: ChromeUtils.generateQI([
     "nsIMsgStatusFeedback",
     "nsIXULBrowserWindow",
     "nsIActivityMgrListener",
     "nsIActivityListener",
     "nsISupportsWeakReference",
   ]),
 
-  // nsIMsgStatusFeedback implementation.
+  /**
+   * @param {string} statusText - The status string to display.
+   * @see {nsIMsgStatusFeedback}
+   */
   showStatusString(statusText) {
-    if (!statusText) {
-      statusText = this._defaultStatusText;
-    } else {
-      this._defaultStatusText = "";
-    }
     // Let's make sure the display doesn't flicker.
-    const timeBetweenDisplay = 500;
-    const now = Date.now();
-    if (now - this._statusLastShown > timeBetweenDisplay) {
-      // Cancel any pending status message. The timeout is not guaranteed
-      // to run within timeBetweenDisplay milliseconds.
-      this._lastStatusText = null;
-
-      this._statusLastShown = now;
+    const MIN_DISPLAY_TIME = 750;
+    if (
+      !this._statusIntervalId &&
+      Date.now() - this._statusLastShown > MIN_DISPLAY_TIME
+    ) {
+      this._statusLastShown = Date.now();
       if (this._statusText.value != statusText) {
         this._statusText.value = statusText;
       }
-    } else {
-      if (this._lastStatusText !== null) {
-        // There's already a pending display. Replace it.
-        this._lastStatusText = statusText;
+      return;
+    }
+    if (this._statusQueue.length >= 10) {
+      // Drop further messages until the queue has cleared up.
+      return;
+    }
+    if (!statusText && this._statusQueue.length > 0) {
+      // Don't queue empty strings in the middle.
+      return;
+    }
+    this._statusQueue.push(statusText);
+    if (this._statusIntervalId) {
+      return;
+    }
+
+    // Arrange for this to be shown in MIN_DISPLAY_TIME ms.
+    this._statusIntervalId = setInterval(() => {
+      const text = this._statusQueue.shift();
+      if (text === undefined) {
+        clearInterval(this._statusIntervalId);
+        this._statusIntervalId = null;
+        if (!this._meteorsSpinning) {
+          this._statusText.value = ""; // Clear status text once done.
+        }
         return;
       }
-      // Arrange for this to be shown in timeBetweenDisplay milliseconds.
-      this._lastStatusText = statusText;
-      setTimeout(() => {
-        if (this._lastStatusText !== null) {
-          this._statusLastShown = Date.now();
-          if (this._statusText.value != this._lastStatusText) {
-            this._statusText.value = this._lastStatusText;
-          }
-          this._lastStatusText = null;
-        }
-      }, timeBetweenDisplay);
-    }
-  },
-
-  setStatusString(status) {
-    if (status.length > 0) {
-      this._defaultStatusText = status;
-      this._statusText.value = status;
-    }
+      this._statusLastShown = Date.now();
+      if (this._statusText.value != text) {
+        this._statusText.value = text;
+      }
+    }, MIN_DISPLAY_TIME);
   },
 
   _startMeteors() {
     this._meteorsSpinning = true;
     this._startTimeoutID = null;
 
     // Turn progress meter on.
     this.updateProgress();
@@ -518,17 +520,17 @@ nsMsgStatusFeedback.prototype = {
     // a stop timeout...
     if (this._stopTimeoutID) {
       clearTimeout(this._stopTimeoutID);
       this._stopTimeoutID = null;
     }
   },
 
   _stopMeteors() {
-    this.showStatusString(this._defaultStatusText);
+    this.showStatusString("");
 
     // stop the throbber
     if (this._throbber) {
       this._throbber.setAttribute("busy", false);
     }
 
     document.querySelector(".throbber")?.classList.remove("busy");
 
--- a/mail/base/test/browser/browser_statusFeedback.js
+++ b/mail/base/test/browser/browser_statusFeedback.js
@@ -64,8 +64,42 @@ add_task(async function testMessageOpen(
   );
 
   // Check that the status message eventually reset
   await TestUtils.waitForCondition(
     () => statusText.value == "",
     "status message should eventually reset"
   );
 });
+
+add_task(async function testManyStatuses() {
+  const statusFeedback = window.MsgStatusFeedback;
+  const statuses = [];
+  for (let i = 0; i < 25; i++) {
+    const str = `Hey hey hey #${i}`;
+    if (i < 10) {
+      // after 10 messages, messages should get dropped
+      statuses.push(
+        BrowserTestUtils.waitForMutationCondition(
+          statusText,
+          {
+            attributes: true,
+            attributeFilter: ["value"],
+          },
+          () => statusText.value == str
+        )
+      );
+    }
+    statusFeedback.showStatusString(str);
+  }
+  await Promise.all(statuses);
+  Assert.ok(true, `The ${statuses.length} first statuses should be shown`);
+
+  // After that, it should clear up.
+  await BrowserTestUtils.waitForMutationCondition(
+    statusText,
+    {
+      attributes: true,
+      attributeFilter: ["value"],
+    },
+    () => statusText.value == ""
+  );
+});
--- a/mailnews/base/content/subscribe.js
+++ b/mailnews/base/content/subscribe.js
@@ -109,32 +109,31 @@ function SetUpTree(forceToServer, getOnl
     gSubscribeTree.view = gSubscribableServer.folderView;
     gSubscribableServer.subscribeListener = MySubscribeListener;
 
     document.getElementById("currentListTab").disabled = true;
     document.getElementById("newGroupsTab").disabled = true;
     document.getElementById("refreshButton").disabled = true;
 
     gStatusFeedback._startMeteors();
-    gStatusFeedback.setStatusString("");
     gStatusFeedback.showStatusString(
       gSubscribeBundle.getString("pleaseWaitString")
     );
     document.getElementById("stopButton").disabled = false;
 
     gSubscribableServer.startPopulating(msgWindow, forceToServer, getOnlyNew);
   } catch (e) {
     if (e.result == 0x80550014) {
       // NS_MSG_ERROR_OFFLINE
-      gStatusFeedback.setStatusString(
+      gStatusFeedback.showStatusString(
         gSubscribeBundle.getString("offlineState")
       );
     } else {
       console.error("Failed to populate subscribe tree: ", e);
-      gStatusFeedback.setStatusString(
+      gStatusFeedback.showStatusString(
         gSubscribeBundle.getString("errorPopulating")
       );
     }
     Stop();
   }
 }
 
 function SubscribeOnUnload() {
--- a/mailnews/base/public/nsIMsgStatusFeedback.idl
+++ b/mailnews/base/public/nsIMsgStatusFeedback.idl
@@ -2,17 +2,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
 
 [scriptable, uuid(AACBFA34-8D29-4A08-9283-A8E5B3AB067F)]
 interface nsIMsgStatusFeedback : nsISupports {
+
   void showStatusString(in AString aStatus);
   void startMeteors();
   void stopMeteors();
   void showProgress(in long aPercent);
-  void setStatusString(in AString aStatus); // will be displayed until next user action
 
   /* aStatusFeedback: a wrapped JS status feedback object */
   void setWrappedStatusFeedback(in nsIMsgStatusFeedback aStatusFeedback);
 };
--- a/mailnews/base/src/FolderCompactor.cpp
+++ b/mailnews/base/src/FolderCompactor.cpp
@@ -787,17 +787,17 @@ static void GUIShowCompactingMsg(nsIMsgW
   NS_ENSURE_SUCCESS_VOID(rv);
 
   // Show message and turn on the progress bar.
   nsCOMPtr<nsIMsgStatusFeedback> feedback;
   window->GetStatusFeedback(getter_AddRefs(feedback));
   if (feedback) {
     // Not all windows have .statusFeedback set, especially during
     // xpcshell-tests (search for gDummyMsgWindow, set up in alertTestUtils.js).
-    feedback->SetStatusString(statusMessage);
+    feedback->ShowStatusString(statusMessage);
     feedback->StartMeteors();
   }
 }
 
 static void GUIShowDoneMsg(nsIMsgWindow* window, int64_t totalBytesRecovered) {
   MOZ_ASSERT(window);
   nsresult rv;
 
@@ -818,17 +818,17 @@ static void GUIShowDoneMsg(nsIMsgWindow*
   AutoTArray<nsString, 1> params = {amount};
   rv = bundle->FormatStringFromName("compactingDone", params, doneMsg);
   NS_ENSURE_SUCCESS_VOID(rv);
 
   // Show message, and turn off progress bar.
   nsCOMPtr<nsIMsgStatusFeedback> feedback;
   window->GetStatusFeedback(getter_AddRefs(feedback));
   if (feedback) {
-    feedback->SetStatusString(doneMsg);
+    feedback->ShowStatusString(doneMsg);
     feedback->StopMeteors();
   }
 }
 
 // This is the sole public-facing function.
 nsresult AsyncCompactFolders(nsTArray<RefPtr<nsIMsgFolder>> const& folders,
                              nsIUrlListener* listener, nsIMsgWindow* window) {
   // Filter out noncompactable folders and ones with zero .expungedBytes.
--- a/mailnews/base/src/nsMsgProgress.cpp
+++ b/mailnews/base/src/nsMsgProgress.cpp
@@ -187,21 +187,16 @@ nsresult nsMsgProgress::ReleaseListeners
   return NS_OK;
 }
 
 NS_IMETHODIMP nsMsgProgress::ShowStatusString(const nsAString& aStatus) {
   return OnStatusChange(nullptr, nullptr, NS_OK,
                         PromiseFlatString(aStatus).get());
 }
 
-NS_IMETHODIMP nsMsgProgress::SetStatusString(const nsAString& aStatus) {
-  return OnStatusChange(nullptr, nullptr, NS_OK,
-                        PromiseFlatString(aStatus).get());
-}
-
 NS_IMETHODIMP nsMsgProgress::StartMeteors() { return NS_ERROR_NOT_IMPLEMENTED; }
 
 NS_IMETHODIMP nsMsgProgress::StopMeteors() { return NS_ERROR_NOT_IMPLEMENTED; }
 
 NS_IMETHODIMP nsMsgProgress::ShowProgress(int32_t percent) {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
--- a/mailnews/base/src/nsMsgStatusFeedback.cpp
+++ b/mailnews/base/src/nsMsgStatusFeedback.cpp
@@ -149,24 +149,16 @@ NS_IMETHODIMP
 nsMsgStatusFeedback::ShowStatusString(const nsAString& aStatus) {
   nsCOMPtr<nsIMsgStatusFeedback> jsStatusFeedback(
       do_QueryReferent(mJSStatusFeedbackWeak));
   if (jsStatusFeedback) jsStatusFeedback->ShowStatusString(aStatus);
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsMsgStatusFeedback::SetStatusString(const nsAString& aStatus) {
-  nsCOMPtr<nsIMsgStatusFeedback> jsStatusFeedback(
-      do_QueryReferent(mJSStatusFeedbackWeak));
-  if (jsStatusFeedback) jsStatusFeedback->SetStatusString(aStatus);
-  return NS_OK;
-}
-
-NS_IMETHODIMP
 nsMsgStatusFeedback::ShowProgress(int32_t aPercentage) {
   // If the percentage hasn't changed...OR if we are going from 0 to 100% in one
   // step then don't bother....just fall out....
   if (aPercentage == m_lastPercent ||
       (m_lastPercent == 0 && aPercentage >= 100))
     return NS_OK;
 
   // Throttle updates.