Closed Bug 616472 Opened 14 years ago Closed 13 years ago

Unify sizes of toolbarbutton images to make life easier for extensions

Categories

(Firefox :: Theme, defect)

Product:
Firefox
Component:
Theme
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: ancestor.ak, Assigned: dmosedale)

References

(
URL
)

Details

(Whiteboard: [target-betaN][softblocker])

Attachments

(2 files, 5 obsolete files)

Linux large icons appear to be 24x24 edge-to-edge
122.72 KB, image/png
Details
windows patch, v6
1.23 KB, patch
dao
: review+
Details | Diff | Splinter Review 
Currently, extensions have to provide at least 4 different images to cover all the platforms: 1 for Windows, 1 for Mac, 2 for Linux. 3 of them have the same basic icon size - they only differ in padding, which was included in the images instead of being specified with CSS.

This is a really unfortunate decision that creates a lot of unnecessary work for extension developers. It forces additional images, CSS files, and conditional lines in chrome.manifest for what could easily be taken care of automatically with a saner design. This is a little ridiculous from the extension developer's perspective. And this is just for Firefox 4, it gets even more complicated if you want to support 3.x as well.

Padding should be specified with CSS. Extensions wouldn't have to worry about padding at all and wouldn't have to deal with Mac and Windows separately.

I know it is unlikely to be changed this late in the cycle. However, it will be even less likely post-Firefox 4 because it would mean breaking extensions yet again. I wish I have filed this bug sooner but just like some other authors I didn't appreciate the situation is until Jorge's post yesterday.
blocking2.0: --- → ?
Blocks: abp
Dão, Markus,

I can redesign the toolbar glyphs on Mac and Windows to be 16x16. I don't think any of them really are larger than that now and the added pixel glow on Windows doesn't seem to be needed anyway.

Would it be possible to accommodate 16x16 glyphs instead of 20x20 and 18x18 as they are now?

The same would apply for the Linux glyphs I am working on.
We need to get all of the images sizes to be the same.  But I still want us to actively encourage add-on developers to use different artwork for different platforms.  For instance, OS X icons should be monochrome.  Obviously not that many add-on developers will go to the extra work of providing platform specific artwork, but even with all the icon sizes the same that should be our recommendation.
I agree with the recommendation, and that is mentioned in the post. But we should definitely do whatever is technically possible to make the themes cross-platform. In this case trimming some white space and adding some extra CSS to the toolbar buttons styles would go a long way to help developers.
I think the decision here must be made ASAP. Like, for real. 

Right now is the time when most authors are starting to update their extensions. The t-shirt contest has been launched to encourage them. Add-on-Con is next week. If we decide to do it, it has to be announced immediately, so that their work isn't wasted.

Though to be honest, I suspect many aren't going to bother matching the theme perfectly in the current situation and are gonna go with 16x16 all around anyway. Of course, they will have to adjust the padding manually, thus screwing up most non-default themes.
> But I still want us to
> actively encourage add-on developers to use different artwork for different
> platforms.  For instance, OS X icons should be monochrome

