[Webkit-unassigned] [Bug 156098] Replace all RenderTheme::popupInternalPadding methods with a single one returning a LengthBox

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=156098

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #275375|review?                     |review+
              Flags|                            |

--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 275375
  --> https://bugs.webkit.org/attachment.cgi?id=275375
Try to fix mac builds

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

> Source/WebCore/rendering/RenderTheme.h:187
> +    virtual LengthBox popupInternalPaddingBox(const RenderStyle&) const { return LengthBox(0); }

Not sure this function needs the word “Box” in its name. Also could write:

    return { 0 };

Instead of return LengthBox(0);

> Source/WebCore/rendering/RenderThemeGtk.cpp:767
> +        return LengthBox(0);

Again, I prefer:

    return { 0 };

> Source/WebCore/rendering/RenderThemeGtk.cpp:782
> +    return LengthBox(borderWidth.top + focusWidth, borderWidth.right + focusWidth + (style.direction() == LTR ? minArrowSize : 0),
> +        borderWidth.bottom + focusWidth, borderWidth.left + focusWidth + (style.direction() == RTL ? minArrowSize : 0));

I like the syntax where we just write:

    return { a, b, c, d };

Rather than naming the type LengthBox.

> Source/WebCore/rendering/RenderThemeIOS.mm:535
> +        return LengthBox(0, MenuListButtonPaddingRight + style.borderTopWidth(), 0, 0);
> +    return LengthBox(0);

Ditto.

> Source/WebCore/rendering/RenderThemeMac.mm:1374
> +        return LengthBox(popupButtonPadding(controlSizeForFont(style))[topPadding] * style.effectiveZoom(),
> +            popupButtonPadding(controlSizeForFont(style))[rightPadding] * style.effectiveZoom(),
> +            popupButtonPadding(controlSizeForFont(style))[bottomPadding] * style.effectiveZoom(),
> +            popupButtonPadding(controlSizeForFont(style))[leftPadding] * style.effectiveZoom());

Ditto.

> Source/WebCore/rendering/RenderThemeMac.mm:1381
> +        return LengthBox(styledPopupPaddingTop * style.effectiveZoom(),
> +            static_cast<int>(ceilf(arrowWidth + (arrowPaddingLeft + arrowPaddingRight + paddingBeforeSeparator) * style.effectiveZoom())),
> +            styledPopupPaddingBottom * style.effectiveZoom(), styledPopupPaddingLeft * style.effectiveZoom());

Ditto.

> Source/WebCore/rendering/RenderThemeMac.mm:1384
> +    return LengthBox(0);

Ditto.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160401/b2ea57e8/attachment-0001.html>