62108 - No Basic Authentication dialog for inline images <IMG>

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect, P2)

Description

[tdowling]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.01; Windows NT 5.0)
BuildID:    2000120604



Reproducible: Always
Steps to Reproduce:
This can be demonstrated by any page in which an <img> element points to a 
directory or location that needs basic authentication.

Actual Results:  Mozilla silently does nothing when it encounters this 
situation.

Expected Results:  All other browsers display the Basic Authentication dialog 
while displaying the page; if authentication is successful, they display the 
inline image.  This is what I expected Mozilla would do.

Comment 1

[Boris Zbarsky [:bzbarsky]]

I see this, linux build 2000-12-05-08.  Confirming, OS -> all, over to networking

Comment 2

[Jesse Ruderman]

Adding to tracking bug 61681 (http auth).

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This prevents my use of the browser for dogfood since I can't use it to view
the materials for a class I'm taking:
http://www.courses.fas.harvard.edu/~hsb13/pages/toc.html

Adding dogfood and 4xp keywords and nominating for mozilla0.9/nsbeta1.

Comment 4

[neeti]

-->authentication bugs

Comment 5

[Darin Fisher]

On target for 0.9

Comment 6

[Darin Fisher]

Who creates the HTTP channel that fetches the image?

Comment 7

[Darin Fisher]

pavlov: it looks like imgLoader.cpp will need access to "the" nsIPrompt impl.

Comment 8

[Stuart Parmenter]

what do i need to do?  http should pop up the dialog without me caring, no?

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