Is there any CSS transform/filter we can use to make the icons monochrome on Mac?
(In reply to comment #4)
> Right now is the time when most authors are starting to update their
> extensions. The t-shirt contest has been launched to encourage them. Add-on-Con
> is next week. If we decide to do it, it has to be announced immediately, so
> that their work isn't wasted.

As far as the UX team is concerned, 16x16 is the recommendation, with possibility for a grayscale version on OS X. If this seems hard to do or unreasonable in any way, let us know ASAP.
(In reply to comment #6) 
> As far as the UX team is concerned, 16x16 is the recommendation, with
> possibility for a grayscale version on OS X. If this seems hard to do or
> unreasonable in any way, let us know ASAP.

I just tested creating a toolbar button using a 16x16 image, and it was stretched to 20x20. If the UX team is recommending 16x16, then the default theme should allow that without any additional coding by add-on developers.
This was on Mac OS, sorry for the bugspam.
>Is there any CSS transform/filter we can use to make the icons monochrome on
>Mac?

Creating new artwork would really provide better results.  Converting a complex color icon to a simple monochrome glyph isn't really something that an automated filter can do very well.  Unless perhaps both icons were starting in svg, and we had some built in way of removing unneeded details in the artwork (but that would be really experimental).
> (In reply to comment #4)
> As far as the UX team is concerned, 16x16 is the recommendation, with
> possibility for a grayscale version on OS X. If this seems hard to do or
> unreasonable in any way, let us know ASAP.

Yes, it does. I don't know how much clearer I can make it while keeping it polite.

The way the default theme is implemented makes it unnecessarily complicated and laborious for extensions to fit it. It is borderline ridiculous. It could be easily avoided with a better implementation, as was the case in all the previous Firefox versions. This opinion is shared by other experienced extension developers as well, as can find out by reading comments under the official guide linked from this bug.
(In reply to comment #10)
> > (In reply to comment #4)
> > As far as the UX team is concerned, 16x16 is the recommendation, with
> > possibility for a grayscale version on OS X. If this seems hard to do or
> > unreasonable in any way, let us know ASAP.
> 
> Yes, it does. I don't know how much clearer I can make it while keeping it
> polite.

Yes the theme would have to change for this. This is acknowledged. As per Comment 1 hopefully Markus or Dão can let us know if this is doable.
(In reply to comment #10)
> Yes, it does. I don't know how much clearer I can make it while keeping it
> polite.

I was asking for reasons of why we *shouldn't* do it. Your concerns have already been stated, heard, and we agree with them. :)
(In reply to comment #12)
> (In reply to comment #10)
> > Yes, it does. I don't know how much clearer I can make it while keeping it
> > polite.
> 
> I was asking for reasons of why we *shouldn't* do it. Your concerns have
> already been stated, heard, and we agree with them. :)

Oh, sorry for the confusion. :) 

Yeah, sounds good. This will allow extensions to cover almost all the cases at once, with the sole exception of 24x24 in large icons mode on Linux. Just to throw an idea out there, maybe we could set a special class for that so that extensions don't have to provide platform-specific CSS files and chrome.manifest declarations just for this single case?
Is the large/small icon thing a common practice on Linux?

Given that small versus large doesn't really mean anything on Mac/Windows anymore (just changing from keyhole to non keyhole), why are we keeping it around?
(In reply to comment #14)
> Is the large/small icon thing a common practice on Linux?
> 
> Given that small versus large doesn't really mean anything on Mac/Windows
> anymore (just changing from keyhole to non keyhole), why are we keeping it
> around?

The plan is to get rid of it. Didn't make it for 4.0, though — more important things to fix. It's on our list. :)
(In reply to comment #1)
> Would it be possible to accommodate 16x16 glyphs instead of 20x20 and 18x18 as
> they are now?

Sure! I can make the Mac patch as soon as I have an updated Toolbar.png.
Dão, any objections from you?

Whether it's possible to make a monochrome filter for OS X with acceptable results is something we can still experiment with after we've fixed the sizes.
(In reply to comment #16)
> (In reply to comment #1)
> > Would it be possible to accommodate 16x16 glyphs instead of 20x20 and 18x18 as
> > they are now?
> 
> Sure! I can make the Mac patch as soon as I have an updated Toolbar.png.
> Dão, any objections from you?

It's not needed, see bug 547419 for example.
Oh, right. So I just need to port bug 547419 to pinstripe?

I'm a little confused about what happens in large icons mode, vs. what this bug requests. Extension toolbar button icons are still larger than 16x16 in large icons mode, right? So why does this bug report say that you only need one icon for Windows?
On Windows and Mac, large and small icon mode on Firefox 4 use the same icons.

So even though addons have a large icon, we wouldn't want to use that on FF 4 since it would be too big for the design.

Basically what we're looking for is a way to use our 16x16 icons as is.
So, in other words you're asking for iconsize="small" to always be set, even when the back button is round, right? Otherwise your extension stylesheet won't set the right icon.

Or, if you're willing to make small changes to the extension stylesheet, would something like this work for you?

#mybutton {
  list-style-image: <large icon>;
}

toolbar[iconsize="small"] #mybutton {
  list-style-image: <small icon>;
}

/* Selector that only matches in Firefox 4 */
:-moz-any(toolbar)[iconsize="large"] #mybutton {
  list-style-image: <small icon>;
}

But in order for that to look good (i.e. without scaling) we'd have to change the default theme to make large add-on icons 16x16, too, instead of the current 18x18.
(In reply to comment #20)
> So, in other words you're asking for iconsize="small" to always be set, even
> when the back button is round, right? Otherwise your extension stylesheet won't
> set the right icon.

That would be perfect because it would allow all extensions to function without changes.

> /* Selector that only matches in Firefox 4 */
> :-moz-any(toolbar)[iconsize="large"] #mybutton {
>   list-style-image: <small icon>;
> }

No, it matches only in Gecko 2.0 which isn't the same thing as Firefox 4. It would also match in SeaMonkey 2.1 for example but I am pretty sure that they won't change their icon sizes.

So the real solution would be to register an additional stylesheet for Firefox 4 in chrome.manifest (with appID condition) or do something similar programmatically.

> But in order for that to look good (i.e. without scaling) we'd have to change
> the default theme to make large add-on icons 16x16, too, instead of the current
> 18x18.

Yes, that's what this request is mainly about (both Windows and OS X icons seem to be essentially 16x16 icons with some padding yet extension authors are supposed to supply 18x16 and 20x20 icons).
(In reply to comment #21)
> (In reply to comment #20)
> > So, in other words you're asking for iconsize="small" to always be set, even
> > when the back button is round, right? Otherwise your extension stylesheet won't
> > set the right icon.
> 
> That would be perfect because it would allow all extensions to function without
> changes.

Dão, what's your opinion on this?
I wonder how that would work with 3rd-party themes.

> > /* Selector that only matches in Firefox 4 */
> > :-moz-any(toolbar)[iconsize="large"] #mybutton {
> >   list-style-image: <small icon>;
> > }
> 
> No, it matches only in Gecko 2.0 which isn't the same thing as Firefox 4. It
> would also match in SeaMonkey 2.1 for example but I am pretty sure that they
> won't change their icon sizes.

Point taken.

> So the real solution would be to register an additional stylesheet for Firefox
> 4 in chrome.manifest (with appID condition) or do something similar
> programmatically.

Is this too much to ask of extension authors? I realize that it's not ideal, but it doesn't seem like an excessive amount of work to me.

> > But in order for that to look good (i.e. without scaling) we'd have to change
> > the default theme to make large add-on icons 16x16, too, instead of the current
> > 18x18.
> 
> Yes, that's what this request is mainly about

Dão, your opinion? :)
(In reply to comment #22)
> > So the real solution would be to register an additional stylesheet for Firefox
> > 4 in chrome.manifest (with appID condition) or do something similar
> > programmatically.
> 
> Is this too much to ask of extension authors? I realize that it's not ideal,
> but it doesn't seem like an excessive amount of work to me.

Personally I don't really care which approach is taken, one more app-specific hack won't make my code blow up. Firefox-only extensions might have more of an issue with that. What I really need are definitive criteria when to use which icon. "Use 16x16 unless you are on Linux and large icons are switched on" is still a little too shaky for my taste, especially given that some themes will certainly decide to stick with 24x24 icons for the "large icons" scenario. So I wouldn't mind an "iconsize" attribute that should be observed instead of "mode" if it is present. It could be a value that is specified in the theme so that third-party themes can choose a different size.
(In reply to comment #22)
> > So the real solution would be to register an additional stylesheet for Firefox
> > 4 in chrome.manifest (with appID condition) or do something similar
> > programmatically.
> 
> Is this too much to ask of extension authors? I realize that it's not ideal,
> but it doesn't seem like an excessive amount of work to me.

In my opinion it is acceptable. I filed this bug with the intention of minimizing the amount of stylesheets required to cover all the platforms under Firefox 4. Two separate stylesheets for 4.0 and pre-4.0 doesn't seem unreasonable. However, if we can figure out an elegant solution to avoid it, it would be great. I think Wladimir's suggestion from comment #23 is very good.

Right now in Firefox 4, iconsize="large" and iconsize="small" have the same actual icon size, except on Linux. Extensions would have to create an additional stylesheet for Linux even after we fix the padding issues. 

Decoupling iconsize from mode is a simple solution to both cross-platform and cross-version problems. If "large" and "small" really mean what they say, extensions won't have to worry about what platform, version, or theme they are running under. They could just provide the icon in two sizes and do:

toolbar[iconsize="large"] #mybutton {
  list-style-image: icon-24x24.png;
}

toolbar[iconsize="small"] #mybutton {
  list-style-image: icon-16x16.png;
}

...and the rest would be taken care of.
Depends on: 618096
I've filed bug 618096 with a patch for 16x16 extension icons in all modes on Mac.
(In reply to comment #24)
> toolbar[iconsize="large"] #mybutton {
>   list-style-image: icon-24x24.png;
> }
> 
> toolbar[iconsize="small"] #mybutton {
>   list-style-image: icon-16x16.png;
> }

I would rather make things explicit with iconsize="24" and iconsize="16". This allows for themes with non-standard icon sizes like 32x32 (particularly important for SeaMonkey that has different icon sizes for the *default* themes on the *same* platform), and it is also more future-proof (what if Firefox 9.3 decides to use 20x20 icons?).
(In reply to comment #5)
> > But I still want us to
> > actively encourage add-on developers to use different artwork for different
> > platforms.  For instance, OS X icons should be monochrome
> 
> Is there any CSS transform/filter we can use to make the icons monochrome on
> Mac?

Here's the result of a quick experiment:
http://tests.themasta.com/monochrome-filter/

If this filter was applied to all extension buttons by default, and could be disabled by setting filter: none on the button, would that be acceptable for extension authors?
> Here's the result of a quick experiment:
> http://tests.themasta.com/monochrome-filter/

That's pretty impressive. I wouldn't mind as long as I could turn it off.

We should make it easy to allow a custom monochrome without a mac specific CSS though

icontype="monochrome" or the like
Depends on: 618324
(In reply to comment #28)
> We should make it easy to allow a custom monochrome without a mac specific CSS
> though

You're right, a new attribute is better. I've filed bug 618324 about the filtering, feel free to brainstorm attribute names there. :)
Whiteboard: [target-betaN]
this is a huge win for add-on developers in a release that otherwise makes their job much harder in this area. blocking+.
blocking2.0: ? → betaN+
Since there are more addon-developer in this bug, please take a look at bug 618324 to discuss what the attribute should be called for Mac monochrome buttons.
Unassigned soft blocker, over to zpao for investigation.

Seems this bug has wandered off into the weeds a bit. I think all we should for FF4 (_can_ do, at this point) is what's suggested by comment 0/1, which is to move padding included in the images to CSS instead, such that the image sizes are more consistent across platforms?
Assignee: nobody → paul
Whiteboard: [target-betaN] → [target-betaN][softblocker]
Yes, if we are to do it at all, this is the minimum that must be done for Firefox 4. Unlike other soft blockers, this is not something that can be easily fixed later - in a minor release or in Firefox 5. It would become a trade-off between making things saner for newly developed extensions and *yet again* breaking existing extensions whose authors have gone through the trouble of adjusting to this mess. In fact, it is a problem already.

I hate to engage in advocacy, but I believe I am expressing sentiments of many add-on developers when I say that this situation is a complete debacle. 

We are talking about an incompatible change affecting a very large number of add-ons which should have been made as early as possible. Add-on authors have warned about the issue at least since July and the UX team was aware of it, but it dropped the ball. Finally, very late in the cycle, the scale of the problem was acknowledged and the decision was made to fix it ASAP. Following which the ball was dropped again - over a month and two betas later the bug remains unfixed. 

I have too much respect for people at Mozilla to suggest this is all anything else than just a management failure. But at this point, I am not surprised that other add-on developers see it as lack of respect for their work and commitment.
Taking for further investigation.
Assignee: paul → dmose
So, bug 618096 has landed and made it into Beta 9, so now we have:
 - Windows: 18x18 in large icons mode, 16x16 in small icons mode
 - Mac: 16x16 in all icon modes
 - Linux: the icon's intrinsic size in all modes

It looks like we should at least use 16x16 in large icons mode on Windows, too.

And an attribute like icondimensions="16x16" on toolbars would be really
helpful.

I have no idea what to do about Linux since we don't use a new toolbarbutton
style there.

Dão, I'd be really grateful if you could chime in with your thoughts.
Once bug 616156 is fixed, icons could use their intrinsic size on Windows except when placed on the nav bar.

Not sure how icondimensions="16x16" is supposed to work exactly...
(In reply to comment #36)
> Once bug 616156 is fixed, icons could use their intrinsic size on Windows
> except when placed on the nav bar.

How, and what would that look like? I don't really understand what the patch there does, could you please elaborate?
Also, is the patch supposed to be ported to Mac?

(In reply to comment #36)
> Not sure how icondimensions="16x16" is supposed to work exactly...

For example, during browser startup we could add a dummy extension button to every toolbar, save the icon's computed size in the toolbar's icondimensions attribute and remove the button again. Then extension stylesheets would have a way of assigning a correctly-sized icon, no matter what size the theme set.
(In reply to comment #37)
> (In reply to comment #36)
> > Once bug 616156 is fixed, icons could use their intrinsic size on Windows
> > except when placed on the nav bar.
> 
> How, and what would that look like? I don't really understand what the patch
> there does, could you please elaborate?

It limits the new custom styling to the nav bar and uses native toolbar button styling for other toolbars, much like in 3.6.

> Also, is the patch supposed to be ported to Mac?

Mac doesn't have a useful default toolbar button appearance, I think, so no.

> > Not sure how icondimensions="16x16" is supposed to work exactly...
> 
> For example, during browser startup we could add a dummy extension button to
> every toolbar, save the icon's computed size in the toolbar's icondimensions
> attribute and remove the button again. Then extension stylesheets would have a
> way of assigning a correctly-sized icon, no matter what size the theme set.

We could do that, but it sounds like it would have impact on Twinopen.
(In reply to comment #38)
> (In reply to comment #37)
> > (In reply to comment #36)
> > > Once bug 616156 is fixed, icons could use their intrinsic size on Windows
> > > except when placed on the nav bar.

Uhh, I had misread that as "even when placed on the nav bar"... nevermind.

In my understanding this bug is mainly about the nav bar anyway (right?), so bug bug 616156 isn't going to change much about the problem.

> > > Not sure how icondimensions="16x16" is supposed to work exactly...
> > 
> > For example, during browser startup we could add a dummy extension button to
> > every toolbar, save the icon's computed size in the toolbar's icondimensions
> > attribute and remove the button again. Then extension stylesheets would have a
> > way of assigning a correctly-sized icon, no matter what size the theme set.
> 
> We could do that, but it sounds like it would have impact on Twinopen.

Could be, but testing it doesn't hurt.
(In reply to comment #39)
> (In reply to comment #38)
> > We could do that, but it sounds like it would have impact on Twinopen.
> 
> Could be, but testing it doesn't hurt.

And if the CSS sizing test has a sensible performance issue, then the
"icondimensions" attribute could simply be persisted like it's done
for "currentset", and its value invalidated when the theme changes.

Regarding the attribute's value, while having the information about the
icon's computed dimensions might be useful for extensions with very smart
custom code, I don't think CSS alone can select a rule based on a mathematical
condition on an attribute. In other words, if an extension has only a 16x16
icon and a 24x24 icon, it can't automatically choose using CSS which of
the two images should be used when icondimensions="26x22" or
icondimensions="32x32", without covering every possible case
explicitly.

Thus, it would also be handy to set iconsize="small" when icondimensions
computes to a value less than "24x24", and "large" otherwise. This should
make the selection of the right icon very easy in 99% of the cases,
compatible with all themes and platforms, and backwards compatible with
Firefox 3.6 on all platforms. Special CSS rules could then be added in
order to support, for example, the case for icondimensions="26x22" and
icondimensions="32x32", if an extension ever wants to do that.

I'm volunteering to experiment with this approach if this is agreed upon
and the task isn't already assigned to someone else. However, I can test
only on Windows, I'd need some help for testing the appearance on other
platforms.
So, Atul and I have been poking at this as well, and I've just floated a proposal out-of-band to mstange and dao, which, in retrospect, I probably should have floated here to minimize confusion.

To summarize, it appears to me that this can be scoped into two separable and complementary pieces:

1) Clean up all the small icon sizes to be 16x16 with the CSS set up so that the same icon can be used on all 3 platforms for small, as well as on Windows and Mac for large.  This means that the bare minimum amount of icons that an add-on author has to supply is one 16x16 icon and one 24x24 icon (which is used only for Linux large).

1a) shorlander has volunteered to generate 16x16 icons for windows to replace the 18x18 ones.  I just spoke to him today, and he's already working on this.

1b) we should clean up all the 16x16 icons & CSS such that they use padding the same way so that authors who so desire can use a single 16x16 icon and have it work everywhere.

2) Do some JS / CSS / attribute magic as has been discussed here by Paolo, Dao, and MStange to allow chrome.manifest contortions to be replaced by more elegant CSS rules.

Is there anything missing or incorrect about this characterization?

If not, it's my belief that 1) is higher priority than 2), but they'll both make a material difference to add-on authors.

Atul and I have all three platforms at hand, and we're willing to sign up to drive the work for 1) on all three platforms.  Furthermore, Paolo could start poking at 2) as a separate patch, and if that bears fruit, I can help drive that into the tree on the other platforms as well.

Thoughts?
(In reply to comment #41)
> Is there anything missing or incorrect about this characterization?

(1) is spot on and it's exactly what add-on authors require. I don't really understand what (2) is about.
(In reply to comment #42)
> I don't really understand what (2) is about.

Basically, it means that if you follow either one of these tutorials...

https://developer.mozilla.org/en/XUL/Toolbars/Custom_toolbar_button#Making_a_button
https://developer.mozilla.org/en/XUL/Toolbars/Creating_toolbar_buttons#Styling_the_button

...you get an extension that works as expected on FF3, FF4, SM and so on,
without having to worry about testing on all those apps and all platforms.
(In reply to comment #41)
> Furthermore, Paolo could start
> poking at 2) as a separate patch, and if that bears fruit, I can help drive
> that into the tree on the other platforms as well.

Thank you very much Dan! I can start looking at this during this week-end.
I think I'll work on a separate bug, to keep this bug clean, like a meta-bug.
(In reply to comment #41)
> Thoughts?

Fantastic. Thank you, I think you covered things perfectly.
Sweet! Here's a github repository Dan and I are using to hack together an addon that tests out our assumptions:

  https://github.com/toolness/bug-616472-extension
Paolo: great, a separate bug sounds like a fine plan to me.
Here is my take on it... Example extension incl. Goals and TODO.
https://github.com/nmaier/compatible-icons
Would require three source-level/build-time changes to make (most of) the pain go away ;)
(In reply to comment #41)
> 1a) shorlander has volunteered to generate 16x16 icons for windows to replace
> the 18x18 ones.  I just spoke to him today, and he's already working on this.

As mentioned before, we don't need this.
> 1) Clean up all the small icon sizes to be 16x16 with the CSS set up so that
> the same icon can be used on all 3 platforms for small, as well as on Windows
> and Mac for large.  This means that the bare minimum amount of icons that an
> add-on author has to supply is one 16x16 icon and one 24x24 icon (which is used
> only for Linux large).

Sounds great.

> 1a) shorlander has volunteered to generate 16x16 icons for windows to replace
> the 18x18 ones.

Our own buttons don't need to be subject to the same size restrictions as extension toolbarbuttons. We have a list of our own buttons and override width and height with "auto" on them. See http://hg.mozilla.org/mozilla-central/annotate/3d4620449437/browser/themes/winstripe/browser/browser.css#l675

> Atul and I have all three platforms at hand, and we're willing to sign up to
> drive the work for 1) on all three platforms.

Thank you so much!
Blocks: 626382
As part of wrapping my head around more of the details here, I took Nils' add-on (thanks, Nils!) and reviewed in detail the current state of the world on Mac.  The results are <https://spreadsheets.google.com/ccc?key=0AisS38JGO7HHdF9ranJPbkhMbnM1dVBSRVdOZ0FuV0E&hl=en&authkey=CL35wMQL>.  My impression is that the results are as follows:

* as Nils has observed, the wrong color/size icons are being chosen because I'm testing without the in-progress fix for bug 626382, as filed by Paolo.  Nils, Paolo, does this piece sound right?

* because the existing CSS forces things to the right sizes, and because of mstange's already having fixed bug 618096, the Mac CSS appears to be in good shape and may not need any changes at all.
(In reply to comment #51)
> * as Nils has observed, the wrong color/size icons are being chosen because I'm
> testing without the in-progress fix for bug 626382, as filed by Paolo.  Nils,
> Paolo, does this piece sound right?

Yes, exactly. Note that if you can try a build with the patch from bug 626382,
to obtain the correct icon sizing you should also adjust the stylesheet in the
extension's "overlay.css" file, due to the slightly different attribute names.
Attached patch patch, v1 (obsolete) — — Splinter Review 
This patch is a first cut at making windows large mode icons a bit more self consistent.  In particular, it forces icon-sizes to 16x16, based on the possibly incorrect assumption that the rules I've touched won't interfere with our primary toolbars because they're looking out for themselves.

In particular, it only fixes the cells colored red in that spreadsheet.

A few things came up in doing this.  The first is that margin around the icon images on Mac is always 2, and on Windows (with this patch) is always 1.  This seems like it might want rationalizing, but then again maybe we're starting to get into territory where things just want to be different on different platforms.

More generally, it's not clear to me why there's so much variance across all the various extension toolbar button margins, padding, and intrinsic sizing (particularly, but not solely, on Windows).  Are there any important pieces of context that would be helpful to know in making decisions around this stuff?

Given that we're at a point in the cycle where it's important to minimize changes as strictly as possible, I'm trying to figure out just how much of these button parameters it's truly necessary to touch at this point.

On particular thing that jumps out at me is that, unlike on Mac, winstripe has a significant number of rules that make changes based on iconsize="small".  I'm not quite sure what to make of that.

Thoughts and historical perspective welcomed!
Attachment #504605 - Flags: feedback?
Attached patch patch, v2 (obsolete) — — Splinter Review 
Here's a cleaned up version of the Windows patch that I attached yesterday.  I decided not to add max-{width,height}, because of points made in bug 626382 comment 2.

Markus, I'm having a hard time figuring out whether your patch in bug 618096 already takes into account whether the fact that we're shrinking default button sizes means that we should be adding in explicit padding to offset that (as suggested in the comment responses in Jorge's blog post).  I was sort of looking for some new "padding: 2px" in that patch that I didn't see, but I did notice a "margin: 2px", which might be addressing the same thing.  Can you elaborate?

To be clear about what this patch that I've attached is, it's what I hope is the minimal necessary patch to make add-on devs lives easier as far as icon duplicity / sizing goes on Windows.  

In the spirit of not introducing unnecessary risk late in the game, there are a whole bunch of things that I did NOT try to rationalize here.  To wit:

* intrinsic size differences of the toolbarbutton elements
* image margin being 1 on Windows & 2 on Mac
* button margins being different across Mac and Windows
* button padding being different across Mac and Windows (other than to compensate for default icon size changes, which I'm already doing)

The main reason that I'm inclined to leave those things alone is that in testing with Nils' extension, the icons appear Good Enough to my untrained eye.

I would find it very very helpful to understand from add-on developers (or anyone else) there's anything on that list that is truly important to try and fix now, and, if not, why not.

Somewhat separately from all this, a Linux is patch is afoot...
Attachment #504605 - Attachment is obsolete: true
Attachment #504932 - Flags: feedback?
Attachment #504605 - Flags: feedback?
Er, "if not, why not" really should have read "if so, why".
Attachment #504932 - Attachment filename: patch-v2 → patch-v2.diff
Attachment #504932 - Attachment is patch: true
Attachment #504932 - Attachment mime type: application/octet-stream → text/plain
Attachment #504932 - Flags: feedback? → feedback?(dao)
Attached patch windows patch, v3 (obsolete) — — Splinter Review 
Non-corrupted patch.
Attachment #504932 - Attachment is obsolete: true
Attachment #504933 - Flags: feedback?(dao)
Attachment #504932 - Flags: feedback?(dao)
(In reply to comment #53)
> On particular thing that jumps out at me is that, unlike on Mac, winstripe has
> a significant number of rules that make changes based on iconsize="small".  I'm
> not quite sure what to make of that.

I think these rules are from bug 547752, to make the buttons a little more compact in small icons mode.

(In reply to comment #54)
> Markus, I'm having a hard time figuring out whether your patch in bug 618096
> already takes into account whether the fact that we're shrinking default button
> sizes means that we should be adding in explicit padding to offset that (as
> suggested in the comment responses in Jorge's blog post).  I was sort of
> looking for some new "padding: 2px" in that patch that I didn't see, but I did
> notice a "margin: 2px", which might be addressing the same thing.

Right, it does address the same thing. If you want an icon to be visually 16x16 but occupy space as if it was 20x20, you can either use "width:16px; height:16px; margin:2px;" or "width:20px; height:20px; padding:2px;". Which one is used shouldn't make any difference for anybody.

> * intrinsic size differences of the toolbarbutton elements
> * image margin being 1 on Windows & 2 on Mac
> * button margins being different across Mac and Windows
> * button padding being different across Mac and Windows (other than to
> compensate for default icon size changes, which I'm already doing)

I don't think any of these points need addressing. Different button appearances don't really hurt anybody as long as the icon sizes are the same everywhere.
(In reply to comment #57)
> > * intrinsic size differences of the toolbarbutton elements
> > * image margin being 1 on Windows & 2 on Mac
> > * button margins being different across Mac and Windows
> > * button padding being different across Mac and Windows (other than to
> > compensate for default icon size changes, which I'm already doing)
> 
> I don't think any of these points need addressing. Different button appearances
> don't really hurt anybody as long as the icon sizes are the same everywhere.

Quite right.
Comment on attachment 504933 [details] [diff] [review]
windows patch, v3

(In reply to comment #53)
> Thoughts and historical perspective welcomed!

I can't provide a historical perspective, but I've tried your latest patch
on Windows and here are my thoughts. I hope they'll be helpful. Thank you
very much for the patch and for your complete analysis on the spreadsheet!

>+++ b/browser/themes/winstripe/browser/browser.css
> .toolbarbutton-1 > .toolbarbutton-menubutton-button,
> .toolbarbutton-1 > .toolbarbutton-menubutton-dropmarker,
> .toolbarbutton-1 {
>   -moz-appearance: none;
>-  padding: 1px 5px;
>+  padding: 1px 3px;

> .toolbarbutton-1 {
>-  margin: 1px 3px;
>+  margin: 1px 2px;
> }

I suppose the disparity in button margin and padding between small and large icons is useful to make the user perceive a difference between the two
modes, otherwise the difference would be only in the toolbar height and the back/forward button, that seems strange to me. The mentioned bug 547752 further makes me think this difference is intentional. In fact, I don't think there's a need to change these rules now.

> .toolbarbutton-1 > .toolbarbutton-menubutton-button > .toolbarbutton-icon,
> .toolbarbutton-1 > .toolbarbutton-icon {
>-  -moz-margin-end: 0;
>-  width: 18px;
>-  height: 18px;
>+  margin: 1px;
>+  width: 16px;
>+  height: 16px;
> }

This is the important change for add-on authors. This rules applies only to them, since default buttons are dealt with separately.

> toolbar[iconsize="small"] .toolbarbutton-1 > .toolbarbutton-menubutton-button > .toolbarbutton-icon,
> toolbar[iconsize="small"] .toolbarbutton-1 > .toolbarbutton-icon {
>   margin: 1px;
>   width: 16px;
>   height: 16px;
> }

This rule for small icon mode becomes identical to the one above and can be removed safely.
(In reply to comment #59)
> 
> I can't provide a historical perspective, but I've tried your latest patch
> on Windows and here are my thoughts. I hope they'll be helpful. Thank you
> very much for the patch and for your complete analysis on the spreadsheet!

You're welcome, and thank you for taking the time to help out with feedback.

> >+++ b/browser/themes/winstripe/browser/browser.css
> > .toolbarbutton-1 > .toolbarbutton-menubutton-button,
> > .toolbarbutton-1 > .toolbarbutton-menubutton-dropmarker,
> > .toolbarbutton-1 {
> >   -moz-appearance: none;
> >-  padding: 1px 5px;
> >+  padding: 1px 3px;
> 
> > .toolbarbutton-1 {
> >-  margin: 1px 3px;
> >+  margin: 1px 2px;
> > }
> 
> I suppose the disparity in button margin and padding between small and large
> icons is useful to make the user perceive a difference between the two
> modes, otherwise the difference would be only in the toolbar height and the
> back/forward button, that seems strange to me. The mentioned bug 547752 further
> makes me think this difference is intentional. In fact, I don't think there's a
> need to change these rules now.

OK, I can buy that.

> > toolbar[iconsize="small"] .toolbarbutton-1 > .toolbarbutton-menubutton-button > .toolbarbutton-icon,
> > toolbar[iconsize="small"] .toolbarbutton-1 > .toolbarbutton-icon {
> >   margin: 1px;
> >   width: 16px;
> >   height: 16px;
> > }
> 
> This rule for small icon mode becomes identical to the one above and can be
> removed safely.

Good catch; I'll remove this as well, and maybe save a bit of perf too.

(In reply to comment #57)
>
> Right, it does address the same thing. If you want an icon to be visually 16x16
> but occupy space as if it was 20x20, you can either use "width:16px;
> height:16px; margin:2px;" or "width:20px; height:20px; padding:2px;". Which one
> is used shouldn't make any difference for anybody.

Ah, indeed.  And better, expression this using margins strikes me as more intuitive to read.

New patch iteration forthcoming.
Attached patch windows patch, v4 (obsolete) — — Splinter Review 

        Attachment #504933 -
        Attachment is obsolete: true

        Attachment #505336 -
        Flags: review?(dao)

        Attachment #504933 -
        Flags: feedback?(dao)

    
    

    
  
Comment on attachment 505336 [details] [diff] [review]
windows patch, v4

> .toolbarbutton-1 > .toolbarbutton-menubutton-button > .toolbarbutton-icon,
> .toolbarbutton-1 > .toolbarbutton-icon {
>-  -moz-margin-end: 0;
>-  width: 18px;
>-  height: 18px;
>-}
>-
>-toolbar[iconsize="small"] .toolbarbutton-1 > .toolbarbutton-menubutton-button > .toolbarbutton-icon,
>-toolbar[iconsize="small"] .toolbarbutton-1 > .toolbarbutton-icon {
>   margin: 1px;
>   width: 16px;
>   height: 16px;
> }

Bug 616156 has landed, so how about:

.toolbarbutton-1 > .toolbarbutton-icon {
  -moz-margin-end: 0;
}

#nav-bar .toolbarbutton-1 > .toolbarbutton-menubutton-button > .toolbarbutton-icon,
#nav-bar .toolbarbutton-1 > .toolbarbutton-icon {
  margin: 1px;
  width: 16px;
  height: 16px;
}

    
    

    
  


  
Hey guys, I was looking at the Linux situation and as far as I can tell, Linux needs 16x16 edge-to-edge icons for "use small icons" mode and 24x24 edge-to-edge icons for the other mode. Jorge mentions in his blog post that his Linux large icon is 24x24 but that the actual content in it is only 20x20, implying that addon authors may need to add 2px of built-in padding to their Linux icons. However, according to the screenshot I'm attaching here, this isn't the case--both Nils' icon and Firefox's "home" icon seem to be 24x24 edge-to-edge.

So my understanding is that to support Linux, addon authors need one 16x16 icon (which they can potentially just reuse from Windows or Linux at this point) and one 24x24 icon (which needs to be specially created just for Linux, and which I can't think of any way of avoiding).

Does that seem reasonable, or am I misunderstanding something?

    
    

    
  
(In reply to comment #63)

Depending on the Gtk icon theme, I think the icons could have different sizes or built-in padding, but add-ons don't need to care about this. Most of the time it's going to be 16x16 and 24x24.

    
    

    
  
(In reply to comment #62)

> Bug 616156 has landed, 

Indeed, this does make things more exciting!
 
> so how about: 
>
> .toolbarbutton-1 > .toolbarbutton-icon {
>   -moz-margin-end: 0;
> }

Out of curiousity, why not also have a .toolbarbutton-1 > .toolbarbutton-menubutton-button > .toolbarbutton selector for the above clause?

    
    

    
  

      
      
      
      

        Attached patch
          
          
        windows patch, v5 (obsolete)
        — 
          — Splinter Review
      

    


  
Here's a copy of the patch as per your original proposal.  Chromebug seems to verify that it's behaving reasonably.

My current understanding of the world is that we don't appear to need any changes for the Linux theme.  So that once this Windows patch is reviewed and landed, this bug can be declared Good Enough for Fx4 and resolved, and the chrome.manifest / stylesheet work can continue forward over in bug 626382.

        Attachment #505336 -
        Attachment is obsolete: true

        Attachment #505526 -
        Flags: review?(dao)

        Attachment #505336 -
        Flags: review?(dao)

    
    

    
  
Comment on attachment 505526 [details] [diff] [review]
windows patch, v5

Removing review request; further probing with chromebug has revealed that we've lost the margin of one on some of the toolbars with this patch.  Investigating.

        Attachment #505526 -
        Flags: review?(dao)

    
    

    
  
(In reply to comment #65)
> Out of curiousity, why not also have a .toolbarbutton-1 >
> .toolbarbutton-menubutton-button > .toolbarbutton selector for the above
> clause?

I missed that one.

(In reply to comment #67)
> lost the margin of one on some of the toolbars with this patch.  Investigating.

Sounds alright, we only want that margin in the nav bar.

    
    

    
  
> (In reply to comment #67)
> > lost the margin of one on some of the toolbars with this patch.  Investigating.
> 
> Sounds alright, we only want that margin in the nav bar.

Recently (both before and after the landing of bug 616156), we've had that margin of 1 around extension toolbar icons everywhere on Windows except the navbar in large mode.  My assumption was that having that held invariant at 1 helped add-on authors have a more constant environment for their icons to live.  Is there something I'm missing here?

    
    

    
  
Setting the margin only on the nav bar completes bug 616156. On other toolbars it's not going to provide any value.

    
    

    
  


  
Fair enough.  OK, here's an updated version of the patch for review with the extra selector that was omitted last time around.

        Attachment #505526 -
        Attachment is obsolete: true

        Attachment #505565 -
        Flags: review?(dao)

    
  

        Attachment #505565 -
        Flags: review?(dao) → review+

    
    

    
  
With the latest patch, are the sizes of add-on toolbar buttons still
constrained if the add-on fails to provide an icon of the correct size,
or the buttons become as big as the add-on-provided image? Do we care?

[I can't test this at present because I've a full rebuild still in progress...]

    
    

    
  
With the latest Windows patch, both Windows and Mac should be constrained, and Linux should not.  Which, as I understand it, is the same state of affairs as 3.x, which makes it a reasonable way to ship, I think.

    
    

    
  
Pushed to mozilla-central: 

changeset:   61069:8d52e3b68ca6

As noted in comment in 66:

My current understanding of the world is that we don't appear to need any
changes for the Linux theme.  So that once this Windows patch is reviewed and
landed, this bug can be declared Good Enough for Fx4 and resolved, and the
chrome.manifest / stylesheet work can continue forward over in bug 626382.

As such, marking RESOLVED FIXED.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED

    
    

    
  
(In reply to comment #15)
> (In reply to comment #14)
> > Is the large/small icon thing a common practice on Linux?
> > 
> > Given that small versus large doesn't really mean anything on Mac/Windows
> > anymore (just changing from keyhole to non keyhole), why are we keeping it
> > around?
> 
> The plan is to get rid of it. Didn't make it for 4.0, though — more important
> things to fix. It's on our list. :)

Small versus large does really mean something for third-party themes!

    
    

    
  
CCing self.

Just read the above discussion.

I totally disagree about doing away with 24x24.

Some of us really value the 24x24 icons, and I don't think it's unreasonable to ask extensions to come up with 2 different icon sizes.  We could even say that if the extension only provides a 16x16 icon, we dynamically add padding to make it up to 24x24.

I speak as an extension writer, who uses his own extension buttons.  I've developer 16x16 and 24x24, and I use 24x24 because I have a high resolution and 16x16 is painfully small for me and hard to click on.  I urge you guys to reconsider this decision to have only one icon size.

You're keeping the option of 24x24 for Linux.  There's a huge set of extensions that already have 16x16 and 24x24 icons for them.  Please can we NOT do away with the small/large icon size change option, and in fact unify this between the platforms?  So, all 3 platforms should have both small and large icon options.

Really, what are the arguments against keeping this useful customizability?  The only one I can see is, "it makes life a bit harder for extension developers", to which I would offer 2 responses.  1) all existing FF3 extensions *already* have a 24x24 icon, so they're not being asked to make a new one.  2) we can offer the option of providing no 24x24 icon, and automatically padding the 16x16 icon to 24x24 in large icon mode.  Frankly, I'm not even that good at graphics and I had no problem creating 2 icon sizes, so I think you're overstating this problem anyway.

    
    

    
  
I think the meaning of the previous discussion (and of bug 584477) is to get
rid of requiring add-ons to provide "large icons" that are 18x18 pixels. This
is not necessary anymore, since add-on icons on Windows are now 16x16 in most
places, and 24x24 in the customization palette and custom toolbars.

I also think that it's not unreasonable to allow custom themes to host add-on
icons that are 24x24 everywhere if they want, regardless of the platform. While
the latest Firefox 4 beta doesn't support this on Windows and Mac, we're fixing
this right now in bug 626382.

Instead, getting rid of the checkbox to select small/large icons and replacing
it with the installation of a custom or alternate theme, with the desired icon
and interface elements size, is something I think possible.

    
    

    
  
Great.  So you have to install a whole new theme to get bigger icons.  Still sucks.  And, that bug you linked to isn't even a hardblocker.

    
    

    
  
(In reply to comment #77)
> I think the meaning of the previous discussion (and of bug 584477) is to get
> rid of requiring add-ons to provide "large icons" that are 18x18 pixels. This
> is not necessary anymore, since add-on icons on Windows are now 16x16 in most
> places, and 24x24 in the customization palette and custom toolbars.
> 
> I also think that it's not unreasonable to allow custom themes to host add-on
> icons that are 24x24 everywhere if they want, regardless of the platform. While
> the latest Firefox 4 beta doesn't support this on Windows and Mac, we're fixing
> this right now in bug 626382.
> 
> Instead, getting rid of the checkbox to select small/large icons and replacing
> it with the installation of a custom or alternate theme, with the desired icon
> and interface elements size, is something I think possible.
And what about themes that offers large AND small icons? Removing that checkbox will break functionality on them (and most of them actually offers two icon sizes).

    
    

    
  
(In reply to comment #79)
> (In reply to comment #77)
> > Instead, getting rid of the checkbox to select small/large icons and replacing
> > it with the installation of a custom or alternate theme, with the desired icon
> > and interface elements size, is something I think possible.
> And what about themes that offers large AND small icons? Removing that checkbox
> will break functionality on them (and most of them actually offers two icon
> sizes).

Right, themes would need to be updated and use one icon size only. It can be
argued that this is a loss of functionality. In any case this is not going to
happen now - and maybe not even after Firefox 4, I don't know.

    
    

    
  
Ooh, what a surprise.  No-can-do from the Firefox UX team.  There's a novelty.

You guys are as useful as a bucket of sand in the Sahara desert.

    
    

    
  
(In reply to comment #81)
> Ooh, what a surprise.  No-can-do from the Firefox UX team.  There's a novelty.
> 
> You guys are as useful as a bucket of sand in the Sahara desert.

Apologies for my comments.  I posted this in the heat of the moment and should've been more civil with what I said.
Depends on: 631020

    
    

    
  
Thanks a lot, guys, for fixing this!
This bug started out exactly right, and ended up right. This removed a huge problem of FF4 for ext devs. Thanks!

    
    

    
  
:-)

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: