Closed Bug 795989 Opened 8 years ago Closed 7 years ago

cleanup of folderpane icons

Categories

(Thunderbird :: Theme, defect)

x86_64
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 20.0

People

(Reporter: andreasn, Assigned: andreasn)

Details

Attachments

(1 file, 3 obsolete files)

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.
Attached patch folder pane icons (wip) (obsolete) — Splinter Review
Need to do some testing to make sure there are no regressions.
OS: Linux → Mac OS X
Attached file updated patch (obsolete) —
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.
Attached patch folder pane icons (v3) (obsolete) — Splinter Review
Third version of the patch, fixing the error Richard pointed out.
Attachment #666604 - Attachment is obsolete: true
Attachment #667987 - Attachment is obsolete: true
Comment on attachment 682502 [details] [diff] [review]
folder pane icons (v3)

Setting up for review
Attachment #682502 - Flags: ui-review?(richard.marti)
Attachment #682502 - Flags: review?(mconley)
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.
Attachment #682502 - Flags: ui-review?(richard.marti) → ui-review-
>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
Attachment #682502 - Attachment is obsolete: true
Attachment #682502 - Flags: review?(mconley)
Attachment #682665 - Flags: ui-review?(richard.marti)
Comment on attachment 682665 [details] [diff] [review]
folder pane icons (v4)

Looks good now.
Attachment #682665 - Flags: ui-review?(richard.marti)
Attachment #682665 - Flags: ui-review+
Attachment #682665 - Flags: review?(mconley)
Comment on attachment 682665 [details] [diff] [review]
folder pane icons (v4)

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

Looks good! Thanks Andreas!
Attachment #682665 - Flags: review?(mconley) → review+
https://hg.mozilla.org/comm-central/rev/d04bed4359e3
Assignee: nobody → bugs
Status: NEW → RESOLVED
Closed: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 20.0
You need to log in before you can comment on or make changes to this bug.