GNOME Bugzilla – Bug 664342
CSS border-width fixes for GtkNotebook and GtkFrame
Last modified: 2011-11-20 21:40:34 UTC
See individual patches for rationale.
Created attachment 201672 [details] [review] notebook: make sure to allocate the CSS border width Instead of taking only the CSS padding into account when allocating the notebook children, also allocate the border width.
Created attachment 201673 [details] [review] frame: add GTK_STYLE_CLASS_FRAME in _init() Instead of adding it every time we use the GtkStyleContext, just add it in _init().
Created attachment 201674 [details] [review] frame: make sure to allocate the CSS border width Similar to GtkNotebook, GtkFrame was only allocating space for the padding width, and not the border. This could be seen by just running tests/testframe. With a theme that renders frame borders, setting xthickness = 0 in the test draws the button border over the frame border, which is wrong.
Created attachment 201675 [details] [review] notebook: make sure to allocate the CSS border width Instead of taking only the CSS padding into account when allocating the notebook children, also allocate the border width.
Created attachment 201676 [details] [review] frame: add GTK_STYLE_CLASS_FRAME in _init() Instead of adding it every time we use the GtkStyleContext, just add it in _init().
Created attachment 201677 [details] [review] frame: make sure to allocate the CSS border width Similar to GtkNotebook, GtkFrame was only allocating space for the padding width, and not the border. This could be seen by just running tests/testframe. With a theme that renders frame borders, setting xthickness = 0 in the test draws the button border over the frame border, which is wrong.
Review of attachment 201676 [details] [review]: Looks fine to me
Review of attachment 201677 [details] [review]: Would be cool to have an actual test for that... Is this get_padding_and_border something that should be available somewhere centrally ? Seems to be the same in notebook and frame, does this show up anywhere else ?
Review of attachment 201675 [details] [review]: Looks fine to me, if the CSS masters agree that this is the proper thing to do.
(In reply to comment #8) > Is this get_padding_and_border something that should be available somewhere > centrally ? Seems to be the same in notebook and frame, does this show up > anywhere else ? > Every widget should respect - and draw - border, padding and margin style properties and the GTK margin properties. It's something I hope to fix via my Views stuff, see https://mail.gnome.org/archives/gtk-devel-list/2011-November/msg00022.html by introducing a GtkCssBox or so that takes care of these things. Of course, we can add an internal _gtk_style_context_get_box_size(GtkStyleContext *context, GtkBorder *out_box) function that takes care of it in the meantime.
About the patches, I do think we should draw the border in any case (as demanded by CSS). And we should use the notebook->show-tabs and frame->shadow_type properties to set style classes (probably .frame in both cases).
(In reply to comment #11) > About the patches, I do think we should draw the border in any case (as > demanded by CSS). And we should use the notebook->show-tabs and > frame->shadow_type properties to set style classes (probably .frame in both > cases). After discussing this on IRC we decided it's best to push these fixes as-is for now; the problem is that we allow widget-specific API calls (such as gtk_notebook_set_show_border) to override how a widget is drawn. By doing what's proposed in comment #11 we would break users of those APIs who set e.g. show-border to FALSE (or shadow-type to NONE) with a theme that doesn't follow a new logic. I think what we need is a way to tweak style for internal pieces of complex stock widgets, but we probably need another set of APIs for that.
(In reply to comment #8) > Is this get_padding_and_border something that should be available somewhere > centrally ? Seems to be the same in notebook and frame, does this show up > anywhere else ? Unfortunately whether to fetch the border depends on private state of the widget itself; probably not worth trying to abstract this until we have a GtkCssBox element that can take care of it automatically.