896008 - Add-on buttons on the navigation toolbar are randomly sized due to iconsize="large"

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Erik Vold [:erikvold] (please needinfo? me)]

Attachment: Nav bar on Australis

See the attachments, on the trunk buttons look like buttons! on Australis they look like clutter and a waste of space.

Comment 1

[Erik Vold [:erikvold] (please needinfo? me)]

Attachment: Beautiful buttons from the trunk

Comment 2

[Erik Vold [:erikvold] (please needinfo? me)]

Notice how far away the menu-button menu arrow is from the icons, that makes it hard to tell what they are for, and so they look like messy clutter.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

I believe this as per spec...

http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis-designSpecs-osx-mainWindow.html

Comment 4

[Erik Vold [:erikvold] (please needinfo? me)]

Notice the varying spacing between buttons on Australis and the uniform spacing on Trunk, this random spacing makes things look like clutter again.

Comment 5

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Mike Conley (:mconley) from comment #3)
> I believe this as per spec...
> 
> http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis-
> designSpecs-osx-mainWindow.html

These specs look nothing like the reality imo, see the screen shot.

Comment 6

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Mike Conley (:mconley) from comment #3)
> I believe this as per spec...
> 
> http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis-
> designSpecs-osx-mainWindow.html

In this spec there is a line between a menu-button's icon and the menu arrow, which I do not see in my screen shot.

Comment 7

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #6)
> (In reply to Mike Conley (:mconley) from comment #3)
> > I believe this as per spec...
> > 
> > http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis-
> > designSpecs-osx-mainWindow.html
> 
> In this spec there is a line between a menu-button's icon and the menu
> arrow, which I do not see in my screen shot.

Tho I think this may make the two images appear separated and not together without more spacing between buttons which I don't think is a good idea.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #5)
> (In reply to Mike Conley (:mconley) from comment #3)
> > I believe this as per spec...
> > 
> > http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis-
> > designSpecs-osx-mainWindow.html
> 
> These specs look nothing like the reality imo, see the screen shot.

I was referring to the lack of chrome / outline around the buttons. I agree that the spacing is pretty rough looking.

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #6)
> (In reply to Mike Conley (:mconley) from comment #3)
> > I believe this as per spec...
> > 
> > http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis-
> > designSpecs-osx-mainWindow.html
> 
> In this spec there is a line between a menu-button's icon and the menu
> arrow, which I do not see in my screen shot.

For the bookmark menu button, sure. Not so for the feed button.

Comment 9

[Erik Vold [:erikvold] (please needinfo? me)]

I'd really like to know why buttons cannot look like buttons too as they always have before.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Sure, I'll needinfo? the designer.

Comment 11

[Brandon Cheng]

This is per spec, but it does look pretty bad. The varying button sizes isn't great to look at either.

Comment 12

[:Gijs (he/him)]

I'm not a designer (nor 'the' designer for the OS X styles) but I'm going to be blunt here: can we clean up the summary and make it, uh, summarize what this bug is about? As it is, I read all of the comments here several times and don't understand if this is supposed to be:

1) about the spacing between individual buttons
2) about the spacing between menu button icons and their dropmarker
3) about the differently-sized toolbarbuttons
4) about the lacking button borders
5) some combination of the above

Please pick (and/or add to this list) and clarify.

Comment 13

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to :Gijs Kruitbosch from comment #12)
> I'm not a designer (nor 'the' designer for the OS X styles) but I'm going to
> be blunt here: can we clean up the summary and make it, uh, summarize what
> this bug is about? As it is, I read all of the comments here several times
> and don't understand if this is supposed to be:
> 
> 1) about the spacing between individual buttons
> 2) about the spacing between menu button icons and their dropmarker
> 3) about the differently-sized toolbarbuttons
> 4) about the lacking button borders
> 5) some combination of the above
> 
> Please pick (and/or add to this list) and clarify.

It's all of the above, if you want to make sub bugs that depend on this then knock yourself out.

Comment 14

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #13)
> (In reply to :Gijs Kruitbosch from comment #12)
> > I'm not a designer (nor 'the' designer for the OS X styles) but I'm going to
> > be blunt here: can we clean up the summary and make it, uh, summarize what
> > this bug is about? As it is, I read all of the comments here several times
> > and don't understand if this is supposed to be:
> > 
> > 1) about the spacing between individual buttons
> > 2) about the spacing between menu button icons and their dropmarker
> > 3) about the differently-sized toolbarbuttons
> > 4) about the lacking button borders
> > 5) some combination of the above
> > 
> > Please pick (and/or add to this list) and clarify.
> 
> It's all of the above, if you want to make sub bugs that depend on this then
> knock yourself out.

Frankly I'd just like to see the style reverted.

Comment 15

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #13)
> 
> It's all of the above, if you want to make sub bugs that depend on this then
> knock yourself out.

I meant sub bugs that block this bug.

Comment 16

[Dão Gottwald [:dao]]

The varying icon sizes are a regression from bug 863299.

Comment 17

[Erik Vold [:erikvold] (please needinfo? me)]

sigh..

Comment 18

[Justin Dolske [:Dolske]]

