Last Comment Bug 739181 - Chat - The contact info header in the sidebar incorrectly gets a persona background.
: Chat - The contact info header in the sidebar incorrectly gets a persona back...
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Theme (show other bugs)
: unspecified
: All Mac OS X
-- normal (vote)
: Thunderbird 14.0
Assigned To: Richard Marti (:Paenglab)
:
:
Mentors:
Depends on:
Blocks: 714733
  Show dependency treegraph
 
Reported: 2012-03-26 04:48 PDT by Andreas Nilsson (:andreasn)
Modified: 2012-04-04 02:32 PDT (History)
3 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
screenshot of the issue (75.11 KB, image/png)
2012-03-26 04:48 PDT, Andreas Nilsson (:andreasn)
no flags Details
Patch (783 bytes, patch)
2012-03-31 01:43 PDT, Richard Marti (:Paenglab)
no flags Details | Diff | Splinter Review
Patch v2 (1.50 KB, patch)
2012-03-31 02:57 PDT, Richard Marti (:Paenglab)
bugs: ui‑review-
Details | Diff | Splinter Review
Patch v3 (1.40 KB, patch)
2012-04-02 11:53 PDT, Richard Marti (:Paenglab)
bugs: review+
bugs: ui‑review+
standard8: approval‑comm‑aurora+
Details | Diff | Splinter Review

Description User image Andreas Nilsson (:andreasn) 2012-03-26 04:48:43 PDT
Created attachment 609283 [details]
screenshot of the issue

The contact info header in the sidebar incorrectly gets a persona background on the mac.
Comment 1 User image Richard Marti (:Paenglab) 2012-03-31 01:43:48 PDT
Created attachment 611134 [details] [diff] [review]
Patch

Set for contextPane a background color to avoid lightweight theme backgrounds.
Comment 2 User image Florian Quèze [:florian] [:flo] 2012-03-31 02:05:21 PDT
My intent was to have in this area the same background as the email headers have in the email tab.
Any reason why http://mxr.mozilla.org/comm-central/source/mail/components/im/themes/chat.css#117 doesn't produce this effect?
Comment 3 User image Richard Marti (:Paenglab) 2012-03-31 02:37:02 PDT
The styles for the Persona (Lightweight themes) are overwriting this style. With a !important this would override the Persona styles. I can do this and add it to this patch. But because this change is in /mail/components/im/themes/chat.css do I need a review from you?
Comment 4 User image Florian Quèze [:florian] [:flo] 2012-03-31 02:45:48 PDT
If just adding an !important to the existing rule is enough (and keeps the grandient that looks like a shadow) I think I would prefer that, but I'm ok with letting Andreas decide how things should look.

I think for the mail/components/im/ folder you can get reviews like for other parts of the Thunderbird UI, but I'm not completely sure of the review policy for that folder actually. As I'm the author of most of these files I'm likely to look and have an opinion even if I'm not the official reviewer. :)
Comment 5 User image Richard Marti (:Paenglab) 2012-03-31 02:57:02 PDT
Created attachment 611142 [details] [diff] [review]
Patch v2

Make the toolbar shadow appear also with Persona applied.
Comment 6 User image Andreas Nilsson (:andreasn) 2012-04-02 10:17:10 PDT
I think the shadow works well in the single message case, because it looks like the toolbar casts a shadow across the whole area under it, but in this case it looks a bit weird that just the right pane gets a shadow.
Comment 7 User image Andreas Nilsson (:andreasn) 2012-04-02 10:30:16 PDT
it would also be good with a line above "Previous conversations"
Comment 8 User image Richard Marti (:Paenglab) 2012-04-02 11:53:18 PDT
Created attachment 611538 [details] [diff] [review]
Patch v3

I've removed the shadow completely and gave the bottom border a !important to show also with Personas enabled.
Comment 9 User image Andreas Nilsson (:andreasn) 2012-04-03 03:35:54 PDT
Comment on attachment 611538 [details] [diff] [review]
Patch v3

I'm not super-happy about parts of this code living inside a %ifdef instead of the platform theme directory, but thats how it was before as well, so it will work for now.
I like to see css documentation though, so that makes me happy again :)
Visually, it's perfect!
Comment 10 User image Florian Quèze [:florian] [:flo] 2012-04-03 03:43:24 PDT
Comment on attachment 611538 [details] [diff] [review]
Patch v3

I'm ok with letting Andreas decide how this should look and review this, so you don't really need my review here (I've looked quickly at the patch and see nothing wrong, but I haven't tested it, so not putting the r+ flag).
Comment 11 User image Ryan VanderMeulen [:RyanVM] 2012-04-03 16:37:19 PDT
http://hg.mozilla.org/comm-central/rev/a767bf83251a
Comment 12 User image Richard Marti (:Paenglab) 2012-04-04 00:36:26 PDT
Comment on attachment 611538 [details] [diff] [review]
Patch v3

[Approval Request Comment]
User impact if declined: Inconsistent UI between versions from beginning
Comment 13 User image Mark Banner (:standard8) 2012-04-04 02:32:46 PDT
http://hg.mozilla.org/releases/comm-aurora/rev/a82506eb7e8c

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