[webkit-reviews] review denied: [Bug 98803] Refactor shouldAddBorderPaddingMargin() : [Attachment 202318] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Robert Hogan <robert at webkit.org>'s
request for review:
Bug 98803: Refactor shouldAddBorderPaddingMargin()
https://bugs.webkit.org/show_bug.cgi?id=98803

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=202318&action=review


> Source/WebCore/rendering/RenderBlockLineLayout.cpp:374
> +	       if (checkStartEdge && parentContributesWidth) {

We don't need to check checkStartEdge here since we no longer update
checkStartEdge in shouldAddBorderPaddingMargin.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:375
> +		   checkStartEdge = parentContributesWidth;

You always set checkStartEdge to true here because parentContributesWidth is
always true in this branch.
What you need to do instead is to add an else clause and set checkStartEdge to
false there.
I'm surprised that none of our layout tests caught this. Maybe we need to a
test for this?
r- because of this logic change.