[Webkit-unassigned] [Bug 51828] [GTK] Port combo box painting to GtkStyleContext

[bugzilla-daemon at webkit.org]

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





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

> > > Source/WebCore/platform/gtk/RenderThemeGtk3.cpp:464
> > > +    gfloat xalign = 0.5;
> > > +    gfloat yalign = 0.5;
> > > +    if (direction != GTK_TEXT_DIR_LTR)
> > > +        xalign = 1.0 - xalign;

> > Doesn't this mean that xalign is always 0.5? If so wouldn't it just be better to use / 2 below and avoid xalign and yalign completely? In any case, yalign can go.

> Both xalign and yalign are 0.5 by default in gtk+, I left the code this way to make it closer to gtk+ code, so that it will be easier to maintain in case gtk+ code changes. 
> To center the arrow, this is also based on gtk+ code:
> http://git.gnome.org/browse/gtk+/tree/gtk/gtkarrow.c#n306

I still think it's better to use "/ 2" then here. We should favor clarity. The GTK+ code fetches the xalign and valign via gtk_misc_get_alignment. I think that's why it uses xalign and valign. Wouldn't it be safer to do the same thing here?

> > I'd prefer cairo_restore here.
> Me too :-), I thought I should use WebCore stuff always when possible.

Hrm. Interesting point. In my opinion, if code can be made simpler by using WebCore conveniences such as IntRect (which provides niceties like inflate), then I say we should use them. On the other hand, some WebCore classes exist only to wrap platform differences (like GraphicsContext). In platform-dependent code there is little reason to use them and it might add unecessary and unexpected overhead.

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