[Webkit-unassigned] [Bug 52258] [GTK] Move toggle button rendering out of gtk2drawing.c

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Martin Robinson <mrobinson at webkit.org>  2011-01-13 09:15:20 PST ---
(In reply to comment #5)

> Unofficial review.

Thanks for the review!

> > Source/WebCore/platform/gtk/RenderThemeGtk2.cpp:106
> > +    gint focusWidth = 0, focusPad = 0;
> > +    gboolean interiorFocus = 0;
> 
> Style properties return the default value when they are not set, so we don't need to initialize these variables.

Done.

> > Source/WebCore/platform/gtk/RenderThemeGtk2.cpp:219
> > +        widgetContext.gtkPaintFocus(focusRect, widget, GTK_STATE_ACTIVE, "checkbutton");
> 
> Why GTK_STATE_ACTIVE, I think you should use the current widget state, getGtkStateType(theme, renderObject).

Mozilla was using always using GTK_STATE_ACTIVE, but it doesn't seem to break rendering to always use the widget state, so I have changed it.

> > Source/WebCore/platform/gtk/RenderThemeGtk2.cpp:225
> > +    if (!info.context->paintingDisabled())
> 
> We don't need this, I already removed it from RenderThemeGtk3, RenderTheme.cpp already checks it every time it calls any painting method.
> 
> > Source/WebCore/platform/gtk/RenderThemeGtk2.cpp:237
> > +    if (!info.context->paintingDisabled())
> 
> Ditto.

Removed.

> > Source/WebCore/platform/gtk/WidgetRenderingContext.cpp:178
> > +    GdkRectangle paintRect = { m_paintRect.x + rect.x(), m_paintRect.y + rect.y(), rect.width(), rect.height() };
> I think we should paint the indicator centered, mozilla code did it 

This isn't actually the location of the indicator, but the location that the indicator will be painted onto the scratch buffer.

> I guess here you mean gtk_toggle_button_set_inconsistent(GTK_TOGGLE_BUTTON(widget), indeterminate);

Nice catch. Fixed.

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