Gtk+ uses spacing between titlebar buttons - the buttons are places at vbox with default spacing 6pix.

Comment on attachment 8952128 [details] Bug 1433092 - Add spacing around titlebar buttons, https://reviewboard.mozilla.org/r/221362/#review227466 ::: commit-message-ad133:1 (Diff revision 1) > +Bug 1433092 - Add spacing among titlebar buttons, r?jhorak s/among/around ::: widget/gtk/gtk3drawing.cpp:313 (Diff revision 1) > return MOZ_GTK_SUCCESS; > } > > +// We support LTR layout only here for now. > +static void > +InitAddToolbarButtonSpacing(WidgetNodeType aWidgetType, The AddTollbarButtonSpacing as a name of the function should be better, because I don't see any initialization there. ::: widget/gtk/gtk3drawing.cpp:346 (Diff revision 1) > + buttonSpacing /= 2; > + > + static_assert(TOOLBAR_BUTTONS >= 4, > + "We need at least 4 titlebar buttons available."); > + // It's a first button - apply spacing to right side only. > + if (buttonPosition == 0) { Please use switch there. ::: widget/gtk/gtk3drawing.cpp:363 (Diff revision 1) > + *spacingLeft += buttonSpacing; > + } > +} > + > static void > InitToolbarButtonMetrics(ToolbarButtonGTKMetrics *aButtonMetrics, There's no initialization of static vars in this function, it just returns ToolbarButtonGTKMetrics, please rename to GetToolbarButtonMetric or similar. ::: widget/gtk/gtk3drawing.cpp:400 (Diff revision 1) > + &aButtonMetrics->buttonMargin.right); > + > gint left = 0, top = 0, right = 0, bottom = 0; > - moz_gtk_add_margin_border_padding(style, &left, &top, &right, &bottom); > + moz_gtk_add_border_padding(style, &left, &top, &right, &bottom); > > + // Buton size is calculated as min-width/height + border/padding. typo Buton/Button ::: widget/gtk/gtk3drawing.cpp:418 (Diff revision 1) > + aButtonMetrics->minSizeWithBorderMargin.height = height + > + aButtonMetrics->buttonMargin.top + aButtonMetrics->buttonMargin.bottom; > } > > -const ToolbarButtonGTKMetrics* > -GetToolbarButtonMetrics(WidgetNodeType aWidgetType) > +static void > +InitAllToolbarButtonMetrics(void) Hm, there seems to be a lot of Init functions which starts to be confusing. InitAllToolbarButtonMetrics -> EnsureToolbarMetrics (or keep the InitToolbarButtonMetrics) InitToolbarButtonMetrics -> CreateToolbarButtonMetrics InitAddToolbarButtonSpacing -> AddToolbarButtonSpacing or consider better names.

Comment on attachment 8952128 [details] Bug 1433092 - Add spacing around titlebar buttons, https://reviewboard.mozilla.org/r/221362/#review228574 ::: widget/gtk/gtk3drawing.cpp:364 (Diff revision 8) > -const ToolbarButtonGTKMetrics* > -GetToolbarButtonMetrics(WidgetNodeType aWidgetType) > +// We support LTR layout only here for now. > +static void > +CalculateToolbarButtonSpacing(WidgetNodeType aWidgetType) > +{ > + int buttonIndex = (aWidgetType - MOZ_GTK_HEADER_BAR_BUTTON_CLOSE); > + ToolbarButtonGTKMetrics* metrics = sToolbarMetrics.button + buttonIndex; Could you please rather pass ToolbarButtonGTKMetrics as a function argument to avoid direct access to the static/global variable? You won't need to compute the buttonIndex after that. Same for CalculateToolbarButtonMetrics. Ideally that we use sToolbarMetrics only in GetToolbarButtonMetrics and EnsureToolbarMetrics. ::: widget/gtk/gtk3drawing.cpp:395 (Diff revision 8) > + metrics->minSizeWithBorderMargin.height += > + metrics->buttonMargin.top + metrics->buttonMargin.bottom; > +} > + > +static int > +GetGtkHeaderBarLayout(WidgetNodeType* aButtonLayout, int aMaxButtonNums) nit: please use GetGtkHeaderBarButtonLayout ::: widget/gtk/gtk3drawing.cpp:398 (Diff revision 8) > + > +static int > +GetGtkHeaderBarLayout(WidgetNodeType* aButtonLayout, int aMaxButtonNums) > +{ > + NS_ASSERTION(aMaxButtonNums >= TOOLBAR_BUTTONS, > + "We're missing titlebar buttons!"); This is a bit misleading, please use something like: Requested number of buttons is higher than storage capacity. ::: widget/gtk/gtk3drawing.cpp:439 (Diff revision 8) > + > + return activeButtonNums; > +} > + > +static void > +EnableAndPlaceTitlebarButtons() I think you can put this into EnsureToolbarMetrics or possibly join with the code in EnsureToolbarMetrics. ::: widget/gtk/gtk3drawing.cpp:456 (Diff revision 8) > + } > + // Mark last button. > + if (i == (activeButtonNums-1)) { > + metrics->lastButton = true; > + } > + } ...and call CalculateToolbar\* stuff there.

Comment on attachment 8952128 [details] Bug 1433092 - Add spacing around titlebar buttons, https://reviewboard.mozilla.org/r/221362/#review229532

Pushed by stransky@redhat.com: https://hg.mozilla.org/integration/autoland/rev/360486fecfe8 Add spacing around titlebar buttons, r=jhorak

The spacing isn't exactly the same as other GNOME applications like Gedit (there's more spacing in Firefox compared to Gedit). I'll wait for bug 1434646 to be fixed before filing a bug.

I noticed that the spacing between the window control buttons and the vertical border is slightly off. See attachment (Arc GTK theme - top: Firefox; bottom: Nautilus) On a different note: In Arc GTK theme the window buttons of *inactive* windows are faded to grayscale, while in Firefox with drawInTitlebar enabled they are not. Should I make a seperate bug report for this or is it on purpose?

(In reply to 3ship from comment #16) > On a different note: In Arc GTK theme the window buttons of *inactive* > windows are faded to grayscale, while in Firefox with drawInTitlebar enabled > they are not. Should I make a seperate bug report for this or is it on > purpose? It's not on purpose, please file a bug. Thanks!