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

RESOLVED FIXED in Firefox 38

Status

()

defect
RESOLVED FIXED
4 years ago
4 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

4 years ago
Posted patch The patchSplinter Review
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

4 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

4 years ago
Blocks: 1129896
Assignee

Updated

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

Comment 2

4 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
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
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

4 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

4 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

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