Follow on from https://bugzilla.mozilla.org/show_bug.cgi?id=1407185 Spec is @ https://mozilla.invisionapp.com/share/ZKBC94BPQ#/screens/229786513 The spec mentioned that the drag space is 40px wide both left and right, currently the drag space is 40px width, however the buttons they are next to (the osx window controls on the left, or the new tab / overflow button on the right) have their own margin giving the drag space a visual width of 47px on the left and 42px on the right. Currently only tested this on OSX, I assume it will effect other platforms

Stephen do we want to collapse these so its 40px including the margins from the buttons, or leave as is?

(In reply to Dale Harvey (:daleharvey) PTO till 23rd from comment #1) > Stephen do we want to collapse these so its 40px including the margins from > the buttons, or leave as is? Yes, it should be a total of 40px including that space (assuming it's draggable space).

On OSX there is always a button to the right when the placeholder appears and the window controls so I figured the simplest patch is to account for them in the placeholder width, modifying the window control margins (or the last button margins) to be aware of the placeholder seemed much more complicated

Comment on attachment 8925013 [details] Bug 1411309 - Remove margin next to chrome controls and drag space. https://reviewboard.mozilla.org/r/196262/#review201838 Maybe we can keep these styles shared, let me know if you have any questions :) ::: browser/themes/linux/browser.css:532 (Diff revision 1) > > /* Tabstrip */ > > %include ../shared/tabs.inc.css > > +.titlebar-placeholder[type="pre-tabs"], It's not relevant if we move this back to shared, but I don't believe Linux has drag space, so this wouldn't be necessary I think. ::: browser/themes/osx/browser.css:767 (Diff revision 1) > +/* > + * We want placeholder total width to be 40px, this accounts for the margins > + * of persistent buttons next to the placeholder > + */ > +.titlebar-placeholder[type="pre-tabs"] { > + width: 33px; I'd prefer if this stayed in shared and we subtracted the 7px margin from https://searchfox.org/mozilla-central/rev/5a60492a53667fc61a62af1847d005a210b7a4f6/browser/base/content/browser-tabsintitlebar.js#252 (Because then everything will be sized correctly) ::: browser/themes/osx/browser.css:770 (Diff revision 1) > + */ > +.titlebar-placeholder[type="pre-tabs"] { > + width: 33px; > +} > +.titlebar-placeholder[type="post-tabs"] { > + width: 38px; This should be 40px - var(--toolbarbutton-outer-padding), to account for UI density (and future changes). Also, I don't understand why we don't need to apply this on Windows. There's also an outer padding on buttons there. Can't we move this back to shared?

(In reply to Johann Hofmann [:johannh] from comment #5) > > +.titlebar-placeholder[type="pre-tabs"], > > It's not relevant if we move this back to shared, but I don't believe Linux > has drag space, so this wouldn't be necessary I think. CAN_DRAW_IN_TITLEBAR support for Linux is on its way (bug 1283299). > > +.titlebar-placeholder[type="post-tabs"] { > > + width: 38px; > > This should be 40px - var(--toolbarbutton-outer-padding), to account for UI > density (and future changes). --toolbarbutton-outer-padding is part of the buttons and doesn't add to the drag space, so we shouldn't use that here: (In reply to Stephen Horlander [:shorlander] from comment #2) > Yes, it should be a total of 40px including that space (assuming it's > draggable space).

(In reply to Dão Gottwald [::dao] from comment #6) > > > +.titlebar-placeholder[type="post-tabs"] { > > > + width: 38px; > > > > This should be 40px - var(--toolbarbutton-outer-padding), to account for UI > > density (and future changes). > > --toolbarbutton-outer-padding is part of the buttons and doesn't add to the > drag space, so we shouldn't use that here: Ah, good point, though I wonder if this really needs to be strictly 40px draggable space instead of 38px. In that case the general approach here would be wrong and we'd need to eliminate the padding on the button somehow. I agree with Dale that that sounds a little less trivial and I was under the impression that we're making it 40px mostly for visual consistency, not for being able to drag the 2px next to the button (who does that?). Would changing this padding to be margin work?

I think we can just leave this as-is at 42px visually, 40px draggable.

> I think we can just leave this as-is at 42px visually, 40px draggable. On window controls placeholder we are 47px visual and 40px draggable, leave it as is and close this WONTFIX, or you think we should fix up the margin next to the controls?

Is that margin draggable?

Yup, clicking anywhere that doesnt register as a different button is draggable for me on osx

(In reply to Dão Gottwald [::dao] from comment #10) > Is that margin draggable? Regardless, I believe that fixing up the window control width to be correct in browser-tabsintitlebar.js is something we should do. :)

(In reply to Dale Harvey (:daleharvey) PTO till 23rd from comment #11) > Yup, clicking anywhere that doesnt register as a different button is > draggable for me on osx Then we should take this into account for that spacer.

Comment on attachment 8925013 [details] Bug 1411309 - Remove margin next to chrome controls and drag space. https://reviewboard.mozilla.org/r/196262/#review201902 ::: browser/base/content/browser-tabsintitlebar.js:171 (Diff revision 2) > > // Get the height of the tabs toolbar: > let fullTabsHeight = rect($("TabsToolbar")).height; > > - // Buttons first: > - let captionButtonsBoxWidth = rect($("titlebar-buttonbox-container")).width; > + // Buttons first: The caption buttons are given a 7 px left margin which is > + // included in their width, but would like to remove that from being double "included in their width" is a little misleading, what's happening here is that we're taking the width of the container of the window controls (#titlebar-buttonbox), so that any margins add up to it. On another thought, have you tried just taking the dimensions of #titlebar-buttonbox instead of #titlebar-buttonbox-container and see how that works out? Maybe that's an even more elegant solution, OTOH there could be a reason why we're using the container here... ::: browser/base/content/browser-tabsintitlebar.js:174 (Diff revision 2) > > - // Buttons first: > - let captionButtonsBoxWidth = rect($("titlebar-buttonbox-container")).width; > + // Buttons first: The caption buttons are given a 7 px left margin which is > + // included in their width, but would like to remove that from being double > + // counted for a right margin as the drag space now sits flush next > + // to the buttons > + let captionButtonsBoxWidth = rect($("titlebar-buttonbox-container")).width - 7; This is only on OSX, right? Probably should move it into the if condition below.

> On another thought, have you tried just taking the dimensions of #titlebar-buttonbox > instead of #titlebar-buttonbox-container and see how that works out? > Maybe that's an even more elegant solution, OTOH there could be a reason why > we're using the container here... Previously we wanted to double count the margin here as the right also needed a margin, but as windows doesnt have / want that margin and we dont want it on osx any more then not using the container works perfect, good idea

Comment on attachment 8925013 [details] Bug 1411309 - Remove margin next to chrome controls and drag space. https://reviewboard.mozilla.org/r/196262/#review202212 Looks good to me, having only tested on OSX. Thank you!

Pushed by dharvey@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c464c8f2c682 Remove margin next to chrome controls and drag space. r=johannh

For verification, on the right hand side will be 42px from the last button to the edge of the window with 40px being draggable and 2px being the margin of the last button. the space between the window controls and the first tab border is 40px exactly

I verified this issue using latest Nightly 58.0a1 on Windows 7 x64, Windows 10 x64 and Mac OS 10.13 with Build ID 20171107220115. I will mark this as verified fixed.