Closed Bug 833015 Opened 7 years ago Closed 7 years ago

Update Suite Download Manager UI for Private Browsing changes

Categories

(SeaMonkey :: Download & File Handling, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.18

People

(Reporter: neil, Assigned: neil)

References

Details

Attachments

(1 file, 4 obsolete files)

We need to update the suite download manager UI following some recent changes in the download manager UI interface.
Attached patch Proposed patch (obsolete) — Splinter Review
* Switched the 2nd parameter from an ID to an nsIDownload
* Added support for the aIsPrivate parameter - necessary to force progress
  dialogs because private downloads don't show in the download manager
* Disabled the "visible" attribute to force the backend to show downloads
  otherwise private downloads don't appear if the download manager is open
* Handled the "flash" pref after the "behaviour" pref - necessary now that
  the backend doesn't handle the "flash" pref any more
* Moved the controls in the pref pane around to reflect the new reality
* Renamed the "flash" entity

Code I used to test noninteractive download manager opening:

Components.classes['@mozilla.org/download-manager-ui;1']
          .getService(Components.interfaces.nsIDownloadManagerUI)
          .show(null, null, 1)

If you want to test actual private downloads, you'll need to build with per-window private browsing enabled and open a private browsing window:

window.openDialog(top.getBrowserURL(), '_blank',
                  'chrome,all,private,dialog=no', 'about:blank');
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #704586 - Flags: review?(philip.chee)
Attached patch With test fix (obsolete) — Splinter Review
I thought I might as well expose the existing recent window property. 
I also remembered to change the uuid. I also tweaked the indentation.
Attachment #704586 - Attachment is obsolete: true
Attachment #704586 - Flags: review?(philip.chee)
Attachment #706041 - Flags: review?(philip.chee)
Depends on: 830190
Attached patch Includes bug 830190 (obsolete) — Splinter Review
I realised that cmd_properties was broken and I couldn't fix it without merging in the changes that you've already seen in bug 830190 except for the tweak to the showProperties function and caller that's needed to fix cmd_properties.
Attachment #706041 - Attachment is obsolete: true
Attachment #706041 - Flags: review?(philip.chee)
Attachment #706353 - Flags: review?(philip.chee)
Attached image Screenshot of the download manager. (obsolete) —
Sat Jan 26 2013 00:21:46
Error: TypeError: dl.dld is undefined
Source file: chrome://communicator/content/downloads/treeView.js
Line: 156
Sat Jan 26 2013 00:33:40
Error: NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: '[JavaScript Error: "dl.dld is undefined" {file: "chrome://communicator/content/downloads/treeView.js" line: 207}]' when calling method: [nsITreeView::cycleCell]
Source file: chrome://global/content/bindings/tree.xml
Line: 1014
Sat Jan 26 2013 00:35:20
Error: NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: '[JavaScript Error: "aDownload is undefined" {file: "chrome://communicator/content/downloads/downloadmanager.js" line: 141}]' when calling method: [nsITreeView::cycleCell]
Source file: chrome://global/content/bindings/tree.xml
Line: 1014
Comment on attachment 706353 [details] [diff] [review]
Includes bug 830190

>   addDownload: function(aDownload) {
>     var attrs = {
>-      dlid: aDownload.id,
>+      guid: aDownload.guid,
>       file: aDownload.target.spec,
>       target: aDownload.displayName,
>       uri: aDownload.source.spec,
>       state: aDownload.state,
>       progress: aDownload.percentComplete,
>       progressMode: nsITreeView.PROGRESS_NONE,
>       resumable: aDownload.resumable,
>       startTime: Math.round(aDownload.startTime / 1000),
>       endTime: Date.now(),
>       referrer: null,
>       currBytes: aDownload.amountTransferred,
>       maxBytes: aDownload.size,
>       lastSec: Infinity, // For calculations of remaining time
>+      download: aDownload
Oops, this should be dld: aDownload - does that fix those errors?
Attached patch Fix addDownloadSplinter Review
A couple of fixes to addDownload that should address all the above issues.
Attachment #706353 - Attachment is obsolete: true
Attachment #706413 - Attachment is obsolete: true
Attachment #706353 - Flags: review?(philip.chee)
Attachment #706423 - Flags: review?(philip.chee)
Comment on attachment 706423 [details] [diff] [review]
Fix addDownload

> A couple of fixes to addDownload that should address all the above issues.
Looks like things are working again f=me.
Attachment #706423 - Flags: review?(philip.chee) → feedback+
Comment on attachment 706423 [details] [diff] [review]
Fix addDownload

(This subsumes bug 830190)
Attachment #706423 - Flags: review?(iann_bugzilla)
Comment on attachment 706423 [details] [diff] [review]
Fix addDownload

When trying to compile I get the following error:
xpidl.IDLError: error: type 'nsIDownload' not found, /comm-central/suite/common/public/nsISuiteDownloadManagerUI.idl line 14:33
                   [optional] in nsIDownload aDownload,
                                 ^
make[6]: *** [_xpidlgen/nsISuiteDownloadManagerUI.h] Error 1
(In reply to Ian Neal from comment #11)
> When trying to compile I get the following error:
> xpidl.IDLError: error: type 'nsIDownload' not found,
> /comm-central/suite/common/public/nsISuiteDownloadManagerUI.idl line 14:33
>                    [optional] in nsIDownload aDownload,
>                                  ^
> make[6]: *** [_xpidlgen/nsISuiteDownloadManagerUI.h] Error 1
Means your tree doesn't have bug 830271 applied yet.
No longer depends on: 830190
Duplicate of this bug: 830190
Attachment #706423 - Flags: review?(iann_bugzilla) → review+
Pushed comm-central changeset dcdffce8cf46.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.18
Blocks: 842194
Blocks: 849440
See Also: → 949994
You need to log in before you can comment on or make changes to this bug.