Last Comment Bug 735217 - Status icons styled incorrectly
: Status icons styled incorrectly
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: All Windows 7
: -- normal (vote)
: Thunderbird 14.0
Assigned To: Andreas Nilsson (:andreasn)
:
Mentors:
Depends on:
Blocks: 714733
  Show dependency treegraph
 
Reported: 2012-03-13 06:56 PDT by Andreas Nilsson (:andreasn)
Modified: 2012-04-04 02:31 PDT (History)
4 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
Patch to fix the issue (4.62 KB, patch)
2012-03-13 09:30 PDT, Andreas Nilsson (:andreasn)
no flags Details | Diff | Splinter Review
works in osx too (6.12 KB, patch)
2012-03-14 06:44 PDT, Andreas Nilsson (:andreasn)
no flags Details | Diff | Splinter Review
and works correctly on Linux too (6.23 KB, patch)
2012-03-14 17:39 PDT, Andreas Nilsson (:andreasn)
mconley: review+
bwinton: ui‑review+
mozilla: approval‑comm‑aurora+
Details | Diff | Splinter Review

Description Andreas Nilsson (:andreasn) 2012-03-13 06:56:41 PDT
There are two places that we still display wrongly styled status icons (from the shared resource rather than from the individual platform theme)

1. Tools > Chat status > Unavailable
2. The icons in the Tools > Chat status > Show accounts dialog.
Comment 1 Andreas Nilsson (:andreasn) 2012-03-13 09:30:45 PDT
Created attachment 605423 [details] [diff] [review]
Patch to fix the issue

As a bonus, I broke out some ifdef's in imAccounts.css
Only tested on Windows so far.
Comment 2 Andreas Nilsson (:andreasn) 2012-03-14 06:44:27 PDT
Created attachment 605726 [details] [diff] [review]
works in osx too

Seems I forgot the Pinstripe part yesterday. Works on Mac too now.
Comment 3 Andreas Nilsson (:andreasn) 2012-03-14 17:39:52 PDT
Created attachment 606037 [details] [diff] [review]
and works correctly on Linux too

Had to do some extra tricks in gnomestripe to show the icons in the two status menus.
Comment 4 Mike Conley (:mconley) - (Needinfo me!) 2012-03-19 08:21:38 PDT
Comment on attachment 606037 [details] [diff] [review]
and works correctly on Linux too

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

Andreas:

This makes sense to me - good work!  Just one question about the "displayNameAndstatusMessageStack" id.  Assuming my assumption is correct, r=me.

Thanks,

-Mike

::: mail/components/im/themes/imAccounts.css
@@ +342,3 @@
>  #statusTypeIcon[status="idle"] {
>    list-style-image: url('chrome://chat/skin/idle-16.png');
>  }

Hooray for axeing ifdefs! :)

::: mail/themes/pinstripe/mail/chat.css
@@ +166,5 @@
> +#displayNameAndstatusMessageStack #statusMessage[editing] {
> +  margin: 29px 2px 0 -4px;
> +}
> +
> +#displayNameAndstatusMessageStack #displayName[editing] {

I assume the ID for this element is indeed @displayNameAndstatusMessageStack, with the lowercased "s" in status, and not a copy-paste error...correct?
Comment 5 Andreas Nilsson (:andreasn) 2012-03-19 08:30:44 PDT
(In reply to Mike Conley (:mconley) from comment #4)

> > +#displayNameAndstatusMessageStack #displayName[editing] {
> 
> I assume the ID for this element is indeed
> @displayNameAndstatusMessageStack, with the lowercased "s" in status, and
> not a copy-paste error...correct?

Yes.
http://mxr.mozilla.org/comm-central/source/mail/components/im/content/imAccounts.xul#149
Comment 6 Mike Conley (:mconley) - (Needinfo me!) 2012-03-19 08:38:49 PDT
(In reply to Andreas Nilsson (:andreasn) from comment #5)
> (In reply to Mike Conley (:mconley) from comment #4)
> 
> > > +#displayNameAndstatusMessageStack #displayName[editing] {
> > 
> > I assume the ID for this element is indeed
> > @displayNameAndstatusMessageStack, with the lowercased "s" in status, and
> > not a copy-paste error...correct?
> 
> Yes.
> http://mxr.mozilla.org/comm-central/source/mail/components/im/content/
> imAccounts.xul#149

Cool, looks good then. :)
Comment 7 Blake Winton (:bwinton) (:☕️) 2012-03-27 13:49:32 PDT
Comment on attachment 606037 [details] [diff] [review]
and works correctly on Linux too

I'm going to say ui-r=me, based on the Mac, and hoping that the other platforms also work.  ;)

Thanks,
Blake.
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-03-28 17:36:10 PDT
http://hg.mozilla.org/comm-central/rev/3aecafffd8e9

Also, to make life easier for those checking in patches for you, please follow the directions below for any future patches you submit. Thanks!
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Comment 9 Florian Quèze [:florian] [:flo] 2012-04-03 06:55:44 PDT
Comment on attachment 606037 [details] [diff] [review]
and works correctly on Linux too

[Approval Request Comment]
User impact if declined: inconsistency between the status icons displayed in different places of the UI.
Comment 10 Mark Banner (:standard8) (afk until 26th July) 2012-04-04 02:31:54 PDT
http://hg.mozilla.org/releases/comm-aurora/rev/71030ae61bce

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