1407185 - Window drag space next to tabs is too big [Mac OS]

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Valentina Claudia Ona]

Attachment: Screen Shot 2017-10-10 at 10.33.03 (2).png

[Affected versions]:

- Nightly 58.0a1 
 
[Affected platforms]:

- Mac OS X 10.12
 
[Steps to reproduce]:

1. Open the browser and open many tabs until the tab strip is overflowed.  


Actual result:

The persistent drag space is bigger than in the specs. 

Expected result:

The persistent drag space should match the specs. https://mozilla.invisionapp.com/share/ZKBC94BPQ#/screens/229786513

[Note]: See the attached print screen with the actual result.

This is a follow-up from:
https://bugzilla.mozilla.org/show_bug.cgi?id=1397265#c32

Comment 1

[Dale Harvey (:daleharvey)]

Attachment: Bug 1407185 - Remove extra toolbar drag space on osx.

Review commit: https://reviewboard.mozilla.org/r/189502/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/189502/

Comment 2

[Dale Harvey (:daleharvey)]

Comment on attachment 8918690 [details]
Bug 1407185 - Remove extra toolbar drag space on osx.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/189502/diff/1-2/

Comment 3

[Dale Harvey (:daleharvey)]

Comment on attachment 8918690 [details]
Bug 1407185 - Remove extra toolbar drag space on osx.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/189502/diff/2-3/

Comment 4

[Dale Harvey (:daleharvey)]

So my first patch just gave osx a 34px width with the rest keeping with 40, bit of duplication code wise and doubt it would work great with mavericks fullscreen button

This seems a little riskier but fixes the alignment whatever is the rightmost in tabstoolbar

Comment 5

[Johann Hofmann [:johannh]]

Comment on attachment 8918690 [details]
Bug 1407185 - Remove extra toolbar drag space on osx.

https://reviewboard.mozilla.org/r/189502/#review194840

The space is caused by the fullscreen button placeholder (https://searchfox.org/mozilla-central/rev/40b456626e2d0409b7034768b4d9526fc7235ea4/browser/themes/osx/browser.css#93) and 2px padding on the last button.

Is this really the only thing we can do to prevent the placeholder from taking up space in OSX > 10.9? I haven't tested this on 10.9 but I have the feeling it wouldn't be correct there (the drag space would be too small). We should try not to show the placeholder when the fullscreen button is not shown.

What do you think?

Comment 6

[Dale Harvey (:daleharvey)]

Ah with the inspector I couldnt see the fullscreen placeholder taking up space but I can see its effect if I modify the placeholder margin, will do a patch to prevent the placeholder taking up space when it isnt used, cheers

Comment 7

[Dale Harvey (:daleharvey)]

Comment on attachment 8918690 [details]
Bug 1407185 - Remove extra toolbar drag space on osx.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/189502/diff/3-4/

Comment 8

[Johann Hofmann [:johannh]]

Comment on attachment 8918690 [details]
Bug 1407185 - Remove extra toolbar drag space on osx.

https://reviewboard.mozilla.org/r/189502/#review195616

This looks good to me. Make sure to coordinate with QA that the expected distance is 42px instead of 40px. I think it's fine not dealing with the extra complexity from removing the button padding right now (though I'd take a patch if someone is willing to work out the edge cases).

::: browser/themes/osx/browser.css:92
(Diff revision 4)
>  .titlebar-placeholder[type="caption-buttons"],
>  #titlebar-buttonbox {
>    margin-left: 7px;
>  }
>  
> +/* The fullscreen button doesnt show on Yosemite(10.9) or above so dont give it a

Nit: Yosemite is 10.10

Comment 9

[Dale Harvey (:daleharvey)]

> work out the edge cases

budum tsh

But yeh the extra 2px from the new tab button isnt particularly clear from the spec if that should collapse or not and if thats what UX would like then will do it in a follow up, will fix the 10.10 then land, cheers

Comment 10

[Dale Harvey (:daleharvey)]

Comment on attachment 8918690 [details]
Bug 1407185 - Remove extra toolbar drag space on osx.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/189502/diff/4-5/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/04a6b79edba9
Remove extra toolbar drag space on osx. r=johannh

Comment 12

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/04a6b79edba9

Comment 13

[Valentina Claudia Ona]

Attachment: Screen Shot 2017-10-20 at 07.08.18.png

I verified this issue using Nightly 58.0a1 with Build ID 20171020100426 on Mac OS X 10.12.In the right side, drag space is 40px but on the left side is still 47 px which is not correct. 
Please see the attachment.

Comment 14

[Dale Harvey (:daleharvey)]

Hey Valentina, I opened a new bug so we can explicitly decide whether the 40px should include or excluse the margins of the buttons @ https://bugzilla.mozilla.org/show_bug.cgi?id=1411309, this bug now just removes the definitely wrong extra margin on an invisible button

Comment 15

[Valentina Claudia Ona]

Based on fact that another bug was filled I will mark this as verified fixed.