768506 - Toolbar buttons should have a min-height

Status: RESOLVED DUPLICATE of bug 1208715

(Firefox :: Theme, defect)

Description

[Johan C]

Attachment: screenshot 1

A min-width should keep buttons added by extensions in check.
The "List all tabs" button could use a min-width as well.

See 'screenshot 1' for the extension case (chatzilla).
See 'screenshot 2' for the "List all tabs" case.

Comment 1

[Johan C]

Attachment: screenshot 2

Comment 2

[Johan C]

This concerns the hovered and selected states obviously.

Comment 3

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

The screenshots attached show more of a need for a min-height to me.

That being said, I'm not sure if we should do this or not.

Comment 4

[Johan C]

(In reply to Jared Wein [:jaws] from comment #3)
> The screenshots attached show more of a need for a min-height to me.

You are quite right, I have no idea why I wrote width rather than height.

Fixed the summary.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

(In reply to Jared Wein [:jaws] from comment #3)
> The screenshots attached show more of a need for a min-height to me.
> 
> That being said, I'm not sure if we should do this or not.

I think it makes sense to have minimum (yet overridable) dimensions on these toolbarbuttons, for consistency. What are the reasons to not do this?

Comment 6

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

(In reply to Mike Conley (:mconley) from comment #5)
> (In reply to Jared Wein [:jaws] from comment #3)
> > The screenshots attached show more of a need for a min-height to me.
> > 
> > That being said, I'm not sure if we should do this or not.
> 
> I think it makes sense to have minimum (yet overridable) dimensions on these
> toolbarbuttons, for consistency. What are the reasons to not do this?

Add-on compat. We can make it overridable, but some add-ons won't update to override the styling. Although I don't know how many add-ons *want* it to be shorter than the default.

Comment 7

[Dão Gottwald [:dao]]

We shipped this button styling long ago -- adding a min-height seems unrelated to all the other Australis work currently happening.

Comment 8

[:Gijs (out for now; he/him)]

This was implemented in bug 1208715.