85771 - [PATCH] Background images should not display before first frame is loaded

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Steve Chapel]

Reproducabilty: Always when the background GIF is not in the cache

Steps to reproduce:
1. Go to http://www.iowa-city.lib.ia.us/
(If you missed it, you can clear the memory and disk caches and load the URL
again, or you can click on the ICPL catalog link and then the Catalog button.)

Expected results: The page initially is drawn without the background image, and
the background appears when the GIF image is loaded.

Actual results: The background image "flashes" as it is loaded.

I've seen this behavior with the nightly builds of the last several weeks,
including 2001061304, on Windows 2000. I don't know why I see the flashing on
this web site and not others.

Comment 1

[Chris Petersen]

Comfirming issue on the June 13th build.

Comment 2

[doctor__j]

Another URL for testing: http://www.dict.org/

Comment 3

[Aaron Kaluszka]

I think this was one of the libpr0n landing side-effects..  There was a period
when all images did this, not just the background.  Its like its trying to
render when it doesn't have enough data yet, or something like that.  The
"flash" isn't exactly a flash (you can see this better on 56k dial-up.) Few more
sites to test: http://kontek.net/ http://kontek.net/pp/

Comment 4

[Aaron Kaluszka]

mozilla0.9.2->mozilla0.9.3
nsbeta2->nsbeta1

Comment 5

[Skewer]

This bug seems to be a side-effect of displaying background images before they 
finish downloading. In the case of the test page <http://www.iowa-
city.lib.ia.us/> running 2001082203 W98, the background image isn't even 
displayed as expected while loading - the parts that haven't loaded yet should 
be transparent (in this case, the background color), but instead they seem to 
be a mixture of black and garbage. However, since there may be a sharp contrast 
between the background image and the background color, the best way to handle 
this is not to display images at all until the first (or only) frame finishes 
loading. We could to display interlaced images after the first pass finishes, 
but even that might not be the best idea. IE6 and NS4 handle this situation 
correctly.

Comment 6

[Kevin McCluskey (gone)]

Pav any ideas on this one? Look like we are painting the tiled background image
even though its only partially decoded.  Were we supressing the rendering of
background images until they were fully loaded in the old imagelib?

Comment 7

[Skewer]

I haven't confirmed the behavior for animated backgrounds (I *hope* we don't
have many of those out there!), but Netscape 4 does not display single-frame
background images until they are completely loaded. For the test page,
<http://www.iowa-city.lib.ia.us/>, the difference is very easily noticeable on
any narrow-band connection.

A note for anyone who attempts a patch (and somebody please take that job!),
that webpages are not the only thing that can have backgrounds, thanks to CSS.
Background images can apply to *any* element according to CSS1 standards.
Background images should never be displayed before they are fully loaded due to
the fact that the contrast between the background color (or black, which seems
to be happening due to an imglib bug) and the image makes any text on top very
difficult to read.

Comment 8

[Kevin McCluskey (gone)]

We need to suppress the rendering of tiled background images until they have
completed loading.

Comment 9

[Kevin McCluskey (gone)]

Marking nsbeta1+. This is very visible on a large number of sites.

Comment 10

[Kevin McCluskey (gone)]

Attachment: Prevent incremental display of background images by redrawing in OnStopFrame instead of OnDataAvailable

Comment 11

[Stuart Parmenter]

Comment on attachment 72866 [details] [diff] [review]
Prevent incremental display of background images by redrawing in OnStopFrame instead of OnDataAvailable

r=pavlov

Comment 12

[Marc Attinasi]

Comment on attachment 72866 [details] [diff] [review]
Prevent incremental display of background images by redrawing in OnStopFrame instead of OnDataAvailable

sr=attinasi - nice patch.  We really should make it clearer that nsImageLoader
in the layout module is used for loading background images only, and that it is
not the same as imgILoader from imgLib or nsImageFrame. But, your patch is fine
and the comment in OnDataAvailable is good.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 72866 [details] [diff] [review]
Prevent incremental display of background images by redrawing in OnStopFrame instead of OnDataAvailable

I quite liked the incremental loading background images, but I understand the
reasoning...

a=roc+moz for trunk

Comment 14

[tor]

Comment on attachment 72866 [details] [diff] [review]
Prevent incremental display of background images by redrawing in OnStopFrame instead of OnDataAvailable

