Bug 759397 - Remove per-item download rate clutter from Downloads Panel. r=mak.

browser/components/downloads/content/download.xml

@@ -33,6 +33,7 @@
                            max="100"
                            xbl:inherits="mode=progressmode,value=progress"/>
         <xul:description class="downloadDetails"
+                         style="width: &downloadDetails.width;"
                          crop="end"
                          xbl:inherits="value=status,tooltiptext=statusTip"/>
       </xul:vbox>

browser/components/downloads/content/downloads.js

@@ -921,8 +921,18 @@ DownloadsViewItem.prototype = {
                                             DownloadsCommon.strings.statePaused,
                                             transfer);
     } else if (this.dataItem.state == nsIDM.DOWNLOAD_DOWNLOADING) {
+      // We don't show the rate for each download in order to reduce clutter.
+      // The remaining time per download is likely enough information for the
+      // panel.
+      [status] =
+        DownloadUtils.getDownloadStatusNoRate(this.dataItem.currBytes,
+                                              this.dataItem.maxBytes,
+                                              this.dataItem.speed,
+                                              this.lastEstimatedSecondsLeft);
+
+      // We are, however, OK with displaying the rate in the tooltip.
       let newEstimatedSecondsLeft;
-      [status, newEstimatedSecondsLeft] =
+      [statusTip, newEstimatedSecondsLeft] =
         DownloadUtils.getDownloadStatus(this.dataItem.currBytes,
                                         this.dataItem.maxBytes,
                                         this.dataItem.speed,

browser/components/downloads/content/downloadsOverlay.xul

@@ -98,7 +98,6 @@
       </menupopup>
 
       <richlistbox id="downloadsListBox"
-                   style="width: &downloads.width;"
                    class="plain"
                    flex="1"
                    context="downloadsContextMenu"

browser/locales/en-US/chrome/browser/downloads/downloads.dtd

@@ -12,12 +12,23 @@
      -->
 <!ENTITY downloads.title                  "Downloads">
 
-<!-- LOCALIZATION NOTE (downloads.width):
+<!-- LOCALIZATION NOTE (downloadDetails.width):
-     Width of the Downloads Panel list items, expressed using a CSS unit.  The
+     Width of details for a Downloads Panel item (which directly influences the
-     longest labels that should fit in the item width are usually those of
+     width of the Downloads Panel) expressed using a CSS unit. The longest
-     in-progress downloads and those of blocked downloads.
+     labels that should fit in the item width are usually those of in-progress
+     downloads and those of blocked downloads.
+
+     A good rule of thumb is to try to determine the longest string possible
+     that an in-progress download could display, and use that value in ch
+     units.
+
+     For example, in English, a long string would be:
+
+     59 minutes, 59 seconds remaining - 1022 of 1023 KB
+
+     That's 50 characters, so we set the width at 50ch.
      -->
-<!ENTITY downloads.width                  "65ch">
+<!ENTITY downloadDetails.width            "50ch">
 
 <!ENTITY cmd.pause.label                  "Pause">
 <!ENTITY cmd.pause.accesskey              "P">

toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties

@@ -50,6 +50,12 @@ downloadsCompleteMsg=All files have finished downloading.
 # %1$S transfer progress; %2$S rate number; %3$S rate unit; %4$S time left
 # example: 4 minutes left — 1.1 of 11.1 GB (2.2 MB/sec)
 statusFormat3=%4$S — %1$S (%2$S %3$S/sec)
+
+# LOCALIZATION NOTE (statusFormatNoRate): — is the "em dash" (long dash)
+# %1$S transfer progress; %2$S time left
+# example: 4 minutes left — 1.1 of 11.1 GB
+statusFormatNoRate=%2$S — %1$S
+
 bytes=bytes
 kilobyte=KB
 megabyte=MB

toolkit/mozapps/downloads/DownloadUtils.jsm

@@ -55,6 +55,7 @@ const kDownloadProperties =
 
 let gStr = {
   statusFormat: "statusFormat3",
+  statusFormatNoRate: "statusFormatNoRate",
   transferSameUnits: "transferSameUnits2",
   transferDiffUnits: "transferDiffUnits2",
   transferNoTotal: "transferNoTotal2",
@@ -102,6 +103,63 @@ this.DownloadUtils = {
    */
   getDownloadStatus: function DU_getDownloadStatus(aCurrBytes, aMaxBytes,
                                                    aSpeed, aLastSec)
+  {
+    let [transfer, timeLeft, newLast, normalizedSpeed]
+      = this._deriveTransferRate(aCurrBytes, aMaxBytes, aSpeed, aLastSec);
+
+    let [rate, unit] = DownloadUtils.convertByteUnits(normalizedSpeed);
+    let params = [transfer, rate, unit, timeLeft];
+    let status = gBundle.formatStringFromName(gStr.statusFormat, params,
+                                              params.length);
+    return [status, newLast];
+  },
+
+  /**
+   * Generate a status string for a download given its current progress,
+   * total size, speed, last time remaining. The status string contains the
+   * time remaining, as well as the total bytes downloaded. Unlike
+   * getDownloadStatus, it does not include the rate of download.
+   *
+   * @param aCurrBytes
+   *        Number of bytes transferred so far
+   * @param [optional] aMaxBytes
+   *        Total number of bytes or -1 for unknown
+   * @param [optional] aSpeed
+   *        Current transfer rate in bytes/sec or -1 for unknown
+   * @param [optional] aLastSec
+   *        Last time remaining in seconds or Infinity for unknown
+   * @return A pair: [download status text, new value of "last seconds"]
+   */
+  getDownloadStatusNoRate:
+  function DU_getDownloadStatusNoRate(aCurrBytes, aMaxBytes, aSpeed,
+                                      aLastSec)
+  {
+    let [transfer, timeLeft, newLast]
+      = this._deriveTransferRate(aCurrBytes, aMaxBytes, aSpeed, aLastSec);
+
+    let params = [transfer, timeLeft];
+    let status = gBundle.formatStringFromName(gStr.statusFormatNoRate, params,
+                                              params.length);
+    return [status, newLast];
+  },
+
+  /**
+   * Helper function that returns a transfer string, a time remaining string,
+   * and a new value of "last seconds".
+   * @param aCurrBytes
+   *        Number of bytes transferred so far
+   * @param [optional] aMaxBytes
+   *        Total number of bytes or -1 for unknown
+   * @param [optional] aSpeed
+   *        Current transfer rate in bytes/sec or -1 for unknown
+   * @param [optional] aLastSec
+   *        Last time remaining in seconds or Infinity for unknown
+   * @return A triple: [amount transferred string, time remaining string,
+   *                    new value of "last seconds"]
+   */
+  _deriveTransferRate: function DU__deriveTransferRate(aCurrBytes,
+                                                       aMaxBytes, aSpeed,
+                                                       aLastSec)
   {
     if (aMaxBytes == null)
       aMaxBytes = -1;
@@ -115,13 +173,8 @@ this.DownloadUtils = {
       (aMaxBytes - aCurrBytes) / aSpeed : -1;
 
     let transfer = DownloadUtils.getTransferTotal(aCurrBytes, aMaxBytes);
-    let [rate, unit] = DownloadUtils.convertByteUnits(aSpeed);
     let [timeLeft, newLast] = DownloadUtils.getTimeLeft(seconds, aLastSec);
-
+    return [transfer, timeLeft, newLast, aSpeed];
-    let params = [transfer, rate, unit, timeLeft];
-    let status = gBundle.formatStringFromName(gStr.statusFormat, params,
-                                              params.length);
-    return [status, newLast];
   },
 
   /**

toolkit/mozapps/downloads/tests/unit/test_DownloadUtils.js

@@ -28,15 +28,14 @@ let gDash = DownloadUtils.getDownloadStatus(0)[0].match(/remaining (.) 0 bytes/)
 
 let gVals = [0, 100, 2345, 55555, 982341, 23194134, 1482, 58, 9921949201, 13498132];
 
-function testStatus(aCurr, aMore, aRate, aTest)
+function testStatus(aFunc, aCurr, aMore, aRate, aTest)
 {
   dump("Status Test: " + [aCurr, aMore, aRate, aTest] + "\n");
   let curr = gVals[aCurr];
   let max = curr + gVals[aMore];
   let speed = gVals[aRate];
 
-  let [status, last] =
+  let [status, last] = aFunc(curr, max, speed);
-    DownloadUtils.getDownloadStatus(curr, max, speed);
 
   if (0) {
     dump("testStatus(" + aCurr + ", " + aMore + ", " + aRate + ", [\"" +
@@ -135,35 +134,72 @@ function run_test()
     }
   }
 
-  testStatus(2, 1, 7, ["A few seconds remaining -- 2.3 of 2.4 KB (58 bytes/sec)", 1.724]);
+  // First, test with rates, via getDownloadStatus...
-  testStatus(1, 2, 6, ["A few seconds remaining -- 100 bytes of 2.4 KB (1.4 KB/sec)", 1.582]);
+  let statusFunc = DownloadUtils.getDownloadStatus.bind(DownloadUtils);
-  testStatus(4, 3, 9, ["A few seconds remaining -- 959 KB of 1.0 MB (12.9 MB/sec)", 0.004]);
-  testStatus(2, 3, 8, ["A few seconds remaining -- 2.3 of 56.5 KB (9.2 GB/sec)", 0.000]);
 
-  testStatus(8, 4, 3, ["17 seconds remaining -- 9.2 of 9.2 GB (54.3 KB/sec)", 17.682]);
+  testStatus(statusFunc, 2, 1, 7, ["A few seconds remaining -- 2.3 of 2.4 KB (58 bytes/sec)", 1.724]);
-  testStatus(1, 3, 2, ["23 seconds remaining -- 100 bytes of 54.4 KB (2.3 KB/sec)", 23.691]);
+  testStatus(statusFunc, 1, 2, 6, ["A few seconds remaining -- 100 bytes of 2.4 KB (1.4 KB/sec)", 1.582]);
-  testStatus(9, 3, 2, ["23 seconds remaining -- 12.9 of 12.9 MB (2.3 KB/sec)", 23.691]);
+  testStatus(statusFunc, 4, 3, 9, ["A few seconds remaining -- 959 KB of 1.0 MB (12.9 MB/sec)", 0.004]);
-  testStatus(5, 6, 7, ["25 seconds remaining -- 22.1 of 22.1 MB (58 bytes/sec)", 25.552]);
+  testStatus(statusFunc, 2, 3, 8, ["A few seconds remaining -- 2.3 of 56.5 KB (9.2 GB/sec)", 0.000]);
 
-  testStatus(3, 9, 3, ["4 minutes remaining -- 54.3 KB of 12.9 MB (54.3 KB/sec)", 242.969]);
+  testStatus(statusFunc, 8, 4, 3, ["17 seconds remaining -- 9.2 of 9.2 GB (54.3 KB/sec)", 17.682]);
-  testStatus(2, 3, 1, ["9 minutes remaining -- 2.3 of 56.5 KB (100 bytes/sec)", 555.550]);
+  testStatus(statusFunc, 1, 3, 2, ["23 seconds remaining -- 100 bytes of 54.4 KB (2.3 KB/sec)", 23.691]);
-  testStatus(4, 3, 7, ["15 minutes remaining -- 959 KB of 1.0 MB (58 bytes/sec)", 957.845]);
+  testStatus(statusFunc, 9, 3, 2, ["23 seconds remaining -- 12.9 of 12.9 MB (2.3 KB/sec)", 23.691]);
-  testStatus(5, 3, 7, ["15 minutes remaining -- 22.1 of 22.2 MB (58 bytes/sec)", 957.845]);
+  testStatus(statusFunc, 5, 6, 7, ["25 seconds remaining -- 22.1 of 22.1 MB (58 bytes/sec)", 25.552]);
 
-  testStatus(1, 9, 2, ["1 hour, 35 minutes remaining -- 100 bytes of 12.9 MB (2.3 KB/sec)", 5756.133]);
+  testStatus(statusFunc, 3, 9, 3, ["4 minutes remaining -- 54.3 KB of 12.9 MB (54.3 KB/sec)", 242.969]);
-  testStatus(2, 9, 6, ["2 hours, 31 minutes remaining -- 2.3 KB of 12.9 MB (1.4 KB/sec)", 9108.051]);
+  testStatus(statusFunc, 2, 3, 1, ["9 minutes remaining -- 2.3 of 56.5 KB (100 bytes/sec)", 555.550]);
-  testStatus(2, 4, 1, ["2 hours, 43 minutes remaining -- 2.3 of 962 KB (100 bytes/sec)", 9823.410]);
+  testStatus(statusFunc, 4, 3, 7, ["15 minutes remaining -- 959 KB of 1.0 MB (58 bytes/sec)", 957.845]);
-  testStatus(6, 4, 7, ["4 hours, 42 minutes remaining -- 1.4 of 961 KB (58 bytes/sec)", 16936.914]);
+  testStatus(statusFunc, 5, 3, 7, ["15 minutes remaining -- 22.1 of 22.2 MB (58 bytes/sec)", 957.845]);
 
-  testStatus(6, 9, 1, ["1 day, 13 hours remaining -- 1.4 KB of 12.9 MB (100 bytes/sec)", 134981.320]);
+  testStatus(statusFunc, 1, 9, 2, ["1 hour, 35 minutes remaining -- 100 bytes of 12.9 MB (2.3 KB/sec)", 5756.133]);
-  testStatus(3, 8, 3, ["2 days, 1 hour remaining -- 54.3 KB of 9.2 GB (54.3 KB/sec)", 178596.872]);
+  testStatus(statusFunc, 2, 9, 6, ["2 hours, 31 minutes remaining -- 2.3 KB of 12.9 MB (1.4 KB/sec)", 9108.051]);
-  testStatus(1, 8, 6, ["77 days, 11 hours remaining -- 100 bytes of 9.2 GB (1.4 KB/sec)", 6694972.470]);
+  testStatus(statusFunc, 2, 4, 1, ["2 hours, 43 minutes remaining -- 2.3 of 962 KB (100 bytes/sec)", 9823.410]);
-  testStatus(6, 8, 7, ["1979 days, 22 hours remaining -- 1.4 KB of 9.2 GB (58 bytes/sec)", 171068089.672]);
+  testStatus(statusFunc, 6, 4, 7, ["4 hours, 42 minutes remaining -- 1.4 of 961 KB (58 bytes/sec)", 16936.914]);
 
-  testStatus(0, 0, 5, ["Unknown time remaining -- 0 of 0 bytes (22.1 MB/sec)", Infinity]);
+  testStatus(statusFunc, 6, 9, 1, ["1 day, 13 hours remaining -- 1.4 KB of 12.9 MB (100 bytes/sec)", 134981.320]);
-  testStatus(0, 6, 0, ["Unknown time remaining -- 0 bytes of 1.4 KB (0 bytes/sec)", Infinity]);
+  testStatus(statusFunc, 3, 8, 3, ["2 days, 1 hour remaining -- 54.3 KB of 9.2 GB (54.3 KB/sec)", 178596.872]);
-  testStatus(6, 6, 0, ["Unknown time remaining -- 1.4 of 2.9 KB (0 bytes/sec)", Infinity]);
+  testStatus(statusFunc, 1, 8, 6, ["77 days, 11 hours remaining -- 100 bytes of 9.2 GB (1.4 KB/sec)", 6694972.470]);
-  testStatus(8, 5, 0, ["Unknown time remaining -- 9.2 of 9.3 GB (0 bytes/sec)", Infinity]);
+  testStatus(statusFunc, 6, 8, 7, ["1979 days, 22 hours remaining -- 1.4 KB of 9.2 GB (58 bytes/sec)", 171068089.672]);
+
+  testStatus(statusFunc, 0, 0, 5, ["Unknown time remaining -- 0 of 0 bytes (22.1 MB/sec)", Infinity]);
+  testStatus(statusFunc, 0, 6, 0, ["Unknown time remaining -- 0 bytes of 1.4 KB (0 bytes/sec)", Infinity]);
+  testStatus(statusFunc, 6, 6, 0, ["Unknown time remaining -- 1.4 of 2.9 KB (0 bytes/sec)", Infinity]);
+  testStatus(statusFunc, 8, 5, 0, ["Unknown time remaining -- 9.2 of 9.3 GB (0 bytes/sec)", Infinity]);
+
+
+  // Now test without rates, via getDownloadStatusNoRate.
+  statusFunc = DownloadUtils.getDownloadStatusNoRate.bind(DownloadUtils);
+
+  testStatus(statusFunc, 2, 1, 7, ["A few seconds remaining -- 2.3 of 2.4 KB", 1.724]);
+  testStatus(statusFunc, 1, 2, 6, ["A few seconds remaining -- 100 bytes of 2.4 KB", 1.582]);
+  testStatus(statusFunc, 4, 3, 9, ["A few seconds remaining -- 959 KB of 1.0 MB", 0.004]);
+  testStatus(statusFunc, 2, 3, 8, ["A few seconds remaining -- 2.3 of 56.5 KB", 0.000]);
+
+  testStatus(statusFunc, 8, 4, 3, ["17 seconds remaining -- 9.2 of 9.2 GB", 17.682]);
+  testStatus(statusFunc, 1, 3, 2, ["23 seconds remaining -- 100 bytes of 54.4 KB", 23.691]);
+  testStatus(statusFunc, 9, 3, 2, ["23 seconds remaining -- 12.9 of 12.9 MB", 23.691]);
+  testStatus(statusFunc, 5, 6, 7, ["25 seconds remaining -- 22.1 of 22.1 MB", 25.552]);
+
+  testStatus(statusFunc, 3, 9, 3, ["4 minutes remaining -- 54.3 KB of 12.9 MB", 242.969]);
+  testStatus(statusFunc, 2, 3, 1, ["9 minutes remaining -- 2.3 of 56.5 KB", 555.550]);
+  testStatus(statusFunc, 4, 3, 7, ["15 minutes remaining -- 959 KB of 1.0 MB", 957.845]);
+  testStatus(statusFunc, 5, 3, 7, ["15 minutes remaining -- 22.1 of 22.2 MB", 957.845]);
+
+  testStatus(statusFunc, 1, 9, 2, ["1 hour, 35 minutes remaining -- 100 bytes of 12.9 MB", 5756.133]);
+  testStatus(statusFunc, 2, 9, 6, ["2 hours, 31 minutes remaining -- 2.3 KB of 12.9 MB", 9108.051]);
+  testStatus(statusFunc, 2, 4, 1, ["2 hours, 43 minutes remaining -- 2.3 of 962 KB", 9823.410]);
+  testStatus(statusFunc, 6, 4, 7, ["4 hours, 42 minutes remaining -- 1.4 of 961 KB", 16936.914]);
+
+  testStatus(statusFunc, 6, 9, 1, ["1 day, 13 hours remaining -- 1.4 KB of 12.9 MB", 134981.320]);
+  testStatus(statusFunc, 3, 8, 3, ["2 days, 1 hour remaining -- 54.3 KB of 9.2 GB", 178596.872]);
+  testStatus(statusFunc, 1, 8, 6, ["77 days, 11 hours remaining -- 100 bytes of 9.2 GB", 6694972.470]);
+  testStatus(statusFunc, 6, 8, 7, ["1979 days, 22 hours remaining -- 1.4 KB of 9.2 GB", 171068089.672]);
+
+  testStatus(statusFunc, 0, 0, 5, ["Unknown time remaining -- 0 of 0 bytes", Infinity]);
+  testStatus(statusFunc, 0, 6, 0, ["Unknown time remaining -- 0 bytes of 1.4 KB", Infinity]);
+  testStatus(statusFunc, 6, 6, 0, ["Unknown time remaining -- 1.4 of 2.9 KB", Infinity]);
+  testStatus(statusFunc, 8, 5, 0, ["Unknown time remaining -- 9.2 of 9.3 GB", Infinity]);
 
   testURI("http://www.mozilla.org/", "mozilla.org", "www.mozilla.org");
   testURI("http://www.city.mikasa.hokkaido.jp/", "city.mikasa.hokkaido.jp", "www.city.mikasa.hokkaido.jp");