Use the new back-end property to get the size of downloads asynchronously

RESOLVED FIXED in Firefox 38

Status

()

Firefox
Downloads Panel
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Paolo, Assigned: Paolo)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 38
Points:
3
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify -

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Created attachment 8557121 [details] [diff] [review]
The patch

We can now use the DownloadTarget.size property and the Download.refresh methods to get information about the size of the download on disk asynchronously.
Attachment #8557121 - Flags: feedback?(mak77)
(Assignee)

Updated

3 years ago
Points: --- → 3
Comment on attachment 8557121 [details] [diff] [review]
The patch

Review of attachment 8557121 [details] [diff] [review]:
-----------------------------------------------------------------

it looks reasonable, and I hated maxBytes from the beginning.

::: browser/components/downloads/DownloadsCommon.jsm
@@ +366,2 @@
>  
> +      if (!download.stopped) {

looks like you removed usage of numScanning, should also remove its definition.

::: browser/components/downloads/content/downloadsViewCommon.js
@@ +172,5 @@
>      let text = "";
>      let tip = "";
>  
>      if (!this.download.stopped) {
> +      let total = this.download.hasProgress ? this.download.totalBytes : -1;

totalBytes (let's keep the unit). is at least slightly clearer than maxBytes.

@@ +206,5 @@
>        if (this.download.succeeded) {
> +        // For completed downloads, show the file size (e.g. "1.5 MB").
> +        if (this.download.target.size !== undefined) {
> +          let [size, unit] = DownloadUtils.convertByteUnits(
> +                                                  this.download.target.size);

nit: I prefer this form:
let [size, unit] =
  DownloadUtils.convertByteUnits(this.download.target.size);
Attachment #8557121 - Flags: feedback?(mak77) → feedback+
(Assignee)

Updated

3 years ago
Blocks: 1129896
(Assignee)

Updated

3 years ago
Attachment #8557121 - Flags: feedback?(smacleod)
Attachment #8557121 - Flags: feedback?(jaws)
(Assignee)

Comment 2

3 years ago
Comment on attachment 8557121 [details] [diff] [review]
The patch

Review comments addressed in bug 1129896.
Attachment #8557121 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/acb012724bf0
Assignee: nobody → paolo.mozmail
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38

Updated

3 years ago
Iteration: --- → 38.3 - 23 Feb
Flags: qe-verify-
Flags: firefox-backlog+
Attachment #8557121 - Flags: feedback?(jaws) → feedback+
Attachment #8557121 - Flags: feedback?(smacleod) → feedback+

Comment 5

3 years ago
Comment on attachment 8557121 [details] [diff] [review]
The patch

>+++ b/browser/components/downloads/content/allDownloadsViewOverlay.js

>+      // These properties may be updated if the user interface is refreshed.
>+      this.exists = false;
Should this be:
this.target.exists = false;
instead?
>+      this.target.size = undefined;
>     }
Flags: needinfo?(paolo.mozmail)
(Assignee)

Comment 6

3 years ago
(In reply to Ian Neal from comment #5)
> Should this be:
> this.target.exists = false;
> instead?

Good catch! Mind to file a new bug to fix the typo? :-)
Flags: needinfo?(paolo.mozmail)

Updated

3 years ago
Blocks: 1195279
You need to log in before you can comment on or make changes to this bug.