1215100 - Assess whether we need more Firefox theme changes to accommodate OmniSidebar and its icons

Status: RESOLVED INVALID

(Firefox :: Theme, defect)

Description

[:Gijs (he/him)]

(In reply to Gary [:streetwolf] from bug 1208715 comment #23)
> I was able to fix the icons with this snippet of css code:
> 
> .toolbarbutton-1 > .toolbarbutton-icon,
>   .toolbarbutton-1 > :-moz-any(.toolbarbutton-menubutton-button,
> .toolbarbutton-badge-stack) > .toolbarbutton-icon { max-width: 48px
> !important; }

That snippet on its own is not going to work for Firefox, because it will regress bug 1208715 because it allows wider (and thereby higher) icons than normal.

What kind of buttons are these? Do they come with OmniSidebar, or are they shipped-by-default Firefox ones? Do they behave properly in other toolbars / the menupanel?

Comment 1

[Gary [:streetwolf52]]

I'm now using this code as it made other icons too big:

.toolbarbutton-1 > .toolbarbutton-icon { max-width: unset !important; }

The two buttons in my screenshot are created by OmniSidebar.  However, even the Fx supplied buttons appear the same way on the OmniSidebar toolbar.

I contacted Luis the developer but I believe he's not around at the moment.

Comment 2

[Gary [:streetwolf52]]

The only problem I've had with buttons are ones I place on Omnisidebar's toolbar.  They appear fine everywhere else even when the patch that changed them landed.

Comment 3

[Luís Miguel [:quicksaver] (not reading bugmail)]

OmniSidebar uses the nav-bar's styling for the sidebar header toolbar through a stylesheet of its own, adapted from the Firefox theme's browser.css. I'll have to change its |width| to a |max-width|, as was done in bug 1208715 for the nav-bar.

Thanks for the forward to this bug Gary! I added this to my to-do list so I don't forget, I'll be sure to close this bug when I do this, there's no need for anything to be done on Firefox's side.

Gijs, have you considered making all the nav-bar button styling code use a class selector instead of using the #nav-bar id selector directly? That way the nav-bar's style can be used in other toolbars directly (by add-ons obviously), instead of them having to keep an almost direct copy of those parts of the stylesheet; this would mean:
- less upkeep by add-on authors
- less work for AMO reviewers because of less updates
- same upkeep for you guys in Firefox code?

Comment 4

[:Gijs (he/him)]

(In reply to Luís Miguel [:quicksaver] from comment #3)
> Gijs, have you considered making all the nav-bar button styling code use a
> class selector instead of using the #nav-bar id selector directly? That way
> the nav-bar's style can be used in other toolbars directly (by add-ons
> obviously), instead of them having to keep an almost direct copy of those
> parts of the stylesheet; this would mean:
> - less upkeep by add-on authors
> - less work for AMO reviewers because of less updates
> - same upkeep for you guys in Firefox code?

I don't understand your suggestion. This would mean updating the class of every button whenever it moves to the navbar, right? How would add-ons opt-in? What would be the advantage?

Right now, add-on buttons that have .toolbarbutton-1 should get the right styling everywhere in the browser. One exception used to be that they'd need a 16x16 icon. The change from the other bugs means that we now normalize icon sizes. I don't really follow what the current problem is for add-ons, and why they would need an "almost direct copy" of our stylesheets.

We should sort out the bookmarks toolbar styling (bug 734326) but that hasn't been a priority so far.

Comment 5

[Luís Miguel [:quicksaver] (not reading bugmail)]

(In reply to :Gijs Kruitbosch from comment #4)
> I don't understand your suggestion. This would mean updating the class of
> every button whenever it moves to the navbar, right?

No, I meant add a class to the nav-bar like "headerToolbar" and throughout the CSS code replace |#nav-bar .toolbarbutton-1| with |.headerToolbar .toolbarbutton-1|. I'm sorry, I should have been more specific.

(BTW might also help with bug 734326, might not, I haven't checked the bookmarks buttons styling code to know for sure.)

Comment 6

[Dão Gottwald [:dao]]

(In reply to Luís Miguel [:quicksaver] from comment #3)
> Thanks for the forward to this bug Gary! I added this to my to-do list so I
> don't forget, I'll be sure to close this bug when I do this, there's no need
> for anything to be done on Firefox's side.

We don't track external add-on bugs like this in bugzilla.