1542631 - Sync folderpane feed account icon with Account Central icon

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[alta88]

This has bothered me for a long long time.

paenglab, I also think the former feed 'news' like icon should be used for newsgroups, which look way too much like chat.

Comment 1

[alta88]

Attachment: feedIcon.patch

Comment 2

[Richard Marti (:Paenglab)]

Comment on attachment 9056421 [details] [diff] [review]
feedIcon.patch

The patch is empty.

Comment 3

[alta88]

Attachment: feedIcon.patch

oops.

Comment 4

[Richard Marti (:Paenglab)]

Comment on attachment 9056422 [details] [diff] [review]
feedIcon.patch

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

Using the accountcentral icons needs scaling them to 16px. Please use the newsblog/rss-feed icons instead. And when you're on it, can you copy the Linux accountcentral/manage-rss.png to windows? Then the icons are consistent.

::: mail/themes/linux/mail/folderPane.css
@@ +191,5 @@
>  
>  .tabmail-tab[type="folder"][IsServer="true"][ServerType="rss"],
>  treechildren::-moz-tree-image(folderNameCol, isServer-true, serverType-rss) {
> +  list-style-image: url("chrome://messenger/skin/accountcentral/manage-rss.png");
> +  -moz-image-region: unset;

Please use -moz-image-region: auto; this is what everywhere is used.

Comment 5

[alta88]

Attachment: feedIcon.patch

Getting the icons is a real jumble and all platforms are inconsistent. This patch (for win and linux currently) standardizes the location of icons in themes/../mail/newsblog. The requirement is that an account/server icon look consistent everywhere, that a folder icon (with no favicon) be the same, and also a feed url item/message (subscribe/tab). Linux account central is copied to win.

OSX tbd.

Comment 6

[alta88]

Attachment: feedIcons2.patch

icon binary file changes (win/linux).

Comment 7

[Richard Marti (:Paenglab)]

Comment on attachment 9056624 [details] [diff] [review]
feedIcon.patch

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

Thanks, looks good. Only f+ because of the missing Mac changes.

::: mail/themes/windows/mail/folderMenus.css
@@ +69,5 @@
>    -moz-image-region: rect(0 16px 16px 0);
>  }
>  
> +.folderMenuItem {
> +  list-style-image: url("chrome://messenger/skin/icons/folder.png");

What about moving this line to line 13?

@@ +73,5 @@
> +  list-style-image: url("chrome://messenger/skin/icons/folder.png");
> +}
> +
> +.folderMenuItem[IsServer="true"] {
> +  list-style-image: url("chrome://messenger/skin/icons/server.png");

And this one to line 69. Then we don't have the same selector twice.

::: mail/themes/windows/mail/folderPane.css
@@ +52,5 @@
>  }
>  .tabmail-tab[type="folder"][IsFeedFolder="true"],
> +treechildren::-moz-tree-image(folderNameCol, isFeedFolder-true) {
> +  list-style-image: url("chrome://messenger-newsblog/skin/rss-feed-folder.png");
> +  -moz-image-region: auto;

Removing the width/height here let the favicon show in it's original dimensions.

Please readd them here. Or maybe better globally on line 17 and remove the the ones on line 50 and 51.

Comment 8

[Richard Marti (:Paenglab)]

Attachment: rss-feed-folder.png

Icon for Mac.

Comment 9

[Richard Marti (:Paenglab)]

Attachment: [e-mail]

HiDPI icon for Mac.

Comment 10

[alta88]

thanks for the mac folder icons. mac is a bit of a mess as manage-rss.png is fine for a account central, but it would have to use the icon in folder-pane.png for its similar server/account icon. and the one in server.png is what folder needs to be. to go along with the easy to understand new location in newsblog/ could you make a similar set of server/feed item group as in win/linux?

the "new" starburst isn't needed as that's now a background image on top of the icon in folderpane.

Comment 11

[Richard Marti (:Paenglab)]

Attachment: rss-feed.png

Like this?

Comment 12

[Richard Marti (:Paenglab)]

Attachment: [e-mail]

Comment 13

[alta88]

yes, that's what the other 2 platforms do. but to be consistent, the manage-rss.png should also have the orange border.

Comment 14

[Richard Marti (:Paenglab)]

Attachment: manage-rss.png

Comment 15

[Richard Marti (:Paenglab)]

Attachment: [e-mail]

Comment 16

[alta88]

Attachment: feedIcon.patch

updated css for comments.

Comment 17

[alta88]

Attachment: feedIcons2.patch

icons.

Comment 18

[Richard Marti (:Paenglab)]

Comment on attachment 9056727 [details] [diff] [review]
feedIcon.patch

The Mac part has some wrong -moz-image-region and background size definitions for HiDPI.

I'll upload a fixed version of this patch which is easier than pointing the errors and wait for a new patch.

Comment 19

[Richard Marti (:Paenglab)]

Attachment: feedIcon.patch

Fixed the mac HiDPI -moz-image-region and background-size definitions.

Comment 20

[alta88]

(In reply to Richard Marti (:Paenglab) from comment #18)

Comment on attachment 9056727 [details] [diff] [review]
feedIcon.patch

The Mac part has some wrong -moz-image-region and background size
definitions for HiDPI.

I'll upload a fixed version of this patch which is easier than pointing the
errors and wait for a new patch.

thanks! (can't test on a mac). and for the icons.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c66f9410c0c0
Sync folderpane feed account icon with Account Central icon (icon changes). r=Paenglab
https://hg.mozilla.org/comm-central/rev/e033ed07d35d
Sync folderpane feed account icon with Account Central icon. r=Paenglab