(In reply to Dão Gottwald [:dao] from comment #16)
> The varying icon sizes are a regression from bug 863299.

Do we know why it's a regression?

Comment 19

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

This isn't a regression from bug 863299. We don't have any plans to place the button shape back on the buttons in the near future.

When the buttons look bad, the add-on authors should fix their icons so as not to look crummy. Particularly, http://screencast.com/t/UwErRGeX1DNd looked pretty bad already on m-c.

Comment 20

[Dão Gottwald [:dao]]

(In reply to Justin Dolske [:Dolske] from comment #18)
> (In reply to Dão Gottwald [:dao] from comment #16)
> > The varying icon sizes are a regression from bug 863299.
> 
> Do we know why it's a regression?

Because that patch made us stop setting iconsize="small" on the navigation toolbar, so now add-ons use random icon sizes.

Comment 21

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #20)
> (In reply to Justin Dolske [:Dolske] from comment #18)
> > (In reply to Dão Gottwald [:dao] from comment #16)
> > > The varying icon sizes are a regression from bug 863299.
> > 
> > Do we know why it's a regression?
> 
> Because that patch made us stop setting iconsize="small" on the navigation
> toolbar, so now add-ons use random icon sizes.

I'm confused. We still set iconsize="large" on the navigation toolbar, which is correct. I don't see what we can do about add-ons then not selecting the right icon size.

Comment 22

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #21)
> (In reply to Dão Gottwald [:dao] from comment #20)
> > (In reply to Justin Dolske [:Dolske] from comment #18)
> > > (In reply to Dão Gottwald [:dao] from comment #16)
> > > > The varying icon sizes are a regression from bug 863299.
> > > 
> > > Do we know why it's a regression?
> > 
> > Because that patch made us stop setting iconsize="small" on the navigation
> > toolbar, so now add-ons use random icon sizes.
> 
> I'm confused. We still set iconsize="large" on the navigation toolbar, which
> is correct.

Why "still"? We used to set iconsize="small" on Windows and OS X. And no, iconsize="large" is not correct. Please take a close look at the two screenshots attached to this bug. The small icons are in line with our own icons, the large ones are not.

Comment 23

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #22)
> (In reply to :Gijs Kruitbosch from comment #21)
> > (In reply to Dão Gottwald [:dao] from comment #20)
> > > Because that patch made us stop setting iconsize="small" on the navigation
> > > toolbar, so now add-ons use random icon sizes.
> > 
> > I'm confused. We still set iconsize="large" on the navigation toolbar, which
> > is correct.
> 
> Why "still"?

Because I interpreted your comment, plus the note that add-ons now use 'random' sizes, to mean we were not setting anything.

> We used to set iconsize="small" on Windows and OS X. And no,
> iconsize="large" is not correct. Please take a close look at the two
> screenshots attached to this bug. The small icons are in line with our own
> icons, the large ones are not.

I'm not getting it, I guess.

On latest OS X m-c Nightly, I created a new profile. This is what I see:
- navigator-toolbox has iconsize="large"
- nav-bar has iconsize="small" (as you noted)
- all icons are small, except the back button

Opening customize mode and ticking 'Use small icons' changes the value of iconsize on the navigator-toolbox to "small". It does not change the value on the nav-bar. The back button now goes small.

Using the browser console to change the value on the nav-bar did not affect any of the icon sizes (including the back-button, which remained 'large' or 'small', depending on the toolbox value, whatever I did to the nav-bar).

Using MXR, I don't see any way for a toolbar's value to be changed, but I may be missing something.

All of this means I don't really understand why we are supposed to set iconsize="small" on the nav-bar. In fact, to me it would make the most sense to remove all of these as they are never changed anymore. Is backwards-compatibility with add-ons the only reason we can't do that?

Comment 24

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

What is the state of this bug? As far as I see it, there is nothing wrong here, just add-ons that aren't providing good iconography.

Comment 25

[Stephen Horlander [:shorlander]]

(In reply to Jared Wein [:jaws] from comment #24)
> What is the state of this bug? As far as I see it, there is nothing wrong
> here, just add-ons that aren't providing good iconography.

I agree that there appears to be an add-on problem, but it seems like we are doing something wrong with regards to spacing and button size.

Comment 26

[Dão Gottwald [:dao]]

We should set iconsize="small" on #nav-bar, like we did before, such that add-ons use small icons, like we want them to.

(In reply to :Gijs Kruitbosch from comment #23)
> All of this means I don't really understand why we are supposed to set
> iconsize="small" on the nav-bar. In fact, to me it would make the most sense
> to remove all of these as they are never changed anymore. Is
> backwards-compatibility with add-ons the only reason we can't do that?

The iconsize stuff is still in toolkit and add-ons often use the same CSS to support multiple applications, different Firefox versions, etc. Even if this weren't the case, we actually don't want to break add-ons and can easily keep them working here, such that they can be marked compatible by AMO without any author interaction.

Comment 27

[:Gijs (he/him)]

I think dao understands this one better than I do. I'm updating the whiteboard to do what I think you meant to do, please correct me if necessary. :-)

Comment 28

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

(In reply to :Gijs Kruitbosch from comment #27)
> I'm updating the
> whiteboard to do what I think you meant to do, please correct me if
> necessary. :-)

Thanks for catching my typo!

Comment 29

[Dão Gottwald [:dao]]

Raising priority as this affects a sizeable number of add-ons and all platforms.

Comment 30

[Dão Gottwald [:dao]]

Attachment: patch

Comment 31

[Dão Gottwald [:dao]]

Attachment: before & after screenshots

Comment 32

[:Gijs (he/him)]

Comment on attachment 815347 [details] [diff] [review]
patch

r=me

Comment 33

[Dão Gottwald [:dao]]

http://hg.mozilla.org/projects/ux/rev/70aba2a2ede7

Comment 34

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/70aba2a2ede7