Bug 12885

Summary: REGRESSION (r19696): Incomplete background repaint
Product: WebKit Reporter: Mark Rowe (bdash) <mrowe>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Major CC: bdakin, hyatt, mitz
Priority: P1 Keywords: HasReduction, InRadar, Regression
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
URL: http://jonaquino.blogspot.com/2007/02/workaround-for-safari-fouc-bug.html
Attachments:
Description Flags
Reduction
none
Earlier regression with the same root cause
none
[WIP] repaint the overflow delta and the border box delta
none
Repaint the overflow delta and the border box delta hyatt: review+

Mark Rowe (bdash)
Reported 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.
Attachments
Reduction (713 bytes, text/html)
2007-02-25 06:24 PST, mitz
no flags
Earlier regression with the same root cause (748 bytes, text/html)
2007-02-25 07:04 PST, mitz
no flags
[WIP] repaint the overflow delta and the border box delta (17.02 KB, patch)
2007-03-03 04:06 PST, mitz
no flags
Repaint the overflow delta and the border box delta (129.57 KB, patch)
2007-03-03 10:00 PST, mitz
hyatt: review+
mitz
Comment 1 2007-02-25 06:24:52 PST
Created attachment 13366 [details] Reduction
mitz
Comment 2 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.
Maciej Stachowiak
Comment 3 2007-02-27 19:35:55 PST
mitz
Comment 4 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).
mitz
Comment 5 2007-03-03 10:00:22 PST
Created attachment 13466 [details] Repaint the overflow delta and the border box delta
mitz
Comment 6 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.
Darin Adler
Comment 7 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.
Dave Hyatt
Comment 8 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?
Mark Rowe (bdash)
Comment 9 2007-03-06 19:48:46 PST
Landed in r19997.
Note You need to log in before you can comment on or make changes to this bug.