9749 2006-07-05 22:55:34 -0700 A nested content editable div causes unnecessary screen redraws. 2006-07-09 19:26:35 -0700 1 1 1 Unclassified WebKit Layout and Rendering 420+ Mac OS X 10.4 RESOLVED FIXED InRadar, Regression P2 Normal --- 1 bdakin bdakin mitz oldest_to_newest 48443 0 bdakin 2006-07-05 22:55:34 -0700 * 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. 48444 1 9220 bdakin 2006-07-05 22:56:55 -0700 Created attachment 9220 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. 48445 2 bdakin 2006-07-05 22:58:12 -0700 Also the bug goes away if you take away the absolute positioning from either div. 48670 3 9243 bdakin 2006-07-06 20:14:41 -0700 Created attachment 9243 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? 48671 4 bdakin 2006-07-06 20:15:23 -0700 Oh by the way, all of the layout tests pass. But I am still not sure this is right. 49006 5 9320 bdakin 2006-07-09 18:37:57 -0700 Created attachment 9320 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. 49007 6 9320 mjs 2006-07-09 18:40:22 -0700 Comment on attachment 9320 New patch r=me May the lord have mercy on my soul if this was wrong. 49009 7 bdakin 2006-07-09 19:26:35 -0700 I committed this with r15273 9220 2006-07-05 22:56:55 -0700 2006-07-05 22:56:55 -0700 Test case note-editor.html text/html 694 bdakin PCFET0NUWVBFIEhUTUwgUFVCTElDICItLy9XM0MvL0RURCBIVE1MIDQuMDEgVHJhbnNpdGlvbmFs Ly9FTiIgImh0dHA6Ly93d3cudzMub3JnL1RSL2h0bWw0L2xvb3NlLmR0ZCI+CjxodG1sPiAKCTxo ZWFkPiAKCQk8bWV0YSBodHRwLWVxdWl2PSdDb250ZW50LVR5cGUnIGNvbnRlbnQ9J3RleHQvaHRt bDsgY2hhcnNldD1JU08tODg1OS0xJz4KCTwvaGVhZD4KCTxzdHlsZSB0eXBlPSd0ZXh0L2Nzcycg bmFtZT0ibm90ZS1lZGl0b3IiPgoJCWRpdiNub3RlLWhlYWRlcgoJCXsKCQkJcG9zaXRpb246YWJz b2x1dGU7CgkJCXRvcDoyMHB4OwoJCQlib3R0b206MHB4OwoJCQlyaWdodDowcHg7CgkJCWxlZnQ6 MHB4OwogICAgICAgICAgICAgICAgICAgICAgICBib3JkZXI6IDJweCBzb2xpZCByZWQ7CgkJfQoK CQlkaXYjbm90ZS1ib2R5CgkJewoJCQlwb3NpdGlvbjphYnNvbHV0ZTsKCQkJdG9wOjBweDsKCQkJ bGVmdDowcHg7CgkJCXJpZ2h0OjBweDsKCQkJYmFja2dyb3VuZDpsaW1lOwogICAgICAgICAgICAg ICAgICAgICAgICBib3JkZXI6IDJweCBzb2xpZCBibHVlOwoJCX0KCgk8L3N0eWxlPgoJPGJvZHk+ CgkJPGRpdiBpZD0ibm90ZS1oZWFkZXIiPgoJCQk8ZGl2IGlkPSJub3RlLWJvZHkiPgoJCQkJPGRp diBjb250ZW50ZWRpdGFibGU9InRydWUiPjwvZGl2PgoJCQk8L2Rpdj4KCQk8L2Rpdj4KCTwvYm9k eT4KPC9odG1sPg== 9243 2006-07-06 20:14:41 -0700 2006-07-09 18:37:57 -0700 patch repainty.txt text/plain 678 bdakin SW5kZXg6IHJlbmRlcmluZy9iaWRpLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSByZW5kZXJpbmcvYmlkaS5j cHAJKHJldmlzaW9uIDE1MTk2KQorKysgcmVuZGVyaW5nL2JpZGkuY3BwCSh3b3JraW5nIGNvcHkp CkBAIC0xNDkxLDcgKzE0OTEsOCBAQAogICAgIC8vIEZpZ3VyZSBvdXQgaWYgd2Ugc2hvdWxkIGNs ZWFyIG91dCBvdXIgbGluZSBib3hlcy4KICAgICAvLyBGSVhNRTogSGFuZGxlIHJlc2l6ZSBldmVu dHVhbGx5IQogICAgIC8vIEZJWE1FOiBEbyBzb21ldGhpbmcgYmV0dGVyIHdoZW4gZmxvYXRzIGFy ZSBwcmVzZW50LgotICAgIGJvb2wgZnVsbExheW91dCA9ICFmaXJzdExpbmVCb3goKSB8fCAhZmly c3RDaGlsZCgpIHx8IHNlbGZOZWVkc0xheW91dCgpIHx8IHJlbGF5b3V0Q2hpbGRyZW4gfHwgY29u dGFpbnNGbG9hdHMoKTsKKyAgICBib29sIGZ1bGxMYXlvdXQgPSAoIWZpcnN0TGluZUJveCgpICYm ICFpc1Bvc2l0aW9uZWQoKSkKKyAgICAgICAgfHwgIWZpcnN0Q2hpbGQoKSB8fCBzZWxmTmVlZHNM YXlvdXQoKSB8fCByZWxheW91dENoaWxkcmVuIHx8IGNvbnRhaW5zRmxvYXRzKCk7CiAgICAgaWYg KGZ1bGxMYXlvdXQpCiAgICAgICAgIGRlbGV0ZUxpbmVCb3hlcygpOwogICAgICAgICAK 9320 2006-07-09 18:37:57 -0700 2006-07-09 18:40:22 -0700 New patch newRepainty.txt text/plain 629 bdakin SW5kZXg6IHJlbmRlcmluZy9iaWRpLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSByZW5kZXJpbmcvYmlkaS5j cHAJKHJldmlzaW9uIDE1MjI0KQorKysgcmVuZGVyaW5nL2JpZGkuY3BwCSh3b3JraW5nIGNvcHkp CkBAIC0xNDkxLDcgKzE0OTEsNyBAQAogICAgIC8vIEZpZ3VyZSBvdXQgaWYgd2Ugc2hvdWxkIGNs ZWFyIG91dCBvdXIgbGluZSBib3hlcy4KICAgICAvLyBGSVhNRTogSGFuZGxlIHJlc2l6ZSBldmVu dHVhbGx5IQogICAgIC8vIEZJWE1FOiBEbyBzb21ldGhpbmcgYmV0dGVyIHdoZW4gZmxvYXRzIGFy ZSBwcmVzZW50LgotICAgIGJvb2wgZnVsbExheW91dCA9ICFmaXJzdExpbmVCb3goKSB8fCAhZmly c3RDaGlsZCgpIHx8IHNlbGZOZWVkc0xheW91dCgpIHx8IHJlbGF5b3V0Q2hpbGRyZW4gfHwgY29u dGFpbnNGbG9hdHMoKTsKKyAgICBib29sIGZ1bGxMYXlvdXQgPSAhZmlyc3RDaGlsZCgpIHx8IHNl bGZOZWVkc0xheW91dCgpIHx8IHJlbGF5b3V0Q2hpbGRyZW4gfHwgY29udGFpbnNGbG9hdHMoKTsK ICAgICBpZiAoZnVsbExheW91dCkKICAgICAgICAgZGVsZXRlTGluZUJveGVzKCk7CiAgICAgICAg IAo=