67852 - nsFTPChannel doesn't always have an nsIPrompt available

Status: RESOLVED WONTFIX

(Core Graveyard :: Networking: FTP, defect)

Description

[Mike Pinkerton (not reading bugmail)]

- instrument nsFTPChannel::AsyncRead with the following:

NS_ASSERTION ( mCallbacks, "no mCallbacks!");
if ( mCallbacks ) {
  nsCOMPtr<nsIPrompt> prompt;
  mCallbacks->GetInterface ( NS_GET_IID(nsIPrompt), getter_AddRefs(prompt) );
  NS_ASSERTION ( prompt, "Channel doesn't have a prompt!!!" );
}

- go to any ftp site, like sweetlou.

You will hit the assertion several times that says that |mCallbacks| is null. If 
anyone needed an nsIPrompt from this channel, they would not be able to get it.

Comment 1

[Marek Z. Jeziorek]

changing target milestone to Mozilla 0.9

Comment 2

[Marek Z. Jeziorek]

I think I messed up the process: 
* reverting to -- as milestone
* proposing Mozilla0.9 as the target milestone
* adding embed keyword

Comment 3

[Andreas Franke (gone)]

Sanity check failed: this bug is targeted at mozilla 0.9.1 but blocks bug 44809
which is targeted at mozilla 0.9. One of these bugs needs to be re-targeted.

Comment 4

[Doug Turner (:dougt)]

I checked in the assertion to the ftp code.  I am only seeing the assertion when
trying to download something.  The nsExternalAppHandler is setting the
InterfaceRequestor correctly, but the implemention behind this requestor does
not have a nsIPrompt.  I would guess that http has the same problem.  Mike, I am
assigning this back to you.  I am not sure who should own this.  maybe law?

Comment 5

[Mike Pinkerton (not reading bugmail)]

well, mscott's name is all over that file, so -->mscott

Comment 6

[scottputterman]

Besides the assertions what incorrect behavior happens because of this bug?  Bug
44809 seems to imply that the ftp dialog will be parented correctly.

Comment 7

[Dan M]

The completion of bug 44809 makes this bug less urgent. I can think of two 
remaining problems attributable to this bug, if 44809 works as well as expected. 
One is the relatively minor problem that nsIPrompts may be posed with a valid but 
incorrect parent. (Though the parent it'll pick may arguably be a better choice: 
the topmost window). My other concern centers around nsHTTPChannel::Authenticate 
(I don't know what the equivalent FTP method would be), which never gets so far 
as worrying about the correct parent window; if not given a valid nsIPrompt, it 
just fails authentication.

FTP isn't the only code that's already holding onto a valid NotificationCallback 
that doesn't happen to be an nsIPrompt. Seems like there's a flaw in our 
architecture here. On the plus side, the work in bug 72111 is intended to find a 
fallback nsIPrompt from the loadgroup. Will that be sufficient to find an 
nsIPrompt from an FTPChannel? Always?

Comment 8

[Scott MacGregor]

pushing off.

Comment 9

[scottputterman]

moving to 0.9.3

Comment 10

[Scott MacGregor]

pushing further off

Comment 11

[Asa Dotzler [:asa]]

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)

Comment 12

[Daniel Bratell]

*** Bug 128570 has been marked as a duplicate of this bug. ***

Comment 13

[Daniel Bratell]

I can reproduce it by going to 
ftp://ftp.sunet.se/pub/pc/games/gamesdomain/demos/warlordsbattlecry2_demo.exe

Comment 14

[saari (gone)]

EDT topembed-. However, we may be wrong :-). danm, others, if you think this is
really bad, like FTP authentication won't work, please renominate or just hit me
on the head

Comment 15

[David Epstein]

Was able to get this assert from www.mozilla.org and then clicking on the
"Windows" Nightly build link (bottom right of page). Mozilla 1.0.0 Gecko
20020509 build.

Comment 16

[benc]

moved [nsbeta1+] status to keyword.

Remove it if it doesn't belong there.

Comment 17

[Jaime Rodriguez, Jr.]

Corfirming topembed- [T2] per EDT triage.

--> danm

Comment 18

[Juan José Mata]

Reassigning futured nsbeta1+ bugs to Samir in case he wants to find people that
can pay attention to them.

Comment 19

[Samir Gehani]

Over to default owner.

Comment 20

[Doug Turner (:dougt)]

moving out.  

Comment 21

[Doug Turner (:dougt)]

mass reassigning to nobody.