Closed Bug 29853 Opened 24 years ago Closed 24 years ago

Animated ads are stopping after one loop, and are then being reloaded from the server

Categories

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

x86
Windows 98
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: chrisn, Assigned: neeti)

References

()

Details

(Whiteboard: [PDT+] under investigation)

Attachments

(1 file)

From Bugzilla Helper:
User-Agent: Mozilla/4.72 [en] (Win98; U)
BuildID:    2000030108

This is a major problem, and I hope this gets some quick attention.

Visit http://www.mozillazine.org or http://www.mycomputer.com with the latest 
build (I've verified this in Win98).

If the advertisement is an animated gif, watch it carefully. If not, reload 
until you see an animated ad. After the animation runs through one loop, another 
ad is pulled from the server. If that ad is an animation, after one loop, 
another ad is pulled from the server. This continues ad infinitum.

Doron (that's how I know him from #mozilla and #mozillazine) has reproduced this 
bug in the M14 final build, as well.

The problem - it is artificially inflating ad counts on mozillazine (I have 
verified this), and we could find ourselves in trouble with our advertisers. 
This is not good. Also, from testing on other sites within my ad network, the 
problem is reproducable there too (which means it could cause problems for other 
sites advertisers, as well). Try www.mycomputer.com as an example of this.

The problem only seems to affect sites in which the ad name contains a unique 
identifier to prevent caching of the image. For example, at mozillazine, the ad 
image URL looks like this:

http://ad.doubleclick.net/ad/mozillazine.adventure/cat1401;cat=1401;lan=01;typ=0
1;ref=01;pos=01;kw=;sz=468x60;ord=100829?

The # at the end, ord=xxxxx, is a random #, which will keep the image from being 
recognized if the page is returned to, causing a new ad to be pulled down.

Reproducible: Always

Again, this is a serious issue for me, and I need to consider options on my end 
if this problem isn't taken care of quickly. I can't afford to be in trouble 
with my advertisers (or lose my ad contract) because of a bug in this product 
(and seeing as I'm *promoting* Mozilla, it makes me look even worse).

This bug looks like it could be a duplicate of bug 5030 or bug 21036, but bug 
5030 has never shown a reason for the bug to be addressed with a higher 
priority. I hope this gives it some more attention, and possibly an interim 
solution.

Also, the ad hit increase leads me to believe that this particular bug became a 
problem not too long ago (within the past week), so maybe something has changed 
in the past week.
In our ad logs, the # of hits increased on the 23rd.

Checking bonsai, two things caught my attention that might (in my 
admittedly limited understanding of the code) have brought about the change. One 
was the fixing of bug 28289 by pnunn on 02/22/2000 around 18:24.

The other is the landing of NSPRPUB_RELEASE_4_0_20000223 onto the trunk.

Maybe this will help some.
I can confirm this bug on the 022808 win32 build.
nominating for beta1 consideration

cc'ing travis since he had a bunch of checkins during the time in question that 
have to do with reloading and history stuff, though i have no idea if it's 
related.  would you take a look at this bug and see if it sounds in your realm?  
it affects mozillazine.org, our biggest fan :)
Keywords: beta1
Can we get simple steps to reproduce... eli please help.  Marking PDT- until 
someone can show us the problem.
Whiteboard: [PDT-]
I have a simple test case - just the document.write code from the page in 
question, but I want to talk to my ad reseller first, to make sure I can put the 
code in a testcase. I'm pretty sure it'll be allowed, but I should know 
tomorrow. The simplified test case does work, and reproduces the problem 
repeatedly.
Also, take a look at this file:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/modules/libimg/src/if.cpp&re
v=3.43&root=/cvsroot

at line 1022. This file was altered on the 22nd. Coudl this be an issue?
Just some background on how this ad is loaded. There are 2 (302) redirects
involved, and both redirects include http headers to disallow caching, such as:
1st redirect: Cache-Control: private, max-age=0, no-cache
2nd redirect: Pragma: no-cache
The image itself does not have a cache control header.

I'm going to take a look at the netlib cache integration in version 3.41 of
if.cpp and related changes...
Ok, it looks like for this case, mCacheContentIsValid in nsHTTPChannel is being
set to false, so a network reload is done. In the simple case, the cached image
is reused for each cycle. The image is "reloaded" when loop_count > 0 in if.cpp,
and the cache is used for this reload.

Interesting data point: I load this page in 4.72 on linux, and wait for the
Document: Done in the status bar. A few seconds after the page loads, there some
more network activity, and the ad server is contacted, BUT, the image does not
change. Also, if I resize the browser window with 4.72 on linux, the ad tends to
change. I'm not sure what the behavior is on windows.
Here's a simplified version of the page in question - with just the ad code in. 
Load the page with the latest build of Mozilla, and if the ad that displays 
isn't an animated gif, reload until you see one, then let the animation play 
out. As soon as the first loop completes (sometimes the hold on the final frame 
is longer than you expect - so give it some time), a new ad will be loaded.

http://www.mozillazine.org/redirect_bug.html

I think that this sample and Steve's investigations and comments in this bug 
report provide enough information to qualify this for pdt+ and beta1 
consideration.
Removing PDT- designation for re-evaluation per investigation provided. (Thanks, 
suttree@bellatlantic.net, smorrison@gte.com, and chrisn@statecollege.com!)
Whiteboard: [PDT-]
Thanks much smorrison. 
good data. Narrows down the issue alot.
-p
Status: NEW → ASSIGNED
Target Milestone: M14
The most likely culprits are either the cache reload policy in
imagelib and the cache manager in necko.

The image lib takes data from the network cache after the first loop
of animation and redecodes it. 
This prevents double buffering. The cost of decoding is trivial compared
to the cost of getting the data from the server.

If the imagelib is making the correct request to get it from the network
cache, then the decision to validate the network cache for modifications
against the server data is in necko's world.

I'm adding gordon and davidm to cc list for necko cache issues.
-p
PDT+ w/b minus on 3/10
Whiteboard: [PDT+] w/b minus on 3/10
My guess is that the image URL is not pointing at the right URI object. In 
nsImageNetContextAsync::OnDataAvailable, if before the call to FirstWrite(), if 
you look at the url that the channel points to, it is different from the one 
that image container holds onto. I think they should either be the same URI 
object or that the urls should be synced up at some point in time. I am not 
familiar enough with imagelib to be able to say what the right solution is.
*** Bug 30556 has been marked as a duplicate of this bug. ***
thanks, davidm. I think you caught it.

I think I see what's happening. In the old days before the
uri struct, there was an url string. The fetched_url (or resolved
url) was saved as a string. It still is....which is a problem in
the new world of uri. 
Unfortunately, the fix is not as easy as I envisioned yesterday.

I'm documenting what I have now so I won't have to recreate the
world tomorrow:

The URI is smart. It has an originating URI and the URI. Unfortunately,
when the image request is made, only the orginating URL string is passed
in. I'm trying to see where the image container and the URI paths cross, so
the image container can get the resolved url string. 

-P
The mURL (ilIURL *) in the ImageConsumer only has the OriginatingURI data.
When the image lib tries to get the resolved/redirected url string,
it does so on mURL held by the ImageConsumer.

mChannel at this point, is NULL. Atleast, as ImageConsumer::OnDataAvailable() 
sees it,
though aChannel is passed in. mURL is loaded 
 

aChannel in nsDocumentOpenInfo::OnDataAvailable() does have good data 
mOriginatingURI
and mURI.

----------
I'm not quite sure where to start updating mURL, but 
ImageConsumer::OnDataAvailable()
looks like a good place to start.  
I have some changes that not only make sense, but appear
to work. I still have code clean up to do before I get
a sanity check/code review.

One question to the reporter:
I see the ads repeat many times and then a new ad
is introduced, I assume as an effect of the js generated
code.  
Is this what you expect from the page?


pnunn,

In answer to your question, no. No new ads on MozillaZine (or in the 
sample I linked to in a previous addition to this bug) should ever be displayed 
without a reload. All the Javscript does is provide a unique # at the end of the 
URL.

If you're seeing ads loop numerous times, what you are actually seeing is the 
same ad being pulled from the server a few times sequentially, before another ad 
is loaded. These ads are only ever going through their loops once. The reason 
they're coming up so many times sequentially is that the ads themselves are in a 
heavy rotation, and the probability of getting them upon a reload is high.
I have a fix and will be sending davidm & gordon 
a copy for a code review.
-P
Has the fix been passed around?  Any verdict from davidm or gordon??
Thanks,
Jim
Whiteboard: [PDT+] w/b minus on 3/10 → [PDT+] fix in hand w/b minus on 3/10
I'm not happy with the fix.
I've modified the fix that deals with clobbering 
the background load setting. And I'd like to get it
in asap.

I'm still working on a way to deal with the redirect url vs
URI.  Fix it for netwerk, break the image cache look up.
This is highest on my list today. (Apart from checking in what
I can.)
-P
Pam,
I saw the patch... but we're past the 3/10 landing date.  What happened?
Is the patch reviewed?  By whom?
Has this been tested on all 3 platforms?
Thanks,
Jim (thinkin' about reducing this to PDT-) Roskind
Patch wont fix the bug per say. It will make animated image loading better. I
dont think we just want to take the patch.

Pam and Neeti are looking at this.
I am looking at this bug, along with Pam. We do not have a fix yet for the 
redirect urls. 
Whiteboard: [PDT+] fix in hand w/b minus on 3/10 → [PDT+] under investigation
We are still working on this bug. Neeti has new direction that she is exploring.

I think we should ship beta without fixing this, if we have to. The effect is we
refetch animated images only that are got after a redirect (read as
advertisements).

We will update on progress later today.
I have made some to the code which make this work. I am in the process of 
testing this further. Also, need to get it reviewed by Pam, to see if these 
changes are ok and safe.
Pam out sick. Neeti is covering. Reassinging to neeti.
Assignee: pnunn → neeti
Status: ASSIGNED → NEW
Just checked in a fix on the nscp_beta1_branch. Will check it in on the tip as 
soon as possible.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
This looks fixed to the best of my ability to discern. I don't have decent
Network trace tools on Windows that work, so I've done the following on Mac:

Mac OS 3.16.00 5:00 AM beta 1 commercial branch build:
	- Viewing Mozillazine results in the animated GIF ad banners being "stringed",
with network traffic at the beginning of each advertisement.


Mac OS 3.16.00 6:00 PM beta 1 commercial branch build:
       - Viewing Mozillazine results in a single animated GIF ad banner being
displayed. The ad banner stops at the last frame after all frames are displayed.
No additional network traffic occurs after the initial page load.

So, smorrison & chrisn, if you see any evidence that there's still a problem,
please re-open this bug. (I'll give it a visual check on Windows & Linux to be
sure.)

Thanks!
Status: RESOLVED → VERIFIED
Just checked the fix in the trunk.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: