[webkit-reviews] review granted: [Bug 71944] CSS 2.1 failure: outline-color-applies-to* tests fail : [Attachment 121898] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has granted Robert Hogan
<robert at webkit.org>'s request for review:
Bug 71944: CSS 2.1 failure: outline-color-applies-to* tests fail
https://bugs.webkit.org/show_bug.cgi?id=71944

Attachment 121898: Patch
https://bugs.webkit.org/attachment.cgi?id=121898&action=review

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=121898&action=review


> Source/WebCore/rendering/RenderTableRow.cpp:221
> +void RenderTableRow::paintOutlineForRow(PaintInfo& paintInfo, const
LayoutPoint& paintOffset)

Nit: paintOutlineForRowIfNeeded may be better as you may not paint it?

> Source/WebCore/rendering/RenderTableRow.cpp:225
> +    if ((paintPhase != PaintPhaseOutline && paintPhase !=
PaintPhaseSelfOutline) || style()->visibility() != VISIBLE)
> +	   return;

I was the one to mention the early return but on second thoughts it is
inconsistent with the rest of the painting code and thus not that great. Better
to be consistent here than following my mumblings!

> Source/WebCore/rendering/RenderTableSection.cpp:1092
> +		       if (row && !row->hasSelfPaintingLayer())

I don't think it is possible for |row| to be NULL here as the recalcCells was
called. Is the |row| check really required or can it be removed?

> Source/WebCore/rendering/RenderTableSection.cpp:1115
> +		   if (row && !row->hasSelfPaintingLayer())

Same question here.