[Webkit-unassigned] [Bug 51812] [GTK] Port buttons painting to GtkStyleContext

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #78227|review?                     |review+
               Flag|                            |




--- Comment #5 from Martin Robinson <mrobinson at webkit.org>  2011-01-07 09:45:21 PST ---
(From update of attachment 78227)
View in context: https://bugs.webkit.org/attachment.cgi?id=78227&action=review

Looks great! Please make the following minor adjustments before landing.

> WebCore/platform/gtk/RenderThemeGtk3.cpp:115
> +    GtkStyleContext* context;

Please initialize this to zero.

> WebCore/platform/gtk/RenderThemeGtk3.cpp:135
> +    adjustRectForFocus(context, rect);

Please ASSERT(context); here just for correctness sake.

> WebCore/platform/gtk/RenderThemeGtk3.cpp:226
> +    gboolean interiorFocus;
> +    gtk_style_context_get_style(context, "interior-focus", &interiorFocus, NULL);

This is only used inside the if (isFocused(...)) block, so it makes sense to move it there.

> WebCore/platform/gtk/RenderThemeGtk3.cpp:274
> +            buttonRect.move(borderWidth.left + focusPad, borderWidth.top + focusPad);
> +            buttonRect.setWidth(buttonRect.width() - (2 * focusPad + borderWidth.left + borderWidth.right));
> +            buttonRect.setHeight(buttonRect.height() - (2 * focusPad + borderWidth.top + borderWidth.bottom));

Instead of setting this piecewise, I think it makes more sense to set it all at once:
buttonRect = IntRect(borderWidth.left + focusPad, borderWidth.top + focusPad.
                     buttonRect.width() - (2 * focusPad + borderWidth.left + borderWidth.right),
                     buttonRect.height() - (2 * focusPad + borderWidth.top + borderWidth.bottom));

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.