1150627 - Use SVG graphics for the toolbar buttons in main window

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Richard Marti (:Paenglab)]

With using svg graphics instead of png should make them DPI independent. Also the size is smaller than png.

I'm trying to make a set of graphics which we use for all platforms (Windows Aero, OS X and Linux. But not for XP as I think we should leave this dead system as it is and only fix bugs).

Comment 1

[Richard Marti (:Paenglab)]

Attachment: useSVGonMainToolbars.patch

This patch is now only for Aero and OS X. Linux comes when the feedback is positive.

The positive of using a shared set is OS X (and later Linux) becomes inverted icons for dark LW themes.

I implemented the colors for every platform differently: Win7, Win8+, OS X up to 9, Yosemite and Linux (Windows non-default themes are using the same colors as Linux). And naturally the inverted theme.

A known issue is the print button icon isn't shown in customize window because the composer window isn't transformed to svg.

Josiah, what do you think to this approach?

Comment 2

[Blake Winton (:bwinton) (:☕️)]

For the record, there are twice as many people on Windows XP as on all versions of Mac and Linux combined.
Dead OS or not, they're still 19% of our users, and our second biggest platform…

Comment 3

[Blake Winton (:bwinton) (:☕️)]

(Wait, that was for Firefox.  But I strongly suspect Thunderbird's numbers are similar.)

Comment 4

[Richard Marti (:Paenglab)]

But they are resistant to or don't like changes (as we see they don't change to a living platform). So it's better to leave it as it is. XP has already a different graphics set because of this.

Comment 5

[Richard Marti (:Paenglab)]

Attachment: useSVGonMainToolbars.patch

This is now a full patch for main window, composer and address book for Linux, OS X and Windows Vista+.

For the graphics I used the colors FX is using for the different platforms.

Comment 6

[Richard Marti (:Paenglab)]

Attachment: useSVGonMainToolbars.patch

Found a wrong class name on two definitions.

Comment 7

[Richard Marti (:Paenglab)]

Attachment: useSVGonToolbars.patch

Found a typo. This should be now really the final patch.

Comment 8

[Josiah Bruner [:jsbruner]]

Comment on attachment 8588676 [details] [diff] [review]
useSVGonToolbars.patch

Review of attachment 8588676 [details] [diff] [review]:
-----------------------------------------------------------------

I've only looked at the OS X parts so far, but I decided I would release comments in waves.

In addition to the comments below, I have some "big picture" comments on the concept.

1. Definitely a little more cartoon-y. Maybe not a bad thing, but doesn't appear as consistent as Firefox.
2. The Send icon appears from a distance to have blurry edges, but it really doesn't. An illusion is created probably due to the narrow cutouts in the icon. Maybe extend them?
3. Really concerned about performance here. Although the PNGs are smaller overall, they take a LOT more effort to render, especially on low-spec machines. I know this makes a lot of things easier for us and theme developers, but this problem does concern me.
4. The icons seem larger than they use to be. Maybe they're just a little bolder, not sure. Regardless, I do think they look slightly less slick than they use to.

All that said, I'm very impressed with this patch, and the icons were done very well!

::: mail/themes/linux/mail/addrbook/addressbook.css
@@ +16,5 @@
>    background-repeat: no-repeat;
>    background-position: top right;
>  }
>  
> +#ab-toolbox > toolbar {

This file doesn't apply cleanly for some reason. In fact, quite severely. Those #addressbookWindow:-moz-lwtheme rules don't even exist on tip.

::: mail/themes/osx/mail/primaryToolbar.css
@@ +809,2 @@
>  #button-chat[unreadMessages="true"] {
>    -moz-image-region: rect(36px, 396px, 54px, 378px);

You need to remove this to get the unread svg icon to show.

::: mail/themes/osx/mail/quickFilterBar.css
@@ +10,1 @@
>    #qfb-show-filter-bar {

Hmm, I think we should move these toolbar button styles to primaryToolbar.css. It's kind of confusing that they're here.

Comment 9

[Josiah Bruner [:jsbruner]]

Oh, also, the forward button doesn't seem to actually disable itself with this patch. It appears disabled, but I can still select the button and obtain the dropdown.

Comment 10

[Richard Marti (:Paenglab)]

