1176865 - The amount of work required for add-ons to use high-res icons in the nav-bar on Windows is ridiculous

Status: RESOLVED INVALID

(Firefox :: Theme, defect)

Description

[Mike Kaply [:mkaply]]

Attachment: Addon that shows the problem

In our add-on, we have buttons, menu buttons and menu-buttons. In order to have these display properly in high resolution on Windows, we have to have the following rules:

/* For Windows 4K */
@media (min-resolution: 2dppx) and (-moz-os-version: windows-xp),
       (min-resolution: 2dppx) and (-moz-os-version: windows-vista),
       (min-resolution: 2dppx) and (-moz-os-version: windows-win7),
       (min-resolution: 2dppx) and (-moz-os-version: windows-win8) {
  toolbar#nav-bar #buttontest-button .toolbarbutton-icon {
    width: 32px; /* 16 pixel image, 1 pixel border, 7 pixel padding left and right */
    height: 24px; /* 16 pixel image, 1 pixel border, 3 pixel padding top and bottom */
  }
  toolbar#nav-bar #buttontest-menubutton .toolbarbutton-icon {
    width: 41px; /* 16 pixel image, 1 pixel border, 7 pixel padding left, 17 pixel padding right */
    height: 24px; /* 16 pixel image, 1 pixel border, 3 pixel padding top and bottom */
  }
  toolbar#nav-bar #buttontest-menu-button .toolbarbutton-icon {
    width: 31px; /* 16 pixel image, 1 pixel border (except on right), 7 pixel padding left and right */
    height: 24px; /* 16 pixel image, 1 pixel border, 3 pixel padding top and bottom */
  }
}

This is simply silly.

The choice to place the borders and padding on the images in the toolbar instead of the buttons creates headaches like these.

It should be easier to use High res icons on Windows.

Comment 1

[:Gijs (he/him)]

You don't need the media query at all, and if you do want to do this only for hidpi, just use:

@media (min-resolution: 1.1dppx) {
}

Built-in Firefox has the same rule for the width of the icon. You don't need a rule for the height of the icon; assuming a square image or moz-image region, it will size appropriately.

Comment 2

[Mike Kaply [:mkaply]]

Builtin Firefox does NOT have this rule because it doesn't have 16x16 icons. It has 18x18 icons. And it has no custom rules for high res add-on icons (at least that I could find).

If you add a 32x32 pixel icon for your add-on at 2dppx, you have to specify a width/height on the toolbarbutton-icon or the icon will be oversized. You can try this and see.

The problem is that the width and height on the nav-bar are not 16x16 because of the padding/border. So you have to add custom sizes. And you have to do that only on Windows.

So this is a problem.

Comment 3

[:Gijs (he/him)]

(In reply to Mike Kaply [:mkaply] from comment #2)
> Builtin Firefox does NOT have this rule

Please do not call something a lie unless you're sure:

http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser.css#689

> because it doesn't have 16x16 icons.
> It has 18x18 icons.

Which have different padding, so it still sums to a width of 32px.

> And it has no custom rules for high res add-on icons (at
> least that I could find).

It does, see deps of bug 1023511.

> If you add a 32x32 pixel icon for your add-on at 2dppx, you have to specify
> a width/height on the toolbarbutton-icon or the icon will be oversized. You
> can try this and see.

Look, I reviewed and wrote some of the patches in the deps. I know how it works. My point was you don't need the "height" part, just the width; the height will autosize in the toolbar cases, afaik.

> The problem is that the width and height on the nav-bar are not 16x16
> because of the padding/border. So you have to add custom sizes.
> And you have to do that only on Windows.

Actually, you need to size things on other OSes for hidpi as well. The sizes are just different.

You can fix the fact that you need different styling by using the right flags on your chrome manifest package for the css file overrides in question.

In any case, ideally you should already be using different icons on different OSes if you're so anxious to fit in.

> So this is a problem.

I disagree. This has been discussed before. We need a bigger click surface on Windows for platform parity, which is why the sizing works the way it does. We don't have a different way of doing this.

One alternative you have, if you dislike the current code so much, is not using the builtin styling and do things yourself entirely. Of course that would mean your click surface is "weird" on Windows. Don't know if that bothers you or not.

A second alternative would be to use background-image and background-size to use "straightforward" sizing.

Comment 4

[Mike Kaply [:mkaply]]

If I read that rule correctly, it only applies for primaryButtons or nestedButtons

http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/browser.inc#5

Is there a way to use CSS to add your own buttons to that list?

All I know is that if I use a 32x32 icon for my add-on on a HiDPI Windows machine, it's too big.

The only way I've found to solve that is to set the width to 32 (not 16, which seems like the logical thing to do).

Comment 5

[:Gijs (he/him)]

(In reply to Mike Kaply [:mkaply] from comment #4)
> If I read that rule correctly, it only applies for primaryButtons or
> nestedButtons
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/browser.
> inc#5
> 

Yes. My point was, we need to do the same thing and didn't think it was too ugly, so I'm not sure why we should change our mind just because add-ons need to do the same thing.

We can't really add this rule for toolbarbutton-1 because too many add-ons don't have 16x16 icons and we would break them.

> Is there a way to use CSS to add your own buttons to that list?

Not that I know of.

> All I know is that if I use a 32x32 icon for my add-on on a HiDPI Windows
> machine, it's too big.

Sure. So you need the width: property set, but not the height: one.