1002931 - "Title bar" button in cust. mode has less left padding than the other footer buttons without icons

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Matthew N. [:MattN]]

Attachment: beta_106_tabsOutsideTitlebar_fourPinned_normal_onlyNavBar_noLWT_customizing.png

Comment 1

[:Gijs (he/him)]

Stephen, this looks like it's inconsistent between platforms (for instance, on win8 the "Themes" button which also has an icon has "wide" padding, but on OS X it has narrow padding, matching the title bar toggle button, but not the toolbar one...), can you clarify what we want to do here?

Comment 2

[Stephen Horlander [:shorlander]]

Not sure if this is defined anywhere but to maintain consistency buttons should have a min-width and consistent padding. 

Since these are based on the incontent controls found in about:preferences I looked at those. It looks like they have a left and right padding of 10px. They should have a min-width but don't appear to in all cases? Or it's variable? Not sure, maybe XUL weirdness?

Anyway, I think we should pick-up the padding-left/right: 10px from the other incontent controls.

Comment 3

[Rakhi(:rakhisharma)]

Attachment: Bug 1002931- "Title bar" button in cust. mode has less left padding than the other footer buttons without icons. .

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

Comment 4

[Rakhi(:rakhisharma)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a8d157a504d

Comment 5

[Rakhi(:rakhisharma)]

Attachment: bug 1002931.png

Stephen, can you please take a look at this? Screenshot and try build comment #4.

Comment 6

[:Gijs (he/him)]

Comment on attachment 8772963 [details]
Bug 1002931- "Title bar" button in cust. mode has less left padding than the other footer buttons without icons. .

https://reviewboard.mozilla.org/r/65632/#review62668

This is unfortunately not enough, when you actually run the code and measure. The problem is that, at least on OS X (but I'm fairly sure the same happens on Windows), the label and icon inside the buttons have margin/padding of their own. You would need to normalize that in order to get the styling completely right. You'd probably want to reset the margin/padding to 0 for both the .button-text and .button-icon members, and for the .button-menu-dropmarker. Then you'd need to ensure there's still enough spacing (margin/padding) between the label and any icons before or dropmarkers afterwards, per button.

Comment 7

[:Gijs (he/him)]

(In reply to Rakhi(:rakhisharma) from comment #5)
> Created attachment 8772967 [details]
> bug 1002931.png
> 
> Stephen, can you please take a look at this? Screenshot and try build
> comment #4.

You can see in the screenshot that the padding is now 14px in front of images, and 19px after dropmarkers, so I'm clearing the ni for Stephen.

Comment 8

[Rakhi(:rakhisharma)]

Comment on attachment 8772963 [details]
Bug 1002931- "Title bar" button in cust. mode has less left padding than the other footer buttons without icons. .

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

Comment 9

[Rakhi(:rakhisharma)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=67ec58df938a

Comment 10

[:Gijs (he/him)]

Comment on attachment 8772963 [details]
Bug 1002931- "Title bar" button in cust. mode has less left padding than the other footer buttons without icons. .

https://reviewboard.mozilla.org/r/65632/#review64184

This looks OK to me on Linux. Unfortunately we need to also set:

```
margin-inline-start: 0; /* on .customizationmode-button .button-icon */
```
and

```
margin-inline-end: 0; /* on .customizationmode-button:not([type=menu]) .button-text */
```

and

```
margin-inline-end: 0;
padding-inline-end: 0; /* both on .customizationmode-button .button-menu-dropmarker */
```

to make this work on OS X.

Comment 11

[Rakhi(:rakhisharma)]

Comment on attachment 8772963 [details]
Bug 1002931- "Title bar" button in cust. mode has less left padding than the other footer buttons without icons. .

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

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8772963 [details]
Bug 1002931- "Title bar" button in cust. mode has less left padding than the other footer buttons without icons. .

https://reviewboard.mozilla.org/r/65632/#review68104

::: browser/themes/shared/customizableui/customizeMode.inc.css:138
(Diff revision 3)
> +  border: 0px;
> +  padding-inline-start: 0px;
> +  padding-inline-end: 0px;

nit, use 0 instead of 0px.

Also, use border-width: 0; instead of border: 0px;

Comment 13

[Rakhi(:rakhisharma)]

Comment on attachment 8772963 [details]
Bug 1002931- "Title bar" button in cust. mode has less left padding than the other footer buttons without icons. .

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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4462f44d0bb9
"Title bar" button in cust. mode has less left padding than the other footer buttons without icons. r=jaws.

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4462f44d0bb9

Comment 16

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 31.0a1 (2014-04-28) on Windows 7 , 64 Bit !
 
This bug's fix is verified with latest Nightly 

Build ID   20160824030337
User Agent Mozilla/5.0(Windows NT 6.1; Win64; x64; rv:51.0) Gecko/20100101 Firefox/51.0

Comment 17

[:Gijs (he/him)]

(In reply to Maruf Rahman from comment #16)
> I have reproduced this bug with Nightly 31.0a1 (2014-04-28) on Windows 7 ,
> 64 Bit !
>  
> This bug's fix is verified with latest Nightly 
> 
> Build ID   20160824030337
> User Agent Mozilla/5.0(Windows NT 6.1; Win64; x64; rv:51.0) Gecko/20100101
> Firefox/51.0

Thanks!