[Webkit-unassigned] [Bug 51454] [GTK] Implement spin buttons in RenderThemeGtk
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jan 24 16:27:16 PST 2011
https://bugs.webkit.org/show_bug.cgi?id=51454
Martin Robinson <mrobinson at webkit.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #79250|review? |review+
Flag| |
--- Comment #9 from Martin Robinson <mrobinson at webkit.org> 2011-01-24 16:27:16 PST ---
(From update of attachment 79250)
View in context: https://bugs.webkit.org/attachment.cgi?id=79250&action=review
Looks good. Consider my suggestions below.
> Source/WebCore/ChangeLog:10
> + Paint inner up/down buttons for spin button elements when building
> + with GTK+ 3.x.
> +
Please make a note here that test results will land with the GTK+ 2.x version of this patch.
> Source/WebCore/platform/gtk/RenderThemeGtk3.cpp:761
> + const PangoFontDescription* fontDesc = gtk_style_context_get_font(context, static_cast<GtkStateFlags>(0));
Please don't abbreviate fontDesc.
> Source/WebCore/platform/gtk/RenderThemeGtk3.cpp:837
> + // Paint arrow centered inside button.
> + IntRect arrowRect;
> + gdouble angle;
> + if (arrowType == GTK_ARROW_UP) {
> + angle = 0;
> + arrowRect.setY(rect.y());
> + arrowRect.setHeight(rect.height() / 2 - 2);
> + } else {
> + angle = G_PI;
> + arrowRect.setY(rect.y() + buttonRect.y());
> + arrowRect.setHeight(rect.height() - arrowRect.y() - 2);
> + }
> + arrowRect.setWidth(rect.width() - 3);
> + if (direction == GTK_TEXT_DIR_LTR)
> + arrowRect.setX(rect.x() + 1);
> + else
> + arrowRect.setX(rect.x() + 2);
> +
> + gint width = arrowRect.width() / 2;
> + width -= width % 2 - 1; // Force odd.
> + gint height = (width + 1) / 2;
For quite some time, I wondered why this logic was so hairy and thought surely there must be a better way. Then I set out to implement this code for GTK+ 2.x. After some time doing that and looking at the GTK+ code, I understand completely now.
I think we should would leave a comment at the top explaining that we're trying to emulate gtkspinbutton.c as closely as possible here. I'd also prefer "int" to "gint" in most places. :)
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list