[Webkit-unassigned] [Bug 7204] float inserted in fixed height block via DOM not repainted

[bugzilla-daemon at opendarwin.org]

http://bugzilla.opendarwin.org/show_bug.cgi?id=7204


opendarwin.org at mitzpettel.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hyatt at apple.com




------- Comment #8 from opendarwin.org at mitzpettel.com  2006-06-24 03:06 PDT -------
(In reply to comment #7)
> (From update of attachment 8865 [edit])
> I'm not convinced on this one quite yet.  When a float moves during layout, it
> should still get its coordinates changed in the floating objects list of the
> block that will ultimately paint it.

The way it works is that the floating objects list is first cleared, then the
float is re-inserted with the changed coordinates, so there's no easy way to
identify the "moved" case.

> What bothers me about this patch is that it will end up repainting the same
> float over and over when you have a bunch of nested blocks that all have the
> same float in their floatingObjects list.

Only if each one of those blocks has only moved during layout, which I had a
hard time creating an example of (I had to change the height of two "spacer"
blocks at the same time, each casuing a different ancestor of the float to move
- but not resize). Actually, it was easier (though still requiring two changes)
when I didn't require both blocks to be ancestors of the float, but I think
that case can be handled by painting only descendants in
repaintFloatingDescendants() if the paintAll flag is true (you may want to
rename that method to repaintOverhangingFloats(), which is what it really does,
and then the bool could be called paintAllDescendants).

> I also don't quite understand your point about repaintAfterLayout vs.
> repaintDuringLayout since the former uses getAbsoluteRepaintRectIncludingFloats
> (which also has the !noPaint check).

Yeah, I was wrong, and I realized it once I started working on a fix.

> Also for a check like this:
> 
> +            if (paintAll || !r->noPaint && !r->node->layer()) {                
> 
> You'll do a repaint even of something with a layer, and that's not necessary
> (since layers handle repainting themselves properly already... make the same
> test case with position:relative on the float and you'll see it work I bet).
> 

Agreed.

To sum up, I think that with the descendants-only condition added, the layer
case fixed, and a possible rename, it comes down to how serious the repeated
painting (invalidation really) issue is...


-- 
Configure bugmail: http://bugzilla.opendarwin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.