Open Bug 648568 Opened 13 years ago Updated 5 months ago

<script src> before <img> which returns non-image data causes double-download of the non-image data

Categories

(Core :: Graphics: ImageLib, defect)

Product:
Core
Component:
Graphics: ImageLib
Platform:
x86
Windows XP
Type:
defect

Tracking

()

People

(Reporter: webkit, Unassigned)

References

Details

(Keywords: dev-doc-needed, perf) 

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0) Gecko/20100101 Firefox/4.0

If a page includes an external JavaScript file before an <img> whose src has a non-image content-type, Firefox downloads the <img> src twice. Reproducible in Firefox 3.6 and Firefox 4.

Simple example:

<!DOCTYPE html>
<html>   
<head>
    <script type="text/javascript" src="http://yui.yahooapis.com/combo?3.3.0/build/yui/yui-min.js&3.3.0/build/loader/loader-min.js"></script>
</head>
<body>
    <img src="http://www.yahoo.com/" width=1 height=1 border=0>
</body>
</html>

This also shows the issue:

<!DOCTYPE html>
<html>   
<head>
</head>
<body>
    <script type="text/javascript" src="http://yui.yahooapis.com/combo?3.3.0/build/yui/yui-min.js&3.3.0/build/loader/loader-min.js"></script>
    <img src="http://www.yahoo.com/" width=1 height=1 border=0>
</body>
</html>

Reproducible: Always

Steps to Reproduce:
1. Include external JS in a page
2. Include an <img> with a src set to a non-image
3. Load page and watch HTTP traffic
Actual Results:  
<img> src is downloaded twice

Expected Results:  
<img> src is downloaded once

This is important because a lot of ad-tracking beacons don't use the image content-type, and this issue can cause double-counting of ads for any page where an external JavaScript is included before the beacon.
Presumably what happens here is that we kick off the image preload and by the time the real image load starts we've detected that the result is not an image (I assume _that_ is what matters, not the content-type; please correct me if I'm wrong), and killed the preload, so we have to make a new request.

It might be nice to cache the "this is not an image" bit in imagelib (by not dropping the imgRequest from the cache just because it's not an image?).  Would improve performance, I guess.  Joe, what do you think?

> This is important because a lot of ad-tracking beacons don't use the image
> content-type, and this issue can cause double-counting of ads for any page
> where an external JavaScript is included before the beacon.

I don't think we should be constrained by that broken setup.  If we think we can get a better browsing experience for our users by making two requests for the image (or none!) we should do just that.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: perf
Product: Firefox → Core
QA Contact: general → general
Summary: <script src> before <img> with non-image content type causes double-download → <script src> before <img> which returns non-image data causes double-download of the non-image data
Version: unspecified → Trunk
Er, ccing Joe too.  Joe, see comment 1?
Component: General → ImageLib
QA Contact: general → imagelib
You could be right. I've seen two scenarios:

1. A response that is missing a content-type and doesn't contain an image
2. A response that has a non-image content-type and also doesn't contain an image

I've not built out a test to see if an image served with a non-image content-type would cause the issue. 

For some background: the tracking beacons used by ads are frequently included as images in the page because there's no cross-domain restriction. They sometimes return images (1x1 tracking pixels) but don't necessarily always return images since the response isn't necessary to the experience...it's only necessary that the request reaches the server for tracking.
> I've not built out a test to see if an image served with a non-image
> content-type would cause the issue. 

OK.  For <img> loads, the Content-Type header is more or less completely ignored, so all that matters is the data that was returned.
If the prefetched-(non)image's cache expiration hasn't expired by the time the
real <img /> load starts the "image" should probably NOT be downloaded again.
If it has expired or has no expiration directive the (non)image SHOULD be
redownloaded; as the contents of the url may have changed in brief interim
between the prefetch and load-start. Right?
David, once we discover that the data is not an image we close the connection.  So the non-image data never gets cached in its entirety.
I think that caching a non-image is just fine, actually! I'm also 100% certain that it will require some changes in the notifications sent by imagelib, because we never assume we're going to keep in-error images around.
Blocks: 654401
Hey, guys.  I'm poking you about figuring out what we want to do here.  Maybe nothing is the right answer, but I'd like some forward progress and ownership.  Thanks!
There's no figuring out to do.  We need to do what comment 1 suggested and comment 7 agreed with.

As for ownership... we need someone other than Joe working in imagelib.  Can you make that happen?
Let me see what I can do. :)
I'd like to add another variation of this bug:
even if the image-tag is part of the external javascript, it is loaded twice:
document.write('<img src="http://www.yahoo.com" width="1" height="1">')

But if the image-tag in the external script is coded this way, it is loaded only once:
document.write(unescape('%3Cscript') + ' type="text/javascript">');
document.write('document.write(\'<img src="http://www.yahoo.com" width="1" height="1">\');');
document.write(unescape('%3C%2Fscript>'));
and it also happens, if the src-url is a non-image, that redirects to an image (e.g. in those tracking-pixels)
Blocks: 727754
Severity: normal → S3

The severity field for this bug is relatively low, S3. However, the bug has 3 duplicates.
:aosmond, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Flags: needinfo?(aosmond)

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Flags: needinfo?(aosmond)
No longer blocks: 727754
You need to log in before you can comment on or make changes to this bug.