(In reply to Josiah Bruner [:JosiahOne] (needinfo! - Won't respond to CC)) from comment #8)
> Comment on attachment 8588676 [details] [diff] [review]
> useSVGonToolbars.patch
> 
> Review of attachment 8588676 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I've only looked at the OS X parts so far, but I decided I would release
> comments in waves.
> 
> In addition to the comments below, I have some "big picture" comments on the
> concept.
> 
> 1. Definitely a little more cartoon-y. Maybe not a bad thing, but doesn't
> appear as consistent as Firefox.

They are based on the icons we are using now and it should be no big difference in appearance. On Yosemite the difference is I use now only one color like FX instead of gradients. Maybe this makes it looking cartoonish. With OS X <10.10 you should still see the gradients.

> 2. The Send icon appears from a distance to have blurry edges, but it really
> doesn't. An illusion is created probably due to the narrow cutouts in the
> icon. Maybe extend them?

I can try it, but I need to look at LoDPI to make them not blurry using half pixels. On HiDPI this would be calculated to even pixels and then looking better.

> 3. Really concerned about performance here. Although the PNGs are smaller
> overall, they take a LOT more effort to render, especially on low-spec
> machines. I know this makes a lot of things easier for us and theme
> developers, but this problem does concern me.

Yes, Robert Longson pointed this out in bug 1153615 and I'm now on implementing his proposals.

> 4. The icons seem larger than they use to be. Maybe they're just a little
> bolder, not sure. Regardless, I do think they look slightly less slick than
> they use to.

This could be again because of the LoDPI needs.

> All that said, I'm very impressed with this patch, and the icons were done
> very well!
> 
> ::: mail/themes/linux/mail/addrbook/addressbook.css
> @@ +16,5 @@
> >    background-repeat: no-repeat;
> >    background-position: top right;
> >  }
> >  
> > +#ab-toolbox > toolbar {
> 
> This file doesn't apply cleanly for some reason. In fact, quite severely.
> Those #addressbookWindow:-moz-lwtheme rules don't even exist on tip.

This is probably because I haven't updated the patch after the check-in of bug 1145974.
I'll update the patch after I made the changes Robert Longson proposed.

Comment 11

[Richard Marti (:Paenglab)]

Attachment: useSVGonToolbars.patch v2

Now with the new approach Robert Longson proposed which should use less memory.

I changed also the icon for "Get Mail" to the Windows version. The one from OS X war too big.
I also changed the "Write Mail" icon to a simpler one (but not so simple as Windows uses). What do you think about it?

(In reply to Josiah Bruner [:JosiahOne] (needinfo! - Won't respond to CC)) from comment #8)
> Comment on attachment 8588676 [details] [diff] [review]
> useSVGonToolbars.patch
> 
> Review of attachment 8588676 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> 2. The Send icon appears from a distance to have blurry edges, but it really
> doesn't. An illusion is created probably due to the narrow cutouts in the
> icon. Maybe extend them?

I made the cutouts deeper. Is this better?

> 3. Really concerned about performance here. Although the PNGs are smaller
> overall, they take a LOT more effort to render, especially on low-spec
> machines. I know this makes a lot of things easier for us and theme
> developers, but this problem does concern me.

SVG rendering was optimized for the Australis tabs with caching etc. so I think this should be no problem. 

> ::: mail/themes/osx/mail/primaryToolbar.css
> @@ +809,2 @@
> >  #button-chat[unreadMessages="true"] {
> >    -moz-image-region: rect(36px, 396px, 54px, 378px);
> 
> You need to remove this to get the unread svg icon to show.

Done

> ::: mail/themes/osx/mail/quickFilterBar.css
> @@ +10,1 @@
> >    #qfb-show-filter-bar {
> 
> Hmm, I think we should move these toolbar button styles to
> primaryToolbar.css. It's kind of confusing that they're here.

Yes, this makes sense -> done.

Comment 12

[Richard Marti (:Paenglab)]

(In reply to Josiah Bruner [:JosiahOne] (needinfo! - Won't respond to CC)) from comment #9)
> Oh, also, the forward button doesn't seem to actually disable itself with
> this patch. It appears disabled, but I can still select the button and
> obtain the dropdown.

This wasn't from this patch, it happens also without. But I fixed it with this patch.

Comment 13

[Richard Marti (:Paenglab)]

Attachment: useSVGonToolbars.patch v3

This patch uses now smaller icons like the original ones from png.

Comment 14

[Richard Marti (:Paenglab)]

Attachment: useSVGonToolbars.patch v4

Unbitrot after bug 1153553 landing.

Comment 16

[Josiah Bruner [:jsbruner]]

Comment on attachment 8596147 [details] [diff] [review]
useSVGonToolbars.patch v4

Review of attachment 8596147 [details] [diff] [review]:
-----------------------------------------------------------------

Assuming you tested this thoroughly on Linux and Windows, looks pretty good to me. Only thing I noticed is that the AB icon doesn't switch to white in brighttext mode.

r+ and ui-r+ with that fixed.

Comment 17

[Richard Marti (:Paenglab)]

Attachment: useSVGonToolbars.patch

Fixed the inverted AB icon on OS X. It was a typo.

I've added also Win10 colors to the buttons. This can change as FX hasn't decided which color is used. I'm using now the original Microsoft Edge button colors (#616161).

Comment 18

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/ab0a4209f0b5 -> FIXED