Bug 12885 - REGRESSION (r19696): Incomplete background repaint
Summary: REGRESSION (r19696): Incomplete background repaint
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P1 Major
Assignee: Nobody
URL: http://jonaquino.blogspot.com/2007/02...
Keywords: HasReduction, InRadar, Regression
Depends on:
Blocks:
 
Reported: 2007-02-25 02:16 PST by Mark Rowe (bdash)
Modified: 2007-03-06 19:48 PST (History)
3 users (show)

See Also:


Attachments
Reduction (713 bytes, text/html)
2007-02-25 06:24 PST, mitz
no flags Details
Earlier regression with the same root cause (748 bytes, text/html)
2007-02-25 07:04 PST, mitz
no flags Details
[WIP] repaint the overflow delta and the border box delta (17.02 KB, patch)
2007-03-03 04:06 PST, mitz
no flags Details | Formatted Diff | Diff
Repaint the overflow delta and the border box delta (129.57 KB, patch)
2007-03-03 10:00 PST, mitz
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Rowe (bdash) 2007-02-25 02:16:49 PST
Steps to reproduce:
1. Load <http://jonaquino.blogspot.com/2007/02/workaround-for-safari-fouc-bug.html>.
2. When page finishes loading, note that the contents of the blog post has a grey background.
3. Hit Cmd-A to select the page.
4. Note that the background of the post changes to white.

This doesn't occur in Safari with WebKit 418.9.1, but does with current ToT.
Comment 1 mitz 2007-02-25 06:24:52 PST
Created attachment 13366 [details]
Reduction
Comment 2 mitz 2007-02-25 07:04:11 PST
Created attachment 13367 [details]
Earlier regression with the same root cause

This test case demonstrates the root cause of the present bug, going back to r11093 (fix for bug 5633), which is the inability of repaintAfterLayoutIfNeeded to distinguish between overflow bounds and border box bounds.

In this test case, the div's "bounds" don't change at all, but what used to be overflow becomes part of the border box. In the previously posted test case (and in the URL), the "bounds" grow by a little (and the delta is repainted), but unknown to repaintAfterLayoutIfNeeded, within those bounds, the border box grows by a lot to occupy area previously occupied by overflow.
Comment 3 Maciej Stachowiak 2007-02-27 19:35:55 PST
<rdar://problem/5028162>
Comment 4 mitz 2007-03-03 04:06:10 PST
Created attachment 13463 [details]
[WIP] repaint the overflow delta and the border box delta

This is my idea for a fix. Still need to:

1) Figure out where to add the outline width. If it's left being done in absoluteBorderBox() is should probably be renamed.
2) Consider having one function return both the absolute border box and the absolute repaint rect to avoid doing the recursion twice.
3) Fix the SVG renderers that currently pass IntRect() in.
4) Optimize repaintAfterLayoutIfNeeded(): it should not ask for the same area to be repainted twice (once as overflow and once as border box) and it should not ask to repaint border box area that was clipped out (all border box repaints should be clipped to the respective repaint bounds).
5) Check what happens with inset outlines (not really part of this bug, but I suspect they're not repainted correctly).
Comment 5 mitz 2007-03-03 10:00:22 PST
Created attachment 13466 [details]
Repaint the overflow delta and the border box delta
Comment 6 mitz 2007-03-03 13:20:09 PST
Comment on attachment 13466 [details]
Repaint the overflow delta and the border box delta

(In reply to comment #4)
> 1) Figure out where to add the outline width. If it's left being done in
> absoluteBorderBox() is should probably be renamed.

Renamed to absoluteOutlineBox(). Also renamed getAbsoluteRepaintRect() to absoluteClippedOverflowRect().

> 2) Consider having one function return both the absolute border box and the
> absolute repaint rect to avoid doing the recursion twice.

Didn't do it.

> 3) Fix the SVG renderers that currently pass IntRect() in.

Done.

> 4) Optimize repaintAfterLayoutIfNeeded(): it should not ask for the same area
> to be repainted twice (once as overflow and once as border box) and it should
> not ask to repaint border box area that was clipped out (all border box
> repaints should be clipped to the respective repaint bounds).

Done.

> 5) Check what happens with inset outlines (not really part of this bug, but I
> suspect they're not repainted correctly).

Need to file a separate bug on that. The present patch does not address it.

I also added a FIXME in RenderLayer::checkForRepaintOnResize(). I think it can be eliminated, but I do not want to risk it now.
Comment 7 Darin Adler 2007-03-04 22:50:02 PST
Comment on attachment 13466 [details]
Repaint the overflow delta and the border box delta

Looks good to me, but it's obvious that Hyatt's going to review this one.
Comment 8 Dave Hyatt 2007-03-04 23:38:12 PST
Comment on attachment 13466 [details]
Repaint the overflow delta and the border box delta

r=me.  We may want to rename the absoluteOutlineBox method eventually, since I assume it will eventually have to deal with box-shadow?
Comment 9 Mark Rowe (bdash) 2007-03-06 19:48:46 PST
Landed in r19997.