Closed Bug 955122 Opened 10 years ago Closed 10 years ago

Use twitter icon for twitter tabs

Categories

(Instantbird Graveyard :: Conversation, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: aleth)

Details

(Whiteboard: [1.3-wanted])

Attachments

(3 files, 2 obsolete files)

*** Original post on bio 1694 at 2012-09-10 22:50:00 UTC ***

Currently the tab icon is set via instantbird.css and all chat tabs get the chat icon. Probably the way to go here is to instead set the icon used for MUCs from the conversation binding and then pick it up somehow via CSS? Not entirely sure how best to do this.
*** Original post on bio 1694 at 2012-09-11 09:25:27 UTC ***

Maybe set a prpl attribute in addition to the "chat" attribute on the tab, and special case [prpl="prpl-twitter"] in the css?
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1694 as attmnt 1960 at 2012-10-14 21:10:00 UTC ***

Since CSS attr() doesn't support uri's yet, this seems the best way to go for now.
Attachment #8353719 - Flags: review?(florian)
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1694 as attmnt 1966 at 2012-10-16 12:42:00 UTC ***

Adds a new icon for a disconnected twitter tab to /chat.

Let me know if the binary part of the patch doesn't apply with hg.

The new file also has to be added to the packaging somewhere (jar.mn, or is /chat separated out somehow?).
Attachment #8353725 - Flags: review?(florian)
Comment on attachment 8353719 [details] [diff] [review]
Patch

*** Original change on bio 1694 attmnt 1960 at 2012-10-16 12:42:10 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353719 - Attachment is obsolete: true
Attachment #8353719 - Flags: review?(florian)
Attached image Twitter-left icon
*** Original post on bio 1694 as attmnt 1967 at 2012-10-16 12:45:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
*** Original post on bio 1694 at 2012-10-16 12:56:31 UTC ***

(In reply to comment #3)
> The new file also has to be added to the packaging somewhere (jar.mn, or is
> /chat separated out somehow?).
Ah, I see each /chat protocol has it's own jar.mn.
Comment on attachment 8353725 [details] [diff] [review]
Patch

*** Original change on bio 1694 attmnt 1966 at 2012-10-27 02:40:11 UTC ***

The prpl of a conversation.xml can change when the imIConversation receives the "target-purple-conversation-changed" notification.
Attachment #8353725 - Flags: review?(florian) → review-
Comment on attachment 8353726 [details]
Twitter-left icon

*** Original change on bio 1694 attmnt 1967 at 2012-10-27 02:42:08 UTC ***

This icon's background needs to be transparent.
Attachment #8353726 - Flags: review-
*** Original post on bio 1694 at 2012-10-27 02:44:06 UTC ***

This looks OKish otherwise, but the new icon needs to be added in jar.mn.
Comment on attachment 8353726 [details]
Twitter-left icon

*** Original change on bio 1694 attmnt 1967 at 2012-10-27 02:56:14 UTC ***

(In reply to comment #7)
> Comment on attachment 8353726 [details] (bio-attmnt 1967) [details]
> Twitter-left icon
> 
> This icon's background needs to be transparent.

Scratch that, it was just Firefox's image display being stupid (what's the point of the black background to center the image of the image is displayed with a white background? :(). Sorry for the noise.
Attachment #8353726 - Flags: review-
*** Original post on bio 1694 at 2012-10-28 13:27:03 UTC ***

(In reply to comment #6)
> The prpl of a conversation.xml can change when the imIConversation receives the
> "target-purple-conversation-changed" notification.

This does not apply for MUCs.

As I mentioned above, I would have preferred a less ugly solution but the patch got messier at each stage due to the limitations of CSS.
Attached patch Patch for jar.mnSplinter Review
*** Original post on bio 1694 as attmnt 2019 at 2012-10-28 15:26:00 UTC ***

I'm pretty sure the FF img viewer used to display transparency "correctly", it must have been changed when they moved to a black background.
Attachment #8353779 - Flags: review?(florian)
Attached patch PatchSplinter Review
*** Original post on bio 1694 as attmnt 2020 at 2012-10-28 15:32:00 UTC ***

Filename change only.
Attachment #8353780 - Flags: review?(florian)
Comment on attachment 8353725 [details] [diff] [review]
Patch

*** Original change on bio 1694 attmnt 1966 at 2012-10-28 15:32:26 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353725 - Attachment is obsolete: true
*** Original post on bio 1694 at 2012-11-02 00:23:12 UTC ***

I think we can take this for 1.3.
Whiteboard: [1.3-wanted]
Comment on attachment 8353779 [details] [diff] [review]
Patch for jar.mn

*** Original change on bio 1694 attmnt 2019 at 2012-11-02 23:44:25 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353779 - Flags: review?(florian) → review+
Comment on attachment 8353780 [details] [diff] [review]
Patch

*** Original change on bio 1694 attmnt 2020 at 2012-11-02 23:44:31 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353780 - Flags: review?(florian) → review+
*** Original post on bio 1694 at 2012-11-03 00:07:06 UTC ***

Comment on attachment 8353780 [details] [diff] [review] (bio-attmnt 2020)
Patch

>diff --git a/chat/protocols/twitter/icons/prpl-twitter-left.png  b/chat/protocols/twitter/icons/prpl-twitter-left.png
>index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..fc1906da078e3e50cd60e67ab513ce49d335baf1 100644

Applying by hand patches that don't apply directly is already painful, but a binary patch that doesn't apply is even worse. It took me several minutes to understand what I had to hand edit in that patch to get rid of the unfriendly error message ("abort: failed to synchronize metadata"). It was because the patch changes a binary file (that doesn't exist) instead of creating a new one. Please don't do that again, thanks :-).
*** Original post on bio 1694 at 2012-11-03 04:23:25 UTC ***

Committed as http://hg.instantbird.org/instantbird/rev/4a82200a2803
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3
*** Original post on bio 1694 at 2012-11-04 13:29:15 UTC ***

(In reply to comment #14)
> Applying by hand patches that don't apply directly is already painful, but a
> binary patch that doesn't apply is even worse. It took me several minutes to
> understand what I had to hand edit in that patch to get rid of the unfriendly
> error message ("abort: failed to synchronize metadata"). It was because the
> patch changes a binary file (that doesn't exist) instead of creating a new one.
> Please don't do that again, thanks :-).
:) I thought something might go wrong there (see comment #3), and that it would probably be easier to checkin the icon file by hand, so I attached it separately. Sorry for all the extra work.
You need to log in before you can comment on or make changes to this bug.