Spin buttons are currently rendered as a text field, we should paint the inner up/down buttons.
Created attachment 77195 [details] Implement spin buttons for gtk3 This patch implements spin buttons using gtk3 (I leave the gtk2 implementation to Martin :-P), the patch applies on top of the patch attached to bug 50820.
Nice patch! Should it be reviewed? It doesn't have an r? flag.
It doesn't have r because it doesn't apply, it depends on gtk-style-context port, so I didn't want bots to try to apply it.
Created attachment 78505 [details] Implement spin buttons in RenderThemeGtk3 Updated patch for current git master and with some other issues fixed.
Comment on attachment 78505 [details] Implement spin buttons in RenderThemeGtk3 View in context: https://bugs.webkit.org/attachment.cgi?id=78505&action=review > Source/WebCore/platform/gtk/RenderThemeGtk3.cpp:779 > + GtkBorder padding; > + gtk_style_context_get_padding(context, static_cast<GtkStateFlags>(0), &padding); > + > + int width = spinButtonArrowSize(context) + padding.left + padding.right; > + style->setWidth(Length(width, Fixed)); > + style->setMinWidth(Length(width, Fixed)); > +} Is it possible to just use reasonable values here rather than staying entirely true to the theme. If so, it might be better, as it will allow us to have the same layout regardless of the GTK+ theme / version.
(In reply to comment #5) > (From update of attachment 78505 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=78505&action=review > > > Source/WebCore/platform/gtk/RenderThemeGtk3.cpp:779 > > + GtkBorder padding; > > + gtk_style_context_get_padding(context, static_cast<GtkStateFlags>(0), &padding); > > + > > + int width = spinButtonArrowSize(context) + padding.left + padding.right; > > + style->setWidth(Length(width, Fixed)); > > + style->setMinWidth(Length(width, Fixed)); > > +} > > Is it possible to just use reasonable values here rather than staying entirely true to the theme. If so, it might be better, as it will allow us to have the same layout regardless of the GTK+ theme / version. gtk2 doesn't support spin buttons, so we can leave this and once gtk2 supports spin buttons, if the test results don't match we can try to use another value.
(In reply to comment #6) > gtk2 doesn't support spin buttons, so we can leave this and once gtk2 supports spin buttons, if the test results don't match we can try to use another value. Carlos convinced me recently that we can just switch all the bots over to GTK 3.x when the first stable release comes out. After that we can still keep a GTK+ 2.x builder around, but not run the tests there. This should help us avoid the metrics problem entirely.
Created attachment 79250 [details] Updated patch Removed juntion sides for the frame since we are only drawing the buttons, not the frame.
Comment on attachment 79250 [details] Updated patch 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. :)
Committed r76577: <http://trac.webkit.org/changeset/76577>