See attached screenshot. A quick inspection shows an attribute on the toolbar-button: "badged-button". Removing that attribute fixes the issue.

This only showed up a few days ago, so I think I can knock out a bisection over lunch.

Looks like: https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1147702&attachment=8588077 diff --git a/browser/base/content/browser.css b/browser/base/content/browser.css --- a/browser/base/content/browser.css +++ b/browser/base/content/browser.css @@ -882,24 +882,16 @@ html|*#gcli-output-frame, .browserStack[responsivemode][notransition] { transition: none; } toolbarbutton[type="socialmark"] { -moz-binding: url("chrome://browser/content/socialmarks.xml#toolbarbutton-marks"); } -toolbarbutton.badged-button > .toolbarbutton-badge-container > .toolbarbutton-icon, -toolbarbutton[type="socialmark"] > .toolbarbutton-icon { - max-width: 18px; -} -toolbarpaletteitem[place="palette"] > toolbarbutton.badged-button > .toolbarbutton-badge-container > .toolbarbutton-icon { - max-width: 32px; -} - to me, but I don't have a (hidpi) windows machine to hand to test with.

Yeah, restoring that set of rules fixes this issue. Dão, was there a particular reason you removed it? Can we just put them back in or will that break other things (and if so, what) ?

Bisecting confirms that this is a regression of bug 1147702.

The removed code in browser/base/content/browser.css was a messy way to deal with the fact that .badged-button's binding imposes a different anonymous content structure such that http://hg.mozilla.org/mozilla-central/annotate/fec90cbfbaad/browser/themes/osx/browser.css#l1356 doesn't cover it anymore. We need to update the theme stylesheet's selector to fix that.

like this

