Closed Bug 77906 Opened 19 years ago Closed 19 years ago

OnStateChange doesn't report the images

Categories

(Core Graveyard :: Embedding: APIs, defect)

x86
Windows 2000
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: ianholsman, Assigned: pavlov)

References

Details

(Keywords: regression)

Attachments

(1 file)

I've got the following code in the OnStateChange function in 
WebBrowserChrome.cpp.
and it fails (with nochannel) when a image request  comes. CSS/JS/HTML files 
work fine. 
This used to work fine about a month ago.
--


	nsCOMPtr<nsIChannel> channel;
	channel=do_QueryInterface(aRequest);
	if (!channel) {
	    	fprintf(stderr, "no channel\n");
	    	
		return NULL;
	}

	
	nsCOMPtr<nsIURI> uri;
	channel->GetURI( getter_AddRefs(uri));
	if (!uri) {
	    	fprintf(stderr, "Failed to get URI\n");
		return NULL;
	}

	nsXPIDLCString uriString;
	uri->GetSpec(getter_Copies(uriString));
	if (!uriString) {

	    	fprintf(stderr, "Failed to get spec\n");
		return NULL;
	}
	*aString = nsCRT::strdup((const char*)uriString);
	if ( *aString == NULL ) {
	    	fprintf(stderr, "Failed to strdup\n");
	}
	    	fprintf(stderr, "returned %s\n",*aString);
	return *aString;
}

Cheers
Ian
Works for me pre and post 0.9 branches.

Have you set a breakpoint on the method to examine aRequest? There should be no 
reason for the QI to fail unless the aRequest pointer is nsnull, points to some 
other kind of object or perhaps the proxy is screwed up.

If it's any of these things, it may be instructive to walk up and down the stack 
a bit to find out why it is so.
I rebuilt it, and it definatly has a request field not null.
I am doing a distclean, and rebuiding everything from scratch, and seeing
if the problem still happens, then I'll start with the debugger
Ok...
the URL seems to get dropped in imgLoader::LoadImage when it creates the 
request.
(but I have no clue at this level, It all looks greek to me)
I'm seeing this too in mozilla. inline images are not getting causing 
OnStateChange() notifications to propegate. I suspect this is an artifact of 
images no longer using the uriLoader for loading. cc'ing pavlov.
Status: UNCONFIRMED → NEW
Ever confirmed: true
rpotts and I were talking and are thinking that this is a result of one of the
following:

1. imagelib isn't adding it's requests to a loadgroup.
2. imagelib isn't adding it's requests to the correct loadgroup.
3. imagelib is adding it's requests to a loadgroup w/ the background flag set
(which would remove it from OnStateChange()) callbacks.
it is adding itself to all of those.  it isn't doing anything with
notificationcallbacks though.  is that the problem?
http://lxr.mozilla.org/seamonkey/source/modules/libpr0n/src/imgRequestProxy.cpp#91
is where imagelib is adding it's requests to loadgroups. I don't know what this
proxy object is for, but I'll assume it corresponse to *every* image request. If
that's true, then imagelib is indeed adding all of it's requests to loadgroups.
The loadgroup comes in from the imglib caller via imgILoader::LoadImage*() calls
which users of imglib use to initiate imglib loads.

So, that either means, callers aren't handing imglib a good loadgroup, or that
the flag manipulation when creating a new channel is getting botched somehow
here http://lxr.mozilla.org/seamonkey/source/modules/libpr0n/src/imgLoader.cpp#122 .
forgot to ans. your question. I don't think notificationcallbacks impact
OnStateChange() notifications. Rick?
a proxy represents each individual request.. those get added to the loadgroup of
the requestor.

the channel itself is not in any loadgroup.  perhaps this is the problem.  if
so, i don't know how to fix it.  the channel can't be added to a loadgroup,
since you could have multiple requests with different loadgroups... adding the
channel to the loadgroup would also mean that you could cancel the channel
directly, which would break different document, same image, different loadgroup
stuff.
oh.  well, my proxy objects do not impl nsIChannel, only nsIRequest.  If the
code in the initial description of this bug is what is happening, the QI will
always fail.  just another guess.. *shrug*
The notification callbacks (nsIProgressEventSink in particular) are used to
generate the OnStateChange(TRANSFERRING) notification and the
nsIWebProgress::OnProgress(...) notifications.

It would sure be nice to get progress info for image downloads :-)
Assignee: rpotts → pavlov
Blocks: 64833, 70229
I actually disagree.  I think that sending progress notifications for images
will slow down performance due to far more progress notifications (its not as if
we don't already get WAY too many).
uh, I think image level progress is a pretty basic feature.
On thing that is not clear to me w/ this, is although we're not producing image
level onStateChange notifications, the mozilla browser UI seems to produce a
progress meter that includes the images (ie. it doesn't complete until all the
images have come in).

the docloader produces nsIWebProgressListener callbacks based on
nsIProgressEventSink, therefore I'm starting to wonder if this is fallout from
the fact that imagelib no longer creates channels, rather than it not providing
notification callbacks. I wonder if we're failing in the docloader somewhere
trying to get a channel for an image, and subsequently we're not producing an
OnStateChange() for it.
get a grip pav...  it's called "user experience".  rendering the image slows 
down the download too :-)

Sending progress notifications require *one* method call!  Since images are the 
bulk of HTTP transactions, not sending progress info makes the whole progress 
notification mechinism useless!
cc'ing darin (explicit question for you below)

I was right in that we are indeed getting the OnStateChange() notification for
image requests. The opperative word there is _requests_. imagelib doesn't impl
channels which is what most OnStateChange() consumers are trying to use to get
some user presentable string looking like a URI out of the callback.

So, the semantic for nsIWebProgressListener implementors who want to get a uri
string out of the requests that come in through the callbacks is that they need
to call the nsIRequest.idl "name" attribute (->GetName(PRUnichar **)) on the
request. That reflects some meaningful string for presentation. 

This semantic has the following affect on callers... where they were once using
"char*" w/ nsIURI->GetSpec(), they now need to use "PRUnichar*" w/
nsIRequest->GetName(). Darin, we were talking about the problem of exposing URI
specs as char* or PRUnichar* a week or two ago. Enforcing this new model would
obviously force users into the "uri's are PRUnichar*'s at the UI level" world.
Thoughts?

This model also requires mozilla to provide "names" that people want. Namely
URI's when possible. I'm attatching a patch to scrape some imagelib specific
strings out of the name attribute so we return a raw URI instead. 
uh, sure.  r=pavlov
sr=rpotts
patch has been checked in.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
No longer blocks: 64833
Clean up verification of dated code change bus
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.