We need to correctly calculate titlebar button sizes from GTK+ CSS properties min-width / min-height and border/padding for the titlebar buttons.

Comment on attachment 8939774 [details] Bug 1427999 - Implement GetToolbarButtonMetrics() to get titlebar button size and border, Seems to be broken on some themes (Arc-Dark), I'll investigate it.

Comment on attachment 8939774 [details] Bug 1427999 - Implement GetToolbarButtonMetrics() to get titlebar button size and border, https://reviewboard.mozilla.org/r/210090/#review220210 ::: widget/gtk/gtk3drawing.cpp:28 (Diff revision 3) > static gboolean notebook_has_tab_gap; > > static ScrollbarGTKMetrics sScrollbarMetrics[2]; > static ToggleGTKMetrics sCheckboxMetrics; > static ToggleGTKMetrics sRadioMetrics; > +static ToolbarButtonGTKMetrics sToolbarButtonMetrics; You can make it sToolbarButtonMetrics[4] to cover maximize, minimize, restore, close but we can do that in another bug. ::: widget/gtk/gtk3drawing.cpp:308 (Diff revision 3) > gtk_style_context_get_style(style, "handle_size", size, NULL); > return MOZ_GTK_SUCCESS; > } > > +gint > +moz_gtk_titlebar_button_get_metrics(GtkBorder* aButtonBorder, This is somehow different implementation approach from how we done ScrollbarGTKMetrics and ToggleGTKMetrics. Please return pointer to ToolbarButtonGTKMetrics instead of providing arguments to be returned. I'm aware that you used other moz_gtk_..._get_metric as a template, but these calls are not caching data. ::: widget/gtk/gtk3drawing.cpp:313 (Diff revision 3) > +moz_gtk_titlebar_button_get_metrics(GtkBorder* aButtonBorder, > + gint* aButtonWidth, > + gint* aButtonHeight) > +{ > + if (!sToolbarButtonMetrics.initialized) { > + GtkStyleContext* style = GetStyleContext(MOZ_GTK_HEADER_BAR_BUTTON_CLOSE); Hm, I'm afraid we can use 'close' button style for every other buttons, at least if we want to mimic the behaviour of GTK CSS styles as much as possible. ::: widget/gtk/gtk3drawing.cpp:313 (Diff revision 3) > + GtkStyleContext* style = GetStyleContext(MOZ_GTK_HEADER_BAR_BUTTON_CLOSE); > + > + gint left = 0, top = 0, right = 0, bottom = 0; > + moz_gtk_add_margin_border_padding(style, &left, &top, &right, &bottom); Please move this closer to the usage, ie to: width += left + right height += top + bottom; ::: widget/gtk/gtk3drawing.cpp:341 (Diff revision 3) > + if (height < iconHeight) > + height = iconHeight; > + > + width += left + right; > + height += top + bottom; > + sToolbarButtonMetrics.minSizeWithBorder.width = width; Rename to minSizeWithBorderMargin to be precise ::: widget/gtk/gtk3drawing.cpp:2355 (Diff revision 3) > } > case MOZ_GTK_HEADER_BAR_BUTTON_CLOSE: > case MOZ_GTK_HEADER_BAR_BUTTON_MINIMIZE: > case MOZ_GTK_HEADER_BAR_BUTTON_MAXIMIZE: > { > - style = GetStyleContext(widget); > + GtkBorder border; This is no longer needed when the bug 1408335 lands, because buttons won't be used as containers anymore. Please don't touch this there and remove it in the 1408335 and move the MOZ_GTK_HEADER_BAR_BUTTON_\* below to the list of elements which are not containers. ::: widget/gtk/gtkdrawing.h:87 (Diff revision 3) > bool initialized; > MozGtkSize minSizeWithBorder; > GtkBorder borderAndPadding; > } ToggleGTKMetrics; > > +typedef struct { What should be sufficient to store is: - icon relative x/y position - for moz_gtk_header_bar_button_paint - min widget size - for GetMinimumWidgetSize

Comment on attachment 8944376 [details] Bug 1427999 - Use GetToolbarButtonMetrics() to get correct titlebar button size at nsNativeThemeGTK::GetMinimumWidgetSize, https://reviewboard.mozilla.org/r/214580/#review220478 ::: widget/gtk/nsNativeThemeGTK.cpp:1556 (Diff revision 1) > + case NS_THEME_WINDOW_BUTTON_CLOSE: > + case NS_THEME_WINDOW_BUTTON_MINIMIZE: > + case NS_THEME_WINDOW_BUTTON_MAXIMIZE: > + case NS_THEME_WINDOW_BUTTON_RESTORE: > + { > + moz_gtk_titlebar_button_get_metrics(nullptr, Please use the cached entries directly, like this do: https://searchfox.org/mozilla-central/source/widget/gtk/nsNativeThemeGTK.cpp#1398

Comment on attachment 8939774 [details] Bug 1427999 - Implement GetToolbarButtonMetrics() to get titlebar button size and border, https://reviewboard.mozilla.org/r/210090/#review220826 ::: widget/gtk/gtk3drawing.cpp:28 (Diff revision 3) > static gboolean notebook_has_tab_gap; > > static ScrollbarGTKMetrics sScrollbarMetrics[2]; > static ToggleGTKMetrics sCheckboxMetrics; > static ToggleGTKMetrics sRadioMetrics; > +static ToolbarButtonGTKMetrics sToolbarButtonMetrics; Let's do that as another bug as well as box spacing. ::: widget/gtk/gtk3drawing.cpp:313 (Diff revision 3) > +moz_gtk_titlebar_button_get_metrics(GtkBorder* aButtonBorder, > + gint* aButtonWidth, > + gint* aButtonHeight) > +{ > + if (!sToolbarButtonMetrics.initialized) { > + GtkStyleContext* style = GetStyleContext(MOZ_GTK_HEADER_BAR_BUTTON_CLOSE); We're going to fix that in another bug as well as the spacing.

Comment on attachment 8939774 [details] Bug 1427999 - Implement GetToolbarButtonMetrics() to get titlebar button size and border, https://reviewboard.mozilla.org/r/210090/#review220898

Comment on attachment 8944376 [details] Bug 1427999 - Use GetToolbarButtonMetrics() to get correct titlebar button size at nsNativeThemeGTK::GetMinimumWidgetSize, https://reviewboard.mozilla.org/r/214580/#review220900

Pushed by stransky@redhat.com: https://hg.mozilla.org/integration/autoland/rev/d1b018ec55a8 Implement GetToolbarButtonMetrics() to get titlebar button size and border, r=jhorak https://hg.mozilla.org/integration/autoland/rev/670be06477e0 Use GetToolbarButtonMetrics() to get correct titlebar button size at nsNativeThemeGTK::GetMinimumWidgetSize, r=jhorak

https://hg.mozilla.org/mozilla-central/rev/d1b018ec55a8 https://hg.mozilla.org/mozilla-central/rev/670be06477e0