Last Comment Bug 735995 - icons of twitter accounts should be shown in each tweet
: icons of twitter accounts should be shown in each tweet
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Instant Messaging (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: Thunderbird 14.0
Assigned To: Andreas Nilsson (:andreasn)
:
Mentors:
Depends on: 741856 742235
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-15 00:38 PDT by Masayuki Nakano [:masayuki] (Mozilla Japan)
Modified: 2012-04-24 06:15 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
first patch (1.34 KB, patch)
2012-03-16 05:51 PDT, Andreas Nilsson (:andreasn)
no flags Details | Diff | Splinter Review
patch (v2) (3.14 KB, patch)
2012-03-16 06:43 PDT, Andreas Nilsson (:andreasn)
no flags Details | Diff | Splinter Review
patch (v3) (4.41 KB, patch)
2012-03-19 04:48 PDT, Andreas Nilsson (:andreasn)
mconley: review+
Details | Diff | Splinter Review
patch (v3) (4.41 KB, patch)
2012-03-26 05:27 PDT, Andreas Nilsson (:andreasn)
bugs: review+
bwinton: ui‑review+
Details | Diff | Splinter Review

Description Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-03-15 00:38:32 PDT
The icons of twitter accounts should be shown in each tweet.
Comment 1 Andreas Nilsson (:andreasn) 2012-03-15 06:13:33 PDT
Do you mean the avatars?
Comment 2 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-03-15 06:41:28 PDT
I meant "Picture" in the editing profile page of Twitter.
Comment 3 Andreas Nilsson (:andreasn) 2012-03-15 14:22:51 PDT
Agreed, lets do it!
Assigning to me.
Comment 4 Andreas Nilsson (:andreasn) 2012-03-16 05:51:06 PDT
Created attachment 606542 [details] [diff] [review]
first patch

Still some small issues to iron out, but mostly there.
Comment 5 Andreas Nilsson (:andreasn) 2012-03-16 06:43:11 PDT
Created attachment 606556 [details] [diff] [review]
patch (v2)

This makes sure that it works even if you don't have a avatar set.
Comment 6 Andreas Nilsson (:andreasn) 2012-03-19 04:48:19 PDT
Created attachment 607118 [details] [diff] [review]
patch (v3)

Also makes sure to give avatars to incoming messages without avatar icon set and gives the photo some border-radius.
Comment 7 Mike Conley (:mconley) - (Needinfo me!) 2012-03-23 12:42:12 PDT
Comment on attachment 607118 [details] [diff] [review]
patch (v3)

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

Ack - I had a review for this, but I guess it never got published!  :/

The code looks good, except for the nit I found. r=me.

::: mail/components/im/messages/main.css
@@ +179,5 @@
>    background: url('Bitmaps/minus-hover.png') no-repeat left top;
>  }
>  
> +.usericon {
> +  position: absolute;'

There's a trailing apostrophe here that needs to go.
Comment 8 Andreas Nilsson (:andreasn) 2012-03-26 05:27:50 PDT
Created attachment 609291 [details] [diff] [review]
patch (v3)

Apostrophe typo removed, carrying over r+ from previous patch
Comment 9 Blake Winton (:bwinton) (:☕️) 2012-03-26 13:41:18 PDT
Comment on attachment 609291 [details] [diff] [review]
patch (v3)

I'm going to say ui-r=me, based on the Mac behaviour.  I'm trying to see how it looks on Windows, but it's going really slowly, and it would be nice if it could get checked in while we wait for my VM to build…

Thanks,
Blake.
Comment 10 Mike Conley (:mconley) - (Needinfo me!) 2012-03-26 13:58:30 PDT
Pushed to comm-central as http://hg.mozilla.org/comm-central/rev/7105b9c97546

Did we want this for comm-aurora too?
Comment 11 Florian Quèze [:florian] [:flo] 2012-04-03 10:05:04 PDT
This caused bug 741856.
Comment 12 Mark Banner (:standard8) 2012-04-04 02:25:35 PDT
Should we not push this to aurora until bug 741856 is fixed then?
Comment 13 Florian Quèze [:florian] [:flo] 2012-04-04 02:27:56 PDT
(In reply to Mark Banner (:standard8) from comment #12)
> Should we not push this to aurora until bug 741856 is fixed then?

That's what I would do, but others may disagree.
Comment 14 Mark Banner (:standard8) 2012-04-04 02:34:20 PDT
Comment on attachment 609291 [details] [diff] [review]
patch (v3)

Ok, backing this down to a request, as we need the other bug fixing first.
Comment 15 Florian Quèze [:florian] [:flo] 2012-04-24 03:53:15 PDT
Comment on attachment 609291 [details] [diff] [review]
patch (v3)

We are disabling IM for Tb13 so I don't think we want this on aurora anymore.
Comment 16 Florian Quèze [:florian] [:flo] 2012-04-24 06:15:13 PDT
(In reply to Florian Quèze from comment #15)
> Comment on attachment 609291 [details] [diff] [review]
> patch (v3)
> 
> We are disabling IM for Tb13 so I don't think we want this on aurora anymore.

Some clarification:
- disabling IM for Tb13 beta is happening in bug 748323
- bug 741856 (the reason why we haven't landed this on aurora earlier) has been resolved as WORKSFORME because bug 742235 got fixed and improved this significantly.

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