Closed Bug 1466330 Opened 2 years ago Closed 2 years ago

Make nsITheme::GetWidgetBorder return the border directly instead of using an out-param

Categories

(Core :: Widget, enhancement, P4, trivial)

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: mats, Assigned: mats)

Details

Attachments

(1 file)

It currently always returns NS_OK and no caller actually checks
what value it returned anyway...
Comment on attachment 8982810 [details] [diff] [review]
Make nsITheme::GetWidgetBorder return the border directly instead of using an out-param (idempotent patch)

Review of attachment 8982810 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, thank you!

::: gfx/src/nsITheme.h
@@ +56,5 @@
>   * the constants in nsThemeConstants.h).
>   */
>  class nsITheme: public nsISupports {
>  public:
> +  using LayoutDeviceIntMargin = mozilla::LayoutDeviceIntMargin;

nit: Doesn't need to be public I think

::: widget/windows/nsNativeThemeWin.cpp
@@ +2844,4 @@
>    switch (aWidgetType) {
>      case NS_THEME_GROUPBOX:
>      case NS_THEME_BUTTON:
> +      result.top = result.left = result.bottom = result.right = 2; 

nit: maybe fix the trailing whitespace while touching this line?

@@ +2863,4 @@
>        break;
>      case NS_THEME_STATUSBARPANEL:
>      case NS_THEME_RESIZERPANEL: {
> +      result.top = 1;      

And this?
Attachment #8982810 - Flags: review?(emilio) → review+
I fixed the nits as suggested, thanks.
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f62ecdf59b6
Make nsITheme::GetWidgetBorder return the border directly instead of using an out-param (idempotent patch).  r=emilio
https://hg.mozilla.org/mozilla-central/rev/1f62ecdf59b6
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.