Use DownloadList notifications directly in front-end download views

RESOLVED FIXED in Firefox 56

Status

()

Firefox
Downloads Panel
P1
normal
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: Paolo, Assigned: Paolo)

Tracking

unspecified
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

a year ago
The DownloadsData front-end object currently relays the notifications from DownloadList to the front-end views. This indirection can be avoided now that there is only one supported back-end.
(Assignee)

Updated

a year ago
Blocks: 1381411
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8886963 - Attachment is obsolete: true
Attachment #8886963 - Flags: review?(mak77)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8886992 - Attachment is obsolete: true
Attachment #8886992 - Flags: review?(mak77)
(Assignee)

Comment 4

a year ago
Comment on attachment 8886992 [details]
Bug 1381409 - Part 2 - Let each view keep the state of downloads relevant to it.

Well, it seems that today MozReview won't let me push multiple changesets.
Attachment #8886992 - Attachment is obsolete: false
Attachment #8886992 - Flags: review?(mak77)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8886992 - Attachment is obsolete: true
Attachment #8886992 - Flags: review?(mak77)
(Assignee)

Updated

a year ago
Priority: -- → P1

Comment 7

a year ago
mozreview-review
Comment on attachment 8886996 [details]
Bug 1381409 - Part 1 - Always add Download objects in their original order.

https://reviewboard.mozilla.org/r/157756/#review163434

We still don't have lots of test coverage over the downloads views right?
Attachment #8886996 - Flags: review?(mak77) → review+

Comment 8

a year ago
mozreview-review
Comment on attachment 8887022 [details]
Bug 1381409 - Part 2 - Let each view keep the state of downloads relevant to it.

https://reviewboard.mozilla.org/r/157770/#review163442

::: browser/components/downloads/DownloadsCommon.jsm:1043
(Diff revision 1)
>     * including progress properties.
>     *
>     * Note that progress notification changes are throttled at the Downloads.jsm
>     * API level, and there is no throttling mechanism in the front-end.
>     *
>     * @note Subclasses should override this.

maybe change these comments (also above onDownloadAdded) saying that they should override this but still call into it if they need the basic functionality?

::: browser/components/downloads/DownloadsCommon.jsm:1368
(Diff revision 1)
>      DownloadsViewPrototype.onDataLoadCompleted.call(this);
>      this._updateViews();
>    },
>  
>    onDownloadAdded(download) {
> +    DownloadsViewPrototype.onDownloadAdded.call(this, download);

Sounds like we may have luck converting these to actual ES6 classes (then we could use super.onDownloadAdded and avoid the __proto__ hack)

Maybe it could be a good mentored bug.
Attachment #8887022 - Flags: review?(mak77) → review+

Comment 9

a year ago
mozreview-review
Comment on attachment 8887023 [details]
Bug 1381409 - Part 3 - Use DownloadList notifications directly in front-end download views.

https://reviewboard.mozilla.org/r/157772/#review163476
Attachment #8887023 - Flags: review?(mak77) → review+

Comment 10

a year ago
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7bf2876b0ffd
Part 1 - Always add Download objects in their original order. r=mak
https://hg.mozilla.org/integration/mozilla-inbound/rev/a529720f06ee
Part 2 - Let each view keep the state of downloads relevant to it. r=mak
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a2fd40a1239
Part 3 - Use DownloadList notifications directly in front-end download views. r=mak

Comment 11

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7bf2876b0ffd
https://hg.mozilla.org/mozilla-central/rev/a529720f06ee
https://hg.mozilla.org/mozilla-central/rev/4a2fd40a1239
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
(Assignee)

Updated

11 months ago
Depends on: 1393418
You need to log in before you can comment on or make changes to this bug.