Closed Bug 27042 Opened 25 years ago Closed 25 years ago

Animated background images never stop painting

Categories

(Core :: CSS Parsing and Computation, defect, P3)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: eric, Assigned: attinasi)

Details

(Whiteboard: [PDT+] 03/09: Need eng to verify)

1) Turn on paint flashing. 2) Load the attached example 3) Now move over the bordered box with the mouse. An animated image will appear. 4) Move the mouse away. You will notice the image will dissappear as it should but the system will continue to draw it indefinitely. This is a big problem for the new progress meter implementation. It uses an animated gif for the barber pole. But it never, ever stops painting it. This is a huge performace problem. ---Example---- <html> <head> <style> div { width: 60px; height: 60px; border: 10px inset black; } div:hover { background-image: url(http://www.pixar.com/logo/animated-luxo.gif); border: 10px inset red; } </style> </head> <body> <div> </div> </body> </html>
This is an interesting problem. I wouldn't do this if I were you, since this probably won't get fixed immediately My guess is that this is a style system problem. When the style changes and we remove the background image, we need to make sure we remove the background image as well. I believe that what Kipp did was have the CSS rendering code load up the background image. I think the pres context maintains the image loader handles
Assignee: troy → pierre
Component: Layout → Style System
QA Contact: petersen → chrisd
I looked into this. Any style change that switched background images anywhere will make this happen. I happens because when the style changes for a frame it does not stop any animated images (it starts them on PaintBackground). Turns out that calling: aPresContext->StopAllLoadImagesFor(this); in nsFrame::ContentStateChanged fixes the problem. If this fix is ok let me know. I'm landing my progress meter changes tommorrow (they fix several PDP+ bugs). If not I may have to make the progress meter show up yellow when busy. That just may give German a heart attack! :)
Is ContentStateChanged() the right place to do this? I thought that function was called for pseudo-class like changes, e.g., hover? We should only do this when the background image property actually changed Calling StopAllLoadImagesFor() is the right call to make. That's what nsFrame::Destroy() calls as well
This is not only a performance issue, but is giving the user false feedback. Nominating for beta
Keywords: beta1
Whiteboard: [PDT+]
I'm taking it over Pierre - I have a nice tidy fix for it.
Assignee: pierre → attinasi
Status: NEW → ASSIGNED
ReResolveStyleContext now checks for case where background image is no longer used, in which case it gets rid of it. Thanks to evaughan for suggesting the fix, and Troy for confirming StopAllLoadImagesFor() is correct call to make. r=buster,pierre
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Eric: I don't have a debug build and it doesn't appear that I can turn Paint Flashing on and off in a commercial build - so, if you agree, could you please mark this bug verified fixed? Since it is a PDT+ bug, I need to get this resolved. Thanks
Whiteboard: [PDT+] → [PDT+] 2/23: Requested verification by reporter
guys, we need you to verify please!
Whiteboard: [PDT+] 2/23: Requested verification by reporter → [PDT+] 03/09: Need eng to verify
For what it's worth, I just verified this is fixed in viewer using Paint Flashing... Of course, I fixed it and had verified it at that time, so maybe somebody else should make sure it is *really* fixed? If not, then mark if VERIFIED on my word, please.
Marking Verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.