Comment on attachment 8590946 [details] [diff] [review] set size for badged SDK buttons in the toolbar Review of attachment 8590946 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/linux/browser.css @@ -595,5 @@ > } > > -toolbarbutton.badged-button > .toolbarbutton-badge-container > .toolbarbutton-icon, > -toolbarbutton[type="socialmark"] > .toolbarbutton-icon { > - max-width: 32px !important; I don't understand why this was removed. Can you explain it to me?

Because it serves no purpose, as far as I can tell, and doesn't exist in browser/themes/osx/browser.css and browser/themes/windows/browser.css.

Shane - that rule that we're removing is one you added in bug 984630... are we good to tear this out?

Comment on attachment 8590946 [details] [diff] [review] set size for badged SDK buttons in the toolbar r=me conditional on mixedpuppy thumbs up.

(In reply to Dão Gottwald [:dao] from comment #9) > Because it serves no purpose, as far as I can tell, and doesn't exist in > browser/themes/osx/browser.css and browser/themes/windows/browser.css. I'm pretty sure this was made unnecessary by the rules in browser/base/content/browser.css that you removed. Now that that's gone this rule should be there on the other OSes as well: If I put the tree status add-on's button in the menupanel on OS X, its icon grows to 48x48px (the svg's defined width + height) instead of the expected 32/36px.

I applied the patch, still get large icons. It's easy to test, just go here and activate: https://socialapi-demo.paas.allizom.org/ signin/signout. You can see that when the button is badged, it is sized correctly, when it is not, it is giant. socialmark buttons (in demo appears as flag) are also now too large.

(In reply to Shane Caraveo (:mixedpuppy) from comment #13) > I applied the patch, still get large icons. It's easy to test, just go here > and activate: > > https://socialapi-demo.paas.allizom.org/ > > signin/signout. You can see that when the button is badged, it is sized > correctly, when it is not, it is giant. socialmark buttons (in demo appears > as flag) are also now too large. That button uses the badged-button class and expects us to set the size for its icon. This is bogus, because whether a button has a badge or not shouldn't make a difference for whether we set a size for it. Long ago we set the size for all toolbar button icons, but some add-on authors complained about it, so we reverted that. Then SDK people came along and requested that we set a size for SDK-provided button icons, so we did that. As it stands, your button needs to be an SDK button or you need to handle its size yourself. So we'll need a SocialAPI-specific fix here or we need to go back to setting a size for all toolbar button icons (I'd support that). Can you please file a bug on what you think is the preferred solution here? (In reply to :Gijs Kruitbosch from comment #12) > I'm pretty sure this was made unnecessary by the rules in > browser/base/content/browser.css that you removed. Now that that's gone this > rule should be there on the other OSes as well: If I put the tree status > add-on's button in the menupanel on OS X, its icon grows to 48x48px (the > svg's defined width + height) instead of the expected 32/36px. Not sure what the deal is with that. I installed the add-on but can't seem to get it to show any UI. Can you please file a new bug on this with some details about what kind of button that is?

(In reply to Dão Gottwald [:dao] from comment #15) > (In reply to :Gijs Kruitbosch from comment #12) > > I'm pretty sure this was made unnecessary by the rules in > > browser/base/content/browser.css that you removed. Now that that's gone this > > rule should be there on the other OSes as well: If I put the tree status > > add-on's button in the menupanel on OS X, its icon grows to 48x48px (the > > svg's defined width + height) instead of the expected 32/36px. > > Not sure what the deal is with that. I installed the add-on but can't seem > to get it to show any UI. Can you please file a new bug on this with some > details about what kind of button that is? I'm confused... https://addons.mozilla.org/en-US/firefox/addon/mozilla-tree-status/?src=cb-dl-created adds a button to the navbar. The patch you put up fixes the button's size in the navbar, but if you right click the button and move it to the menu panel, its size there is still not fixed. So essentially, what I'm saying is your patch fixes half the issue. I don't think we should split the other half off when it's essentially the same regression caused by the same block of rules being removed in the same bug, and can be fixed in the same way.

(fwiw, the addon in question's button is the green tree-like icon in the initial attachment 8590849 [details])

(In reply to :Gijs Kruitbosch from comment #16) > (In reply to Dão Gottwald [:dao] from comment #15) > > (In reply to :Gijs Kruitbosch from comment #12) > > > I'm pretty sure this was made unnecessary by the rules in > > > browser/base/content/browser.css that you removed. Now that that's gone this > > > rule should be there on the other OSes as well: If I put the tree status > > > add-on's button in the menupanel on OS X, its icon grows to 48x48px (the > > > svg's defined width + height) instead of the expected 32/36px. > > > > Not sure what the deal is with that. I installed the add-on but can't seem > > to get it to show any UI. Can you please file a new bug on this with some > > details about what kind of button that is? > > I'm confused... > https://addons.mozilla.org/en-US/firefox/addon/mozilla-tree-status/?src=cb- > dl-created adds a button to the navbar. It didn't do that for me. > The patch you put up fixes the > button's size in the navbar, but if you right click the button and move it > to the menu panel, its size there is still not fixed. So essentially, what > I'm saying is your patch fixes half the issue. I don't think we should split > the other half off when it's essentially the same regression caused by the > same block of rules being removed in the same bug, and can be fixed in the > same way. Is it a badged button? Is it an SDK button? Both? The removed code wrongly set the size for badged button icons. Do we need to set a size for SDK buttons in the panel? If so, I still think that's fodder for a separate bug.

(In reply to Dão Gottwald [:dao] from comment #18) > Do we need to set a size for SDK > buttons in the panel? If so, I still think that's fodder for a separate bug. Turns out we're doing this in panelUIOverlay.inc.css, but it suffers from the same flaw as the browser.css styles mentioned in comment 6. This is easy enough to fix here, I'll attach a patch.

https://hg.mozilla.org/integration/fx-team/rev/abf68360159d https://hg.mozilla.org/integration/fx-team/rev/8fa1ddc64165

https://hg.mozilla.org/mozilla-central/rev/abf68360159d https://hg.mozilla.org/mozilla-central/rev/8fa1ddc64165

This fix badly breaks the toolbar button sizes, etc of all new template Complete Themes, i.e. ones that overlay the default theme, rather than replacing it. This method has many advantages, like new features automagically appearing and working, but it is susceptible to bug fixes like this one and I would ask you to bear that in mind when coding up default. The theme author/extension writer's fix to the fix of this bug is to use unset, i.e. - #nav-bar toolb...yadda yadda ...marker-icon { /* horizontal padding + border + actual icon width */ width: unset !important; } ....a tip I hope saves some poor slob a few hours work. :)