735717 - chat - toolbar icon and tab icon are different

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Andreas Nilsson (:andreasn)]

Attachment: screenshot

Right now we're using a different, non-os-specific icon for the chat tab icon. This should probably use a version of the toolbar icon instead.

Comment 1

[Andreas Nilsson (:andreasn)]

Attachment: change the tab to use the same icon as in the toolbar

Only tested on Linux so far and I need to hunt down how to populate the all-tabs-popup.

Comment 2

[Andreas Nilsson (:andreasn)]

Comment on attachment 607189 [details] [diff] [review]
change the tab to use the same icon as in the toolbar

Sorry, mixed this bugs tab up with another one, cancelling reviews. :)

Comment 3

[Andreas Nilsson (:andreasn)]

Attachment: for all platforms (v2)

Unbitrotted and updated

Comment 4

[Andreas Nilsson (:andreasn)]

Attachment: for all platforms (v3)

When I tested this on the mac, the icon was a bit too big and fuzzy, so this gives the right dimensions (16x16) in the css.

Comment 5

[Andreas Nilsson (:andreasn)]

Attachment: patch in action (win7)

Comment 6

[Andreas Nilsson (:andreasn)]

Attachment: patch in action (xp)

Comment 7

[Andreas Nilsson (:andreasn)]

Attachment: patch in action (osx)

Comment 8

[Andreas Nilsson (:andreasn)]

Attachment: patch in action (linux)

Comment 9

[Florian Quèze [:florian]]

Comment on attachment 638661 [details] [diff] [review]
for all platforms (v3)

Not sure if you meant to request review or not, but this patch regresses the chat icon in the all tabs menu (btw, see also bug 717881 for somehow related issues with the icons in the all tabs menu).

You need .alltabs-item[type="chat"] CSS selectors, and this change to mail/base/content/tabmail.xml:

diff --git a/mail/base/content/tabmail.xml b/mail/base/content/tabmail.xml
--- a/mail/base/content/tabmail.xml
+++ b/mail/base/content/tabmail.xml
@@ -2455,16 +2455,17 @@
           var menuItem = document.createElementNS(
             "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", 
             "menuitem");
 
           menuItem.setAttribute("class", "menuitem-iconic alltabs-item");
 
           menuItem.setAttribute("label", aTab.label);
           menuItem.setAttribute("crop", aTab.getAttribute("crop"));
+          menuItem.setAttribute("type", aTab.getAttribute("type"));
           menuItem.setAttribute("image", aTab.getAttribute("image"));
 
           if (aTab.hasAttribute("busy"))
             menuItem.setAttribute("busy", aTab.getAttribute("busy"));
           if (aTab.selected)
             menuItem.setAttribute("selected", "true");
 
           // Keep some attributes of the menuitem in sync with its

Comment 10

[Andreas Nilsson (:andreasn)]

Attachment: for all platforms (v4)

This patch should do the trick.

Comment 11

[Florian Quèze [:florian]]

Comment on attachment 641036 [details] [diff] [review]
for all platforms (v4)

The code is fine with me. I haven't tested the patch, so I'm assuming the values given in -moz-image-region are right :).

Comment 12

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

Comment on attachment 641036 [details] [diff] [review]
for all platforms (v4)

Sorry it took so long to get to this. This looks good.

Comment 13

[Ludovic Hirlimann [:Usul]]

Andreas did we forget this bug ?

Comment 14

[Andreas Nilsson (:andreasn)]

The patch don't apply cleanly on trunk so removing checkin-needed until I sort this out.

Comment 15

[Richard Marti (:Paenglab)]

(In reply to Andreas Nilsson (:andreasn) from comment #14)
> The patch don't apply cleanly on trunk so removing checkin-needed until I
> sort this out.

Removing the changes for tabmail.xml is enough. Then the patch applies and all is working.

Comment 16

[Andreas Nilsson (:andreasn)]

Attachment: fixes bitrot

Fixes bitrot and carrying over r+ and ui-r+
Ready for checkin

Comment 17

[Ryan VanderMeulen [:RyanVM]]

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