13325 - ImageFrame displays error icons too quickly

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[gemma]

After repeatedly rolling over the side menu for a while, the viewer will throw
an assertion error of "invalid float type" at nsBlockReflowState::PlaceFloater.

Comment 1

[kipp]

With a current build pulled this morning, I can't reproduce the bug. HOWEVER,
there is some serious horkage in the way that the rollovers are working. For
some reason we are painting the intermediate rendering for the images even
though the images have already been loaded, when we are switching back and forth
to different urls (on linux anyway...).

I think something is wrong with the notifications in the system such that we are
switching images, doing the reflow, looking up the new image and deciding that
we don't have it yet.

Worse, my network busy light comes on when I'm rolling up and down *even though
I've already hit the image before*. This smells like the image cache isn't
working right either.

I'm going to give this to the necko folks first to do more diagnosis on the
rollover loading issues; it may be a bug in the image lib -- it used to be true
that if the image lib already had an image in the cache that the notifications
would be done synchronously with the lookup request. If this has been broken
then we have a problem to work out...

I'm assinging it to Mr. Necko to look into...

Comment 2

[Warren Harris]

Pam: Can you verify whether we're hitting the image cache on this or not?
Thanks.

Comment 3

[pnunn]

Warren:
yep. When we mouse over one of the images on the left,say blah.gif for
example,  an image request goes out for  blah_ovr.gif.

And we can display both blah.gif and blah_ovr.gif.
Maybe the invisible/visible flag is confused? or the view is whacked?

back over to you.
-p

Comment 4

[pnunn]

I forgot to mention,  we DO check in the image cache
for the blah_ovr.gif image.
Beep again if I can track more info on this one.
-pn

Comment 5

[Warren Harris]

Moving what's not done for M12 to M13.

Comment 6

[Warren Harris]

I really don't know anything about image rollovers or how the imagelib cache
works. And reading through this, I can't tell if this is still an issue or not.
Can you own this one Pam? Thanks.

Comment 7

[pnunn]

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

Comment 8

[pnunn]

I think the wacky rollover problem is now fixed. But
I see another bug. As you mouse over an image, you get
a loading icon before the new *hilighted* image loads.
And you get a yucky box drawn around it. I want to trace
the error condition that triggers these ugly and
unnecessary visual feedback.
-pn

Comment 9

[pnunn]

Nisheeth:
I'm not sure if this bug is in your domain. If its
not, I'll bet you know who should own it.

The problem is that when we get a new image (by way
of js in this case), the 'no image'icon comes up too
quickly.  In this example, the mouseover should show
the text and image in red. Before the red image can 
display, the code in nsImageFrame jumps the gun and
puts out a badimage icon and a rectangle for the size
of the image. And then the image loads.

This looks terrible.
I can 'fix' this by disabling the DisplayAltFeedback() line
in nsImageFrame::Paint()... but since I don't know the real
consequences to disabling this, it seems right to pass this bug
to someone who does know the consequences.  

The line is in mozilla/layout/html/base/src/nsImageFrame.cpp
line 482. DisplayAltFeedback().  

Perhaps some other conditions need to be in the surrounding if
statement?

-P

Comment 10

[pnunn]

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

Comment 11

[neeti]

Adding myself to cc list

Comment 12

[Nisheeth Ranjan]

Troy, what do you think about this problem?  The image rollovers on 
http://www.shout3d.com/ do look pretty hokey.  DisplayAltFeedback() gets called 
from nsImageFrame::Paint() whenever the image isn't available from the image 
loader.  If the image has failed to load, the broken image icon is drawn.  If 
the image is loading, the loading image icon is drawn.

Some ways to fix this would be to

a) Never draw the loading image icon.  This would be pretty bad on pages with 
slow links because the user would see blank rectangles on the screen until the 
image got loaded.

b) Draw the loading image icon only after some interval of time has elapsed.

Any other ideas?  

Comment 13

[Nisheeth Ranjan]

Also, I don't think this is a beta blocker.  So, setting target milestone to 
M15...

Comment 14

[troy]

I think that what Nav 4.x did was for the initial load it would display the 
loading icon and loading text, but for subsequent loads (e.g., rollover) we 
would not and we would just leave the current image up until the rollover image 
was loaded

I assume it's still the same way now and that we're re-using the same image 
frame and not throwing away the old one and creating a new one when the rollover 
occurs?

Comment 15

[Eli Goldberg]

[Chris, please be sure to verify 13325 when you verify this bug --- or QA assign 
it to me when you're done, so that I can do so. Thanks!]

Comment 16

[Nisheeth Ranjan]

I have a fix for this in my tree.  I'm going to get it code reviewed by Troy and 
check it in.

Comment 17

[Nisheeth Ranjan]

The fix is checked in.

Comment 18

[Chris Petersen]

With the May 22nd build, the rollover image ar displayed correctly.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a7e8a107375a
[fenix] For https://github.com/mozilla-mobile/fenix/issues/13325 - Positions toolbar according to user preference