Last Comment Bug 735717 - chat - toolbar icon and tab icon are different
: chat - toolbar icon and tab icon are different
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: Thunderbird 19.0
Assigned To: Andreas Nilsson (:andreasn)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-14 09:22 PDT by Andreas Nilsson (:andreasn)
Modified: 2012-11-16 16:08 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
screenshot (5.52 KB, image/png)
2012-03-14 09:22 PDT, Andreas Nilsson (:andreasn)
no flags Details
change the tab to use the same icon as in the toolbar (2.62 KB, patch)
2012-03-19 09:22 PDT, Andreas Nilsson (:andreasn)
no flags Details | Diff | Review
for all platforms (v2) (2.46 KB, patch)
2012-07-02 09:37 PDT, Andreas Nilsson (:andreasn)
no flags Details | Diff | Review
for all platforms (v3) (2.41 KB, patch)
2012-07-03 05:22 PDT, Andreas Nilsson (:andreasn)
florian: review-
Details | Diff | Review
patch in action (win7) (18.67 KB, image/png)
2012-07-03 05:37 PDT, Andreas Nilsson (:andreasn)
no flags Details
patch in action (xp) (15.14 KB, image/png)
2012-07-03 05:39 PDT, Andreas Nilsson (:andreasn)
no flags Details
patch in action (osx) (12.45 KB, image/png)
2012-07-03 06:18 PDT, Andreas Nilsson (:andreasn)
no flags Details
patch in action (linux) (8.85 KB, image/png)
2012-07-03 06:21 PDT, Andreas Nilsson (:andreasn)
no flags Details
for all platforms (v4) (3.00 KB, patch)
2012-07-11 06:48 PDT, Andreas Nilsson (:andreasn)
florian: review+
mconley: ui‑review+
Details | Diff | Review
fixes bitrot (2.50 KB, patch)
2012-11-16 09:24 PST, Andreas Nilsson (:andreasn)
bugs: review+
bugs: ui‑review+
Details | Diff | Review

Description Andreas Nilsson (:andreasn) 2012-03-14 09:22:34 PDT
Created attachment 605796 [details]
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) 2012-03-19 09:22:17 PDT
Created attachment 607189 [details] [diff] [review]
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) 2012-07-02 06:55:54 PDT
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) 2012-07-02 09:37:40 PDT
Created attachment 638396 [details] [diff] [review]
for all platforms (v2)

Unbitrotted and updated
Comment 4 Andreas Nilsson (:andreasn) 2012-07-03 05:22:16 PDT
Created attachment 638661 [details] [diff] [review]
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) 2012-07-03 05:37:50 PDT
Created attachment 638666 [details]
patch in action (win7)
Comment 6 Andreas Nilsson (:andreasn) 2012-07-03 05:39:12 PDT
Created attachment 638667 [details]
patch in action (xp)
Comment 7 Andreas Nilsson (:andreasn) 2012-07-03 06:18:28 PDT
Created attachment 638684 [details]
patch in action (osx)
Comment 8 Andreas Nilsson (:andreasn) 2012-07-03 06:21:04 PDT
Created attachment 638685 [details]
patch in action (linux)
Comment 9 Florian Quèze [:florian] [:flo] 2012-07-10 09:17:10 PDT
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) 2012-07-11 06:48:29 PDT
Created attachment 641036 [details] [diff] [review]
for all platforms (v4)

This patch should do the trick.
Comment 11 Florian Quèze [:florian] [:flo] 2012-07-11 07:28:36 PDT
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) - (needinfo me!) 2012-08-01 11:11:28 PDT
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] 2012-09-12 03:04:11 PDT
Andreas did we forget this bug ?
Comment 14 Andreas Nilsson (:andreasn) 2012-09-12 06:51:02 PDT
The patch don't apply cleanly on trunk so removing checkin-needed until I sort this out.
Comment 15 Richard Marti (:Paenglab) 2012-09-16 05:35:46 PDT
(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) 2012-11-16 09:24:56 PST
Created attachment 682500 [details] [diff] [review]
fixes bitrot

Fixes bitrot and carrying over r+ and ui-r+
Ready for checkin
Comment 17 Ryan VanderMeulen [:RyanVM] 2012-11-16 16:08:11 PST
https://hg.mozilla.org/comm-central/rev/eee2bbaeaff1

Note You need to log in before you can comment on or make changes to this bug.