Closed
Bug 1381409
Opened 7 years ago
Closed 7 years ago
Use DownloadList notifications directly in front-end download views
Categories
(Firefox :: Downloads Panel, enhancement, P1)
Firefox
Downloads Panel
Tracking
()
RESOLVED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: Paolo, Assigned: Paolo)
References
Details
Attachments
(3 files, 2 obsolete files)
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.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8886963 -
Attachment is obsolete: true
Attachment #8886963 -
Flags: review?(mak77)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8886992 -
Attachment is obsolete: true
Attachment #8886992 -
Flags: review?(mak77)
Assignee | ||
Comment 4•7 years 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•7 years ago
|
Attachment #8886992 -
Attachment is obsolete: true
Attachment #8886992 -
Flags: review?(mak77)
Assignee | ||
Updated•7 years ago
|
Priority: -- → P1
Comment 7•7 years 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•7 years 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•7 years 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•7 years 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•7 years 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
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in
before you can comment on or make changes to this bug.
Description
•