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)
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
Reporter | ||
Comment 2•25 years ago
|
||
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
Comment 4•25 years ago
|
||
This is not only a performance issue, but is giving the user false feedback.
Nominating for beta
Keywords: beta1
Assignee | ||
Comment 5•25 years ago
|
||
I'm taking it over Pierre - I have a nice tidy fix for it.
Assignee: pierre → attinasi
Assignee | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•25 years ago
|
||
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
Comment 7•25 years ago
|
||
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
Updated•25 years ago
|
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
Assignee | ||
Comment 9•25 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•