I originally intended to just fix the double focus ring we're getting on gtk2 for radio and checkbox elements, but I ended up totally rethinking the way we lay out the elements inside the checkbox container. The net result of all of this is that with my patch, we get pixel-perfect spacing of checkbox and radio controls with every theme included on Fedora Core 1 (with a couple of minor problems, detailed below). So here are the main changes I made: - First, to fix the focus ring problems, I pay much better attention to the interior_focus property. - Previously to work around themes which misreport the indicator size, I just forced the indicator to be 14px minimum and hoped for the best. What I do now is I have the indicator (i.e. .checkbox-check) take up all of the padding on all sides of it. That is, it extends all the way to the left, top, and bottom of the container, and all the way over to the label on the right. This allows the "slop" to come into the padding between the indicator and the label, which is what happens on native controls. - Added a new CHECKBOX/RADIO_LABEL theme part to handle drawing the focus border. - Some misc cleanups to the gtkdrawing API -- no longer check all out parameters for NULL (you have to pass in pointers for all out-params), and fold all of the scrollbar metrics into a struct. I tried not to regress gtk1 here but I can't promise things didn't shift by a pixel or two. I did test that things show up ok when theming gets disabled on gtk2 (i.e. a bad theme engine). Now, the minor problems I mentioned: 1. gtk has what I'd consider a minor bug in the way it calculates a height to request for check/radio buttons. In particular, temp = indicator_size + indicator_spacing * 2; requisition->height = MAX (requisition->height, temp) + 2 * (focus_width + focus_pad); In the case where the indicator and its vertical padding are taller than the requested height of the child, it will take the indicator size and add the focus border to that. I don't believe this is optimal for interior focus because it results in requesting more space than you need. For example, suppose the indicator and its padding require 17px height, the child requires 16px height, and the focus border requires 4px height (2 on top, 2 on bottom). Using this formula, it will take the indicator's 17px and add 4px to that to get a height of 21px. But in reality it would be quite happy with 20px, since that provides enough space for the interior focus border around the child. The net effect here is that with larger indicator sizes mozilla is "missing" some vertical padding, but IMO the result often looks better than native controls, where the extra rows end up outside the interior focus outline. 2. In gtk_default_draw_flat_box, there's a check that causes it to call gdk_draw_rectangle rather than gtk_style_apply_default_background if GDK_IS_PIXMAP(window). So even if there's a bg pixmap it doesn't draw it if it's rendering into a pixmap. I can't figure out the point of that check; things seem to work fine if I remove it (gtk_style_apply_default_background seems to be able to handle rendering into a pixmap fine).
Oh, also: I had to add some extra boxes to the xbl bindings for these widgets to get things to continue to look correct if theming gets disabled. In particular, the right border of the .checkbox-check can't be used to supply the padding between the indicator and the label when theming is disabled, since the border is used to paint the checkbox itself. So I had to add an outer box that could supply the padding, as a border. Also, I wanted the indicator to stretch to the full vertical height of the control (-moz-box-align: stretch), but the label needs to be centered (-moz-box-align: center). So I made the outer box stretch and put the label in an inner box which has center alignment.
Attachment #151193 - Flags: superreview?(blizzard) → superreview+
15 years ago
Attachment #151193 - Flags: review?(caillon) → review+
Is there a particular reason to stick with -moz-box-align styles when dealing with anonymous nodes? I realize that setting align="center" on a bound element is probably a bad idea but I thought on anonymous nodes it was acceptable.
attachment #151193 [details] [diff] [review] reverts quite a few licenses from the css files back to NPL. Why?
(In reply to comment #3) > Is there a particular reason to stick with -moz-box-align styles when dealing > with anonymous nodes? I realize that setting align="center" on a bound element > is probably a bad idea but I thought on anonymous nodes it was acceptable. Doesn't really matter to me one way or the other. (In reply to comment #4) > attachment #151193 [details] [diff] [review] reverts quite a few licenses from the css files back to NPL. > Why? Because I copied the files wholesale from toolkit/skin/gtk2 (now toolkit/themes/gnomestripe), where they had been copied from toolkit/skin/win (now toolkit/themes/winstripe).
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
Hey brian, I think this patch broke the thunderbird account wizard on the aviary 1.0 branch. We are failing to get values back from the radio buttons: See Bug #252655
Comment on attachment 151193 [details] [diff] [review] patch a=mkaply
Attachment #151193 - Flags: approval1.7.x+
You need to log in before you can comment on or make changes to this bug.