Bug 22159 – Repaint issue with outlines in child objects

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

The attached testcase shows a bug where the focus ring is not correctly invalidated when a text input is moved by layout. It happens to use an animation on another element, but that's just incidental. There are no transforms here.

[Simon Fraser (smfr)]

Created attachment 25021 [details]
Testcase

[Simon Fraser (smfr)]

Created attachment 25332 [details]
Simple testcase with outline

This bug has nothing to do with focus, or animations. There's a basic repaint bug with outlines.

[Simon Fraser (smfr)]

The problem is that RenderBox::absoluteClippedOverflowRect() only looks at the outline size on the style of the element being laid out, but not any of its contents. We could either grovel through the child objects looking at outline size, or just use view->maximalOutlineSize().

I think it's going to have to be view->maximalOutlineSize() :-(

[Simon Fraser (smfr)]

Created attachment 25338 [details]
Patch, testcase, changelog

Patch includes some new bases for repaint tests whose repaint rects alter with this change (because of focus rings)

[Darin Adler]

Comment on attachment 25338 [details]
Patch, testcase, changelog

> +    if (style()->outlineWidth() > 0 && style()->outlineSize() > maximalOutlineSize(PaintPhaseOutline))
> +        static_cast<RenderView*>(document()->renderer())->setMaximalOutlineSize(style()->outlineSize());

Since you had to move this code, maybe there's a way to write a brief comment explaining the order dependency?

I feel a little uncomfortable reviewing this with Hyatt on vacation. You should check with him afterwards just in case he thinks this will result in excessive painting.

r=me

[Simon Fraser (smfr)]

Yes, the over-repaint is a worry, especially since any focus ring makes setMaximalOutlineSize() non-zero. The alternatives would be:
1. Store outline dimensions on RenderBox, and explicitly account for them whem computing repaint rects (but they don't affect overflow, unlike shadows, so we can't just put them in the overflow sizes), or
2. Set a bit on RenderObject to say that it has outlined children, and only inflate by maxOutlineSize when the object or children have outline.

[Simon Fraser (smfr)]

Committed r38678
	M	WebCore/ChangeLog
	M	WebCore/rendering/RenderBox.cpp
	M	LayoutTests/platform/mac/fast/repaint/outline-repaint-glitch-expected.checksum
	A	LayoutTests/platform/mac/fast/repaint/outline-child-repaint-expected.checksum
	A	LayoutTests/platform/mac/fast/repaint/outline-child-repaint-expected.png
	M	LayoutTests/platform/mac/fast/repaint/4776765-expected.checksum
	A	LayoutTests/platform/mac/fast/repaint/outline-child-repaint-expected.txt
	M	LayoutTests/platform/mac/fast/repaint/delete-into-nested-block-expected.png
	M	LayoutTests/platform/mac/fast/repaint/4776765-expected.png
	M	LayoutTests/platform/mac/fast/repaint/delete-into-nested-block-expected.checksum
	M	LayoutTests/platform/mac/fast/repaint/outline-repaint-glitch-expected.png
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/repaint/outline-child-repaint.html
r38678 = 0ebe80da11f747b44e3ba9e89ae67bf891820c51 (trunk)

I'll send mail to Hyatt to look at this change.

[Simon Fraser (smfr)]

Maybe this should have used getAbsoluteRepaintRectWithOutline()?

[David Kilzer (:ddkilzer)]

*** Bug 14050 has been marked as a duplicate of this bug. ***