932719 - Zoom controls percentage label is too narrow when on the toolbar on OS X (at least on hidpi)

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[:Gijs (he/him)]

Attachment: Screen Shot 2013-10-30 at 11.20.25 .png

Comment 1

[Mike de Boer [:mikedeboer]]

It shouldn't be visible in the toolbar. This is a regression.

Comment 2

[:Gijs (he/him)]

I wonder what broke this...

Comment 3

[Dão Gottwald [:dao]]

(In reply to Mike de Boer [:mikedeboer] from comment #1)
> It shouldn't be visible in the toolbar. This is a regression.

This was a deliberate change. Bug 924933.

Comment 4

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

Attachment: Patch

Disabling the min-width on the toolbarbutton for the #zoom-reset-button was necessary to get the .toolbarbutton-text min-width to apply. Also tweaked the values for OSX and Windows to look better.

Comment 5

[:Gijs (he/him)]

Comment on attachment 824769 [details] [diff] [review]
Patch

Review of attachment 824769 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM!

Comment 6

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

https://hg.mozilla.org/projects/ux/rev/776725482f65

Comment 7

[Mike de Boer [:mikedeboer]]

Comment on attachment 824769 [details] [diff] [review]
Patch

Review of attachment 824769 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/shared/customizableui/panelUIOverlay.inc.css
@@ +410,5 @@
>    display: none;
>  }
>  
>  #zoom-controls[cui-areatype="toolbar"] > #zoom-reset-button > .toolbarbutton-text {
> +%ifdef XP_MACOSX

don't we have OS-specific panelUIOverlay.css files for this?

(I was looking at this patch, because I'm touching the widgets too ;) )

Comment 8

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

Yeah, we do. We need to have a larger discussion around when we think it is acceptable to place OS-specific styling side-by-side like here or in a separate file. For this case, I felt that it was better to have them close to each other instead of the OS-specific one overriding the shared style.

Comment 9

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/776725482f65