Removing a= for the moment.

It appears as though you're trying to get around a win32 gfx problem
(not making undecoded portions transparent in a tiled image) by 
changing the incremental loading behaviour.  Is this the case?

Comment 15

[Kevin McCluskey (gone)]

This patch was not intended as a band aid for not making undecoded portions
transparent in a tiled image on WIN32.

It doesn't make much sense to incrementally load background images, especially
tiled background images. The visual effect of displaying a portion of a
background image tiled accross the page does not look very professional.  It
also has an impact on page load performance on these pages because we are
repainting the entire page as each piece of image data arrives. In addition, if
you have a slow connection you spend a lot of time looking at a page with only a
portion of the background decoded which is distracting. No other major browser
incrementally displays background images including 4.x, IE, and opera so content
authors assume the background will appear all at once. In most cases page
authors have picked a suitable background color which will be replaced with the
full background image when it arrives. This effect/bug was introduced when the
new imagelib landed. It was not the intention of the imagelib authors to make
background images load incrementally it was just a side effect. 

Comment 16

[tor]

Comment on attachment 72866 [details] [diff] [review]
Prevent incremental display of background images by redrawing in OnStopFrame instead of OnDataAvailable

I agree with roc - incremental background images loading was a nice effect.

Re-approving for trunk.

Comment 17

[Kevin McCluskey (gone)]

patch checked into the trunk

Comment 18

[Adam D. Moss]

FWIW I enjoyed incremental background image
loading also.  Oh well.

Comment 19

[Skewer]

This bug doesn't seem to be fixed in 20020309 WinXP, only covered up by a messy redraw bug. If I switch tasks to Mozilla while a background is loading, or drag a window (Winamp works great to test this) over it, I get garbage on the redraw.

Comment 20

[Kevin McCluskey (gone)]

I filed a bug 129908 for the remaining problem

Comment 21

[Markus Hübner]

There is a regression on 
https://www.payplus.at/intro.asp
The background image is no longer displayed correctly - this regression 
started very recently.

Comment 22

[Markus Hübner]

The regression started with build 2002031203 (build 2002031103 is fine).

Comment 23

[Markus Hübner]

Could this have caused regression bug 135626 ?

Comment 24

[Vadim Berezniker]

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

Comment 25

[Chris Petersen]

Verified in the April 23rd build (2002-04-23-06).

Comment 26

[Christian 'CeeJay' Jensen]

While I personally enjoyed the progressive rendering of background images (i
have a fast connection) I can see how it could get annoying fast if you had a
slow connection.

So I think this patch is a Good Thing(tm) , but I have an idea that could make
it even better.

Before the patch we had background images that rendered as they were being loaded.
This produced a cool transition effect and more importantly it gave the feeling
that the browser was actually doing something instead of just sitting there
idle... Thus increasing the PERCEPTION of speed (at least to me).
However just like slow IE transition effects this could be really annoying to
thoose with a slow connection, mainly because it distracted from the content ,
especially if the contrast between the background color and the background
images was high.

With the patch , we don't have this transition effect or the feeling that the
browser is working .. the background image just pops up when it's finished which
can sometimes, with a slow connection to the image, seem as if the browser have
stalled because you can tell it's loading but you can't see anything happening.
It does fix the annoyences however.

I propose instead of rendering the background image once it have loaded , it
should be rendered once all the pixels in it have been filled.
So that progressive JPEGs and PNGs and interlaced GIF's will be rendered every
time they have completed a "cycle" .. if you know what I mean.
If it's an 5-pass image it would be updated 5 times each time improving the
quality of the image.
The background images appear much faster because they can be rendered as soon as
enough data have been retrieved to render all the pixels instead of waiting for
the entire file to load , AND they don't become annoying because they don't
cause  big changes in contrast or draw too much attention to themselves from
being in motion constantly.

Note for this to work completely interlaced gif's should be rendered with
pixeldoubling (or even bi-linear filtering) , not scanlines, until the image
have loaded.

Comment 27

[Andrei Boros]

On commnet #26:
Another way to do it, considering the suggested render of progressive 
JPEG, interlaced GIF is to add a configuration option (default off) in 
Preferences->Advanced next to "Automatically load images".