[webkit-reviews] review denied: [Bug 127835] ASSERTION FAILED: x2 >= x1 in WebCore::RenderObject::drawLineForBoxSide : [Attachment 228846] Proposed patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Martin Hodovan
<mhodovan.u-szeged at partner.samsung.com>'s request for review:
Bug 127835: ASSERTION FAILED: x2 >= x1 in
WebCore::RenderObject::drawLineForBoxSide
https://bugs.webkit.org/show_bug.cgi?id=127835

Attachment 228846: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=228846&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=228846&action=review


> LayoutTests/ChangeLog:8
> +	   Test #1: Negative right margin + positive right padding
> +	   Test #2: Negative left margin + positive left padding

Tests only covert vertical. Should cover horizontal too.

> LayoutTests/ChangeLog:12
> +	   * fast/css/padding-margin-overlap-expected.txt: Added.

Please make this a reference test instead of a render-tree-dumping test.

> Source/WebCore/platform/graphics/LayoutRect.h:83
> +    bool hasPositiveDimensions() const;

Please don’t add this. It’s the same as !isEmpty. If you look at
IntSize::isEmpty and LayoutSize::isEmpty you will find that they are checking
for positive dimensions, not just non-zero.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1731
> +    if (!outerBorder.rect().hasPositiveDimensions())
> +	   return;

Should be:

    if (outerBorder.rect().isEmpty())
	return;