Bug 232650 – Verify borderRect is Renderable

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brandon]

Update whether an object is renderable

[Brandon]

<rdar://82980915>

[Brandon]

Created attachment 443157 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 443157 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=443157&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests needed.

You need some more words here for why this change makes sense. It looks like it was copied from FloatRoundedRect::isRenderable(), so say that.

What's the consequence of the different behaviors of RoundedRect and FloatRoundedRect? Explain here.

[Brandon]

Created attachment 443237 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 443237 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=443237&action=review

> Source/WebCore/ChangeLog:3
> +        Update whether an object is renderable

You can be more specific than "on object". It's about rounded rects.

> Source/WebCore/ChangeLog:8
> +        No new tests needed.

Just remove this line.

[Brandon]

Created attachment 443268 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 443268 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=443268&action=review

> Source/WebCore/ChangeLog:5
> +        https://bugs.webkit.org/show_bug.cgi?id=232650
> +

Please put the radar number here too.

[Brandon]

Created attachment 445318 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 445318 [details]
Patch

How do we know in blendMixedTypes that negative values should not be allowed? For example, negative margins, or top/left are perfectly valid.

[Brandon]

Created attachment 445616 [details]
patch

[zalan]

Comment on attachment 445616 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=445616&action=review

> Source/WebCore/platform/graphics/RoundedRect.cpp:175
> +    return m_radii.topLeft().width()     >= 0 && m_radii.topLeft().height()     >= 0
> +        && m_radii.bottomLeft().width()  >= 0 && m_radii.bottomLeft().height()  >= 0
> +        && m_radii.topRight().width()    >= 0 && m_radii.topRight().height()    >= 0
> +        && m_radii.bottomRight().width() >= 0 && m_radii.bottomRight().height() >= 0
> +        && m_radii.topLeft().width()    + m_radii.topRight().width()     <= m_rect.width()
> +        && m_radii.bottomLeft().width() + m_radii.bottomRight().width()  <= m_rect.width()
> +        && m_radii.topLeft().height()   + m_radii.bottomLeft().height()  <= m_rect.height()
> +        && m_radii.topRight().height()  + m_radii.bottomRight().height() <= m_rect.height();

some weird spacing going on here.

[Brandon]

Created attachment 445726 [details]
patch

[EWS]

Committed r286449 (244791@main): <https://commits.webkit.org/244791@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 445726 [details].