RESOLVED FIXED 9749
A nested content editable div causes unnecessary screen redraws. 
https://bugs.webkit.org/show_bug.cgi?id=9749
Summary A nested content editable div causes unnecessary screen redraws.
Beth Dakin
Reported 2006-07-05 22:55:34 PDT
* STEPS TO REPRODUCE 1. Load the test case. 2. Use Quartz Debug and enable "Flash screen updates." 3. Type into the content editable div. * RESULTS Each key stroke causes the whole outer div (which is most of the window) to redraw. This is a regression from shipping Safari.
Attachments
Test case (694 bytes, text/html)
2006-07-05 22:56 PDT, Beth Dakin 		no flags
Details
patch (678 bytes, patch)
2006-07-06 20:14 PDT, Beth Dakin 		no flags
DetailsFormatted DiffDiff
New patch (629 bytes, patch)
2006-07-09 18:37 PDT, Beth Dakin 		mjs: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Beth Dakin
Comment 1 2006-07-05 22:56:55 PDT
Created attachment 9220 [details] Test case We are repainting the outer div, and we used to not do that. It is being repainted from RenderBlock::layoutInlineChildren(). We call repaintViewRectangle() with the layer's repaint rect. I doubt that the repaint rect is wrong -- it seems more likely that we shouldn't be calling it in the first place, but I am not sure since I don't know enough about the repaint code.
Beth Dakin
Comment 2 2006-07-05 22:58:12 PDT
Also the bug goes away if you take away the absolute positioning from either div.
Beth Dakin
Comment 3 2006-07-06 20:14:41 PDT
Created attachment 9243 [details] patch I feel like this patch could not possibly be right. But it definitely hunts down the source of the problem. I remembered fixing a bug a while ago where the root cause was that a bunch of our old code held assumptions that positioned elements would always have line boxes, but it turns out that this isn't true. This assumption is also causing extra repaints because in layoutInlineChildren() we set fullLayout to true if we don't have any line boxes...meaning that positioned elements without line boxes always get a full layout. This is clearly overzealous. I imagine that the patch I attached is undezealous. (underzealous?? forgive me, for it has been a long day.) But I figure that attaching it is a good way to start a discussion about what the right fix is. Thoughts?
Beth Dakin
Comment 4 2006-07-06 20:15:23 PDT
Oh by the way, all of the layout tests pass. But I am still not sure this is right.
Beth Dakin
Comment 5 2006-07-09 18:37:57 PDT
Created attachment 9320 [details] New patch So Maciej and I talked about this on IRC, and we couldn't figure out why the check for the firstLineBox was there at all. So this patch just removes that check entirey. It causes no layout test failures. We are assuming that its presense in determining if we need a full layout is antiquated.
Maciej Stachowiak
Comment 6 2006-07-09 18:40:22 PDT
Comment on attachment 9320 [details] New patch r=me May the lord have mercy on my soul if this was wrong.
Beth Dakin
Comment 7 2006-07-09 19:26:35 PDT
I committed this with r15273
Note You need to log in before you can comment on or make changes to this bug.