73106 - Change posing of download progress for embedding.

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, enhancement)

Description

[Peter Trudelle]

Need to change how this dialog is invoked so it can be overridden by embedding 
clients.  Some details below, more at URL above; danm will be happy to help and 
answer questions.

UI Posed by Gecko (Up Calls)
============================
These are calls to window.open, in one form or another, which are relevant to 
embedding. They're split into calls and callers, where a call is considered to 
be a base-level call to window.open which should be API-ized like we're 
discussing, and a caller is something which currently uses window.open but 
should use one of these new APIs when they're created.

I've tried to categorize these sensibly... Each category implies a UI-posing 
component with methods for each item marked "CALL" in that category. Items 
marked "CALLER" in a given category may imply a new method in that component 
(eg. the print dialog stuff), or may just be a caller of some other extant 
method in a different component (eg. the uri loader opening a new window).

Download Progress:
CALL: /xpfe/components/xfer/src/nsStreamXferOp.cpp#102
  chrome://global/content/downloadProgress.xul

Comment 1

[Peter Trudelle]

Adding mozilla0.9 keyword, dependency

Comment 2

[Dan Rosen]

Apologies for all the bug spam. For the detailed overview of this task, see
danm's document:

  http://www.mozilla.org/xpfe/embedding-dialogs.html

See my first cut at a component-wise categorization:

  http://bugzilla.mozilla.org/showattachment.cgi?attach_id=27972

And if you want to laugh, see also my brainless incomplete IDL for these components:

  http://bugzilla.mozilla.org/showattachment.cgi?attach_id=28175

dr

Comment 3

[Bill Law]

Targetting mozilla0.9 and adding "nsbeta1+" so it appears on my radar.

Comment 4

[Bill Law]

I'm not getting to this dialog till next round...

Comment 5

[Bill Law]

OK, moving back to mozilla0.9 'cause these are "embedding" bugs.

Comment 6

[Bill Law]

damnit, mozilla0.9!

Comment 7

[Bill Law]

Setting date in status whiteboard.

Comment 8

[Peter Trudelle]

Bill, you can move this to 0.9.1, unless you have other need for it in 0.9.

Comment 9

[Bill Law]

We changed our minds; targetting 0.9.1 now.

Comment 10

[Bill Law]

Overlaps with bug 65770

Comment 11

[Viswanath Ramachandran]

nav triage: dont think this is holding any product release, it shd be targeted 
to the mozilla1.0 architecture freeze. moving as such. 

Comment 12

[Peter Trudelle]

Vishy, this is an *embedding* bug, why are you triaging it, and delaying it past
all known embedding deadlines?  cc vishy, marek

Comment 13

[Bill Law]

Attachment: New progress dialog stuff

Comment 14

[Bill Law]

Attachment: xfer component changes to use new progress dlg

Comment 15

[Bill Law]

I attached a new "progress dialog" component (first patch), and, changes to the 
xfer component (file-save-as, save-link-as code) that changes it to use this 
component.

The progress dialog is implemented using one of the existing progress dialogs 
so the implementation is a little thin.  It just creates some JS glue objects 
that bind the new interface to the old implementation.

What's missing is the change to the other client of the progress dialog: the 
uriloader/exthandler code.  Either it needs to change to use the stream 
listener from the xfer component (desirable, because that gets it error 
handling; undesirable because it's more work), or, it needs to be wired up to 
the new interface/component (so that embedders can hook in).  The latter 
probably isn't too hard (it's sort of the inverse of the glue layer in that 
component now).

This code isn't ideal, but probably accomplishes the objective.

Comment 16

[Viswanath Ramachandran]

Peter- sorry about that. I moved it out of mozilla0.9.1 as it was not a netscape 
beta stopper. Pls work with Bill to get the fix we need for embedding. 

Comment 17

[Philip Langdale]

This bug seems to be depressingly stalled. Apart from the null plugin issue,
this is the last non-compliant dialog left, and it's looking as if the null
plugin will be fixed in such a way as to use the prompt service so embedders
will get it for free. 

I don't know what other embedders feel about this, but for galeon it's a very
frustrating bug. Due to the way the external helper service is setup, even
though it's technically non-compliant, there isn't a problem because 
ShowProgressDialog lives in nsIHelperAppLauncherDialog, but nsStreamXferOp's
usage cannot be overridden without this landing. So, our users are placed in
the confusing situation of seeing our gnome progress dialog if the download
goes through the external helper service and the xul dialog when it goes 
through the stream transfer service.

What's the new embedding api finalisation target seeing as 0.9.2 has come and
gone?

Comment 18

[Bill Law]

I think we should try applying the patches I've attached and see if that gets
things working better.  I'll work on applying them to my current source tree and
verifying that it works for me (bringing up the same xul progress dialog as we
get via the uriloader).  I'd also like to make sure that it works OK with your
"override" of that dialog.

I'll post an update (and updated patch if necessary) later today.

Comment 19

[Bill Law]

Attachment: Updated patch for mozilla/xpfe/components/xfer/src files

Comment 20

[Bill Law]

I've attached the updated patch (no substantive changes).  This appears to 
still work for me.

The other patch attached previously (the one labelled "New progress dialog 
stuff" is unchanged.

Comment 21

[Philip Langdale]

I've successfully applied the patches and mozilla works as it should. I'm going
to start looking at constructing our GNOME impl of the interface as soon as I 
can. Have to think about the best way to reuse our existing nsIWebProgressListener.

Comment 22

[Philip Langdale]

Heh, this implementation is pretty crazy. :-)

I'm tempted to run screaming for the trees and hope that adam lock's new
nsIWebBrowserPersist lands. It uses nsIWebProgressListener in the same way that
nsIHelperAppLauncher does. We can use it's save function instead of going
through the stream transfer manager.

I'd hate to see what the c++ version of this ( ie: what our code would be )
would look like. :-)

Comment 23

[sairuh (rarely reading bugmail)]

spam: over to File Handling.

Comment 24

[Bill Law]

->mozilla0.9.8

See also bug 102556.

Comment 25

[Bill Law]

All these download related items are moving out; delayed due to overhaul of 
downloading and many regressions in this area.

Comment 26

[Bill Law]

Spam: Setting target milestone for all these to Future.

Please note that most, if not all, will be fixed in the course of the work I'm 
doing for bug 27609.  That fact is noted in the "depends on" field for each of 
these bugs (I think; go ahead and remedy that if you like).

I just don't have time to deal with the wrath that comes with having too many 
bugs.

Comment 27

[Bill Law]

New nsIProgressDialog interface is now in place.  Embedders, have at it.

Comment 28

[sairuh (rarely reading bugmail)]

over to embed qa [load-balancing]...

Comment 29

[Michael Dunn]

Clean up verification of dated code change bus