Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0 ID:20180131100706 Ubuntu 17.10, OSX-Arc White theme, layout.css.devPixelsPerPx set to 1.5 When I activate the CSD through the customization pane, the window controls are drawn to the top edge of the window instead of being vertically centered in the tabbar like in other gnome apps such as gedit or nautilus. I am attaching a screenshot The buttons are also no longer spaced consistently with other apps (not enough space between the widgets) but maybe this is bug 1433092? If not I can file another bug

Bug 1433092 is not related here as it's about horizontal spacing.

Let's fix that at Bug 1419442.

Reopening per https://bugzilla.mozilla.org/show_bug.cgi?id=1419442#c23 - I'd fix that separately after all.

Comment on attachment 8952077 [details] Bug 1434621 - [Linux/Titlebar rendering] Center titlebar buttons, https://reviewboard.mozilla.org/r/221308/#review227136 Code analysis found 5 defects in this patch: - 5 defects found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: browser/base/content/browser-tabsintitlebar.js:238 (Diff revision 1) > let extraMargin = tabAndMenuHeight - titlebarContentHeight; > - if (AppConstants.platform != "macosx") { > + if (AppConstants.platform == "linux") { > + // Linux centers buttons at titlebar so let's emulate that. > + if (!fullMenuHeight) { > + // Menubar is not visible - center buttons according to tabs. > + titlebarContent.style.marginTop = extraMargin/2 + "px"; Error: Infix operators must be spaced. [eslint: space-infix-ops] ::: browser/base/content/browser-tabsintitlebar.js:239 (Diff revision 1) > - if (AppConstants.platform != "macosx") { > + if (AppConstants.platform == "linux") { > + // Linux centers buttons at titlebar so let's emulate that. > + if (!fullMenuHeight) { > + // Menubar is not visible - center buttons according to tabs. > + titlebarContent.style.marginTop = extraMargin/2 + "px"; > + titlebarContent.style.marginBottom = extraMargin/2 + "px"; Error: Infix operators must be spaced. [eslint: space-infix-ops] ::: browser/base/content/browser-tabsintitlebar.js:241 (Diff revision 1) > + if (!fullMenuHeight) { > + // Menubar is not visible - center buttons according to tabs. > + titlebarContent.style.marginTop = extraMargin/2 + "px"; > + titlebarContent.style.marginBottom = extraMargin/2 + "px"; > + } else { > + if (fullMenuHeight > titlebarContentHeight) { Error: Unexpected if as the only statement in an else block. [eslint: no-lonely-if] ::: browser/base/content/browser-tabsintitlebar.js:244 (Diff revision 1) > + titlebarContent.style.marginBottom = extraMargin/2 + "px"; > + } else { > + if (fullMenuHeight > titlebarContentHeight) { > + // Buttons are smaller than menubar so center them according to menubar. > + let fullMenuHeightMargin = fullMenuHeight - titlebarContentHeight; > + titlebarContent.style.marginTop = fullMenuHeightMargin/2 + "px"; Error: Infix operators must be spaced. [eslint: space-infix-ops] ::: browser/base/content/browser-tabsintitlebar.js:245 (Diff revision 1) > + } else { > + if (fullMenuHeight > titlebarContentHeight) { > + // Buttons are smaller than menubar so center them according to menubar. > + let fullMenuHeightMargin = fullMenuHeight - titlebarContentHeight; > + titlebarContent.style.marginTop = fullMenuHeightMargin/2 + "px"; > + titlebarContent.style.marginBottom = fullMenuHeightMargin/2 + (extraMargin - fullMenuHeightMargin) + "px"; Error: Infix operators must be spaced. [eslint: space-infix-ops]

I'm trying to figure out a simpler fix. How is this?

Comment on attachment 8952673 [details] [diff] [review] patch That's very good!

I also suggest to remove titlebar padding loaded from Gtk+ theme and apply only the one you added at #titlebar-buttonbox. Otherwise the titlebar is too big on themes which defines titlebar padding (like Adwaita).

Comment on attachment 8952739 [details] [diff] [review] titlebar padding patch Yeah, I figured we'd want to take this part of your patch but forgot about it.

Gonna land both patches in a bit.

Great, Thanks!

Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/cf8ac6e2c5e0 part 1: vertically center title bar buttons on Linux. r=stransky https://hg.mozilla.org/integration/mozilla-inbound/rev/134bfa14acf9 part 2: ignore vertical titlebar padding from the Gtk+ theme since we set it with CSS on #titlebar-buttonbox. r=dao

https://hg.mozilla.org/mozilla-central/rev/cf8ac6e2c5e0 https://hg.mozilla.org/mozilla-central/rev/134bfa14acf9