795989 - cleanup of folderpane icons

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Andreas Nilsson (:andreasn)]

Qute and Gnomestripe themes make use of imagemaps for the folderpane icons. This requires less amount of files and also helps with performance slightly.
By doing this to pinstripe too, the code will be easier to read due to being more homogeneous between the different themes.

Comment 1

[Andreas Nilsson (:andreasn)]

Attachment: folder pane icons (wip)

Need to do some testing to make sure there are no regressions.

Comment 2

[Andreas Nilsson (:andreasn)]

Attachment: updated patch

Comment 3

[Richard Marti (:Paenglab)]

This looks good. I found one error in a -moz-image-region:

 /* ..... News Folders ..... */
 .tabmail-tab[type="folder"][ServerType="nntp"],
 treechildren::-moz-tree-image(folderNameCol, serverType-nntp) {
-  list-style-image: url("chrome://messenger/skin/icons/folder-newsgroup.png");
+  -moz-image-region: rect(0 176px 160px 0);

should be:
+  -moz-image-region: rect(0 176px 16px 160px);

Please can you look to finish this patch so it can land in the near future? I'm basing bug 781333 on this bug.

Comment 4

[Andreas Nilsson (:andreasn)]

Attachment: folder pane icons (v3)

Third version of the patch, fixing the error Richard pointed out.

Comment 5

[Andreas Nilsson (:andreasn)]

Comment on attachment 682502 [details] [diff] [review]
folder pane icons (v3)

Setting up for review

Comment 6

[Richard Marti (:Paenglab)]

Comment on attachment 682502 [details] [diff] [review]
folder pane icons (v3)

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

Why is gnomestripe/mail/icons/server.png in this patch? Have you something fixed here?

ui-r- because of the three comments.

Sorry to say the patch is looking good for review with not checking it deeply.

::: mail/themes/pinstripe/mail/folderPane.css
@@ +21,5 @@
>  
>  /* ..... News Folders ..... */
>  .tabmail-tab[type="folder"][ServerType="nntp"],
>  treechildren::-moz-tree-image(folderNameCol, serverType-nntp) {
> +  -moz-image-region: rect(0 176px 16px 160px);

Sorry, my bad this should be: rect(0 160px 16px 144px) to show the news folder and not the saved search folder.

@@ +146,1 @@
>    list-style-image: url("chrome://messenger/skin/icons/server-newsblog.png");

This rule should be removed. The icon is no more existent.

@@ +149,2 @@
>  /* .....  Newsgroup .....  */
>  .tabmail-tab[type="folder"][ServerType="nntp"] {

This rule needs also to be removed.

Comment 7

[Andreas Nilsson (:andreasn)]

Attachment: folder pane icons (v4)

>Why is gnomestripe/mail/icons/server.png in this patch? Have you something fixed here?

Reverted to the old one. Will fix this graphics in a separate patch.


>::: mail/themes/pinstripe/mail/folderPane.css
>@@ +21,5 @@
>>  
>>  /* ..... News Folders ..... */
>>  .tabmail-tab[type="folder"][ServerType="nntp"],
>>  treechildren::-moz-tree-image(folderNameCol, serverType-nntp) {
>> +  -moz-image-region: rect(0 176px 16px 160px);

>Sorry, my bad this should be: rect(0 160px 16px 144px) to show the news folder and not the saved search folder.

Fixed


>@@ +146,1 @@
>>    list-style-image: url("chrome://messenger/skin/icons/server newsblog.png");

>This rule should be removed. The icon is no more existent.

Fixed.


>@@ +149,2 @@
>>  /* .....  Newsgroup .....  */
>>  .tabmail-tab[type="folder"][ServerType="nntp"] {
>This rule needs also to be removed.

Fixed

Comment 8

[Richard Marti (:Paenglab)]

Comment on attachment 682665 [details] [diff] [review]
folder pane icons (v4)

Looks good now.

Comment 9

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

Comment on attachment 682665 [details] [diff] [review]
folder pane icons (v4)

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

Looks good! Thanks Andreas!

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/d04bed4359e3