Closed Bug 1002931 Opened 9 years ago Closed 6 years ago

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

Categories

(Firefox :: Theme, defect)

29 Branch
All
Windows 8
defect
Not set
minor

Tracking

()

VERIFIED FIXED
Firefox 51
Tracking Status
firefox51 --- verified

People

(Reporter: MattN, Assigned: rakhisharma)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

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?
Flags: needinfo?(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.
Flags: needinfo?(shorlander)
Attached image bug 1002931.png
Stephen, can you please take a look at this? Screenshot and try build comment #4.
Flags: needinfo?(shorlander)
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.
Attachment #8772963 - Flags: review?(gijskruitbosch+bugs)
(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.
Assignee: nobody → Rakhish1994
Status: NEW → ASSIGNED
Flags: needinfo?(shorlander)
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/
Attachment #8772963 - Flags: review?(gijskruitbosch+bugs)
Attachment #8772963 - Flags: review?(gijskruitbosch+bugs)
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 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;
Attachment #8772963 - Flags: review?(jaws) → review+
Pushed by jwein@mozilla.com:
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.
https://hg.mozilla.org/mozilla-central/rev/4462f44d0bb9
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
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
(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!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.