That sounds like a good way to fix this bug, since that would also fix it for
applets, etc. (for which I would guess that it's probably broken, although I
haven't checked)...

Comment 10

[Darin Fisher]

in the current world, you have to pass in notificationCallbacks when you create
a channel.  HTTP will GetInterface its notificationCallbacks for a nsIPrompt if
it needs to prompt for authentication.  HTTPS also has to pass this nsIPrompt
down to PSM.

you should be able to get a nsIPrompt implementation from whomever invoked the
imgLoader.

perhaps (as you suggest) this could be solved by simply having HTTP GetService
something that could provide nsIPrompt... but, I have no idea what service that
might be.

jud, scott, rick: is there a service out there that provides a nsIPrompt
implementation?

Comment 11

[Judson Valeski]

this smells a bit like http://bugzilla.mozilla.org/show_bug.cgi?id=67852. I
swear there is/was a bug floating around about the URI loader not providing a
prompt (via whatever mechanism) to loads, though I suspect that it was ill-born.
imagelib uses the uriloader to do loads (even though it shouldn't), and the URI
loader has no concept of prompts. it's currently up to the consumer to provide
nsIPrompt if it wants to/can. The consumer is the only one that can effectively
parent a dialog.

It is possible to get a nsIPrompt out of thin air (talk to danm about it)
although you loose parentage and modality... two things that IMO are important
when throwing auth dialogs. 

if imagelib weren't doing it's own uriLoader level loads, and instead the parser
(?) were doing the inline load for it, we'd have a prompt available (w/ the
correct parentage and modality). The point is that a load is associated w/ a
window, and currently inline images, being loaded directly by imageLib, have no
window level context to provide a prompt for.

Comment 12

[Stuart Parmenter]

Imagelib loads shouldn't be associated with a window.  It seems to me that 
there is a fundamental problem here...

Images in web pages (image frames (<img ...>)) fire off a load through 
imagelib, which in turn gives stuff back to whomever or whatever asked for it.  
Image loads don't have to be associated with a window.  Isn't the purpose of 
the loadgroup to know what loads are happening?  Why do we also need a window 
to know what loads are happening or why does the load need to know what window 
it is coming from?  I (as a consumer) happen to be using HTTP, through necko, 
which needs to pop up a dialog, but this really shouldn't have anything to do 
with the image load.  I simply want data from some URI, which necko is supposed 
to give me.  What it [necko] has to do to get that data, I shouldn't care about.

It is possible to have 1 channel (image "load") associated with more than 1 
window, so I don't think it is possible to properly associate a load with a 
window even if I thought doing so made sense.

It seems like it would be quite possible to have other things having a single 
channel associated with multiple windows, so this idea just seems bad.

Comment 13

[Terri Preston]

Still no authentication dialog Win 98 build 2001041104

Comment 14

[Dan M]

<preach>But for every load you do, there is an associated window. It's the nature 
of the application. Not acknowledging that occasionally gets you into trouble. 
Like here.</preach>

Sometimes you have to do UI, and you've just got have a window to do it in.
</preach>

Knowing that you were going to do this to me, though, we've made accommodations. 
I almost hesitate to point this out, but the patch below will fix this problem. 
It's probably evil -- I suspect that stopping and fetching a service in the 
middle of a networking transaction is inadviseable. Run it by Gagan before you 
check it in, eh?

Index: netwerk/protocol/http/src/nsHTTPChannel.cpp
===================================================================
RCS file: /cvsroot/mozilla/netwerk/protocol/http/src/nsHTTPChannel.cpp,v
retrieving revision 1.285
diff -u -r1.285 nsHTTPChannel.cpp
--- nsHTTPChannel.cpp	2001/04/10 18:26:02	1.285
+++ nsHTTPChannel.cpp	2001/04/13 06:21:09
@@ -40,6 +40,7 @@
 #include "nsIInputStream.h"
 #include "nsIStreamListener.h"
 #include "nsIIOService.h"
+#include "nsIWindowWatcher.h"
 #include "nsXPIDLString.h"
 #include "nsHTTPAtoms.h"
 #include "nsNetUtil.h"
@@ -2699,6 +2700,14 @@
         // Skip prompting if we already have an authentication string.
         if (!authString || !*authString) {
             // Delay this check until we absolutely would need the prompter
+            if (!mAuthPrompter) {
+                NS_WARNING("that bastard Pavlov didn't give us a prompter");
+                nsCOMPtr<nsIWindowWatcher> wwatch(do_GetService("@mozilla.org/
embedcomp/window-watcher;1"));
+                if (wwatch) {
+                    wwatch->GetNewAuthPrompter(0, 
getter_AddRefs(mRealAuthPrompter));
+                    BuildNotificationProxies();
+                }
+            }
             if (!mAuthPrompter) {
                 NS_WARNING("Failed to prompt for username/password: 
nsHTTPChannel::mAuthPrompter == NULL");
                 return NS_ERROR_FAILURE;

Comment 15

[Darin Fisher]

no.. no.. we've added notificationCallbacks to nsILoadGroup.  HTTP will be
able to inherit its prompt from there.  please don't make necko dependent
on the window watcher!

Comment 16

[rpotts (gone)]

I'm totally with darin on this one!!  Necko is *not* responsible for locating 
"some random nsIAuthprompt" if the consumer (in this case pav) explicitly 
chooses not to provide one!!

Necko has no business knowing about the Window Watcher!  It is the consumer's 
responsibility to provide an nsIAutoPrompt to necko.

It seems that in this case, pav doesn't want to "pollute" his code with ui-ish 
stuff - like getting a prompter...  That's his call, but for the love of god, 
don't put this hack in necko!

It's pav's problem (as the necko consumer) and he has to deal with it!!!

-- rick

Comment 17

[Stuart Parmenter]

moving to 0.9.1

Comment 18

[Stuart Parmenter]

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

Comment 19

[benc]

qa to me

Comment 20

[Gagan]

dropping priority per PDT

Comment 21

[Stuart Parmenter]

This isn't going to make 0.9.1.  Moving to 0.9.2

Comment 22

[benc]

clarifying subject.
+mostfreq (found several lost dupes)

Comment 23

[benc]

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

Comment 24

[benc]

qa -> default
I'll help you set up a test case if you need it. There is a larger issue, which
is everyone who calls necko for FRAME, SRC, and IMG tags needs to test a pile of
stuff that is supposed to sent to Necko (like Referer lines, basic and proxy
auth). I'm hoping to write a generic document that could be used by the various
testers of necko service "consumers" so you can get good coverage.

Obviously I don't code, so Pavlov might have to translate what I am saying to
modern geek.

Comment 25

[benc]

there.

Comment 26

[Jason Kersey]

-> 0.9.4

Comment 27

[Boris Zbarsky [:bzbarsky]]

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

Comment 28

[Gagan]

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

Comment 29

[Stuart Parmenter]

.

Comment 30

[Matthias Versen [:Matti]]

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

Comment 31

[Olav Vitters]

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

Comment 32

[Olav Vitters]

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

Comment 33

[Matthias Versen [:Matti]]

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

Comment 34

[Boris Zbarsky [:bzbarsky]]

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

Comment 35

[Arthur]

....pssst, this bug has been sleeping for half a year. Don't wake it up.... It's
only dogfood, 4xp. Mozilla 1.0 still far away... 

Comment 36

[Likai Liu]

i'm surprised that this bug has been around for so long. i was about to file a
duplicate bug report myself before i found out this bug. since this bug effects
those web services using kerberos https cookie for authentication (basically
they have a form with an inline image submit button), i think the keyword
"kerberos" could be added.

Comment 37

[scottputterman]

Moving Netscape owned 0.9.9 and 1.0 bugs that don't have an nsbeta1, nsbeta1+,
topembed, topembed+, Mozilla0.9.9+ or Mozilla1.0+ keyword.  Please send any
questions or feedback about this to [e-mail].  You can search for
"Moving bugs not scheduled for a project" to quickly delete this bugmail.

Comment 38

[Stuart Parmenter]

Attachment: fix

this patch makes the channel use the notification callbacks from the loadgroup

Comment 39

[Gagan]

Comment on attachment 72853 [details] [diff] [review]
fix


>+      aLoadGroup->GetNotificationCallbacks(getter_AddRefs(interfaceRequestor));
>+      newChannel->SetNotificationCallbacks(interfaceRequestor);

You may want to SetNotificationCallbacks only if you do get the
interfaceRequestor, cuz there is a chance (for embedding cases) where there may
not be an interfaceRequestor returned.

So I'm suggesting a change to--

    if (interfaceRequestor)
	 newChannel->SetNotificationCallbacks(interfaceRequestor);

Comment 40

[Darin Fisher]

Comment on attachment 72853 [details] [diff] [review]
fix

sr=darin

Comment 41

[Stuart Parmenter]

ok

Comment 42

[Darin Fisher]

i should add:  sr=darin w/ gagan's suggested change.

Comment 43

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 72853 [details] [diff] [review]
fix

r=[e-mail]

Comment 44

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 72853 [details] [diff] [review]
fix

a=shaver for 1.0 trunk.

Comment 45

[Stuart Parmenter]

oh yeah, i checked this in

Comment 46

[Terri Preston]

Verified Fixed win 2k build 2002032603, Mac OS X build 2002032608 and linux
build 2002032608

Comment 47

[Mike Kaply [:mkaply]]

I believe I have found a regression that is the result of this bug.

Go to:

http://www.kaply.com/work/fluff.html

when the document is finished loading, there is still a "Sending request"
message at the bottom.

You might have to Shift+Reload a couple times to see this.

I have verified that it doesn't happen ever if you back out this bug.