Closed Bug 954545 Opened 10 years ago Closed 10 years ago

Only show active nicks in color in participant list

Categories

(Instantbird Graveyard :: Conversation, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: aleth)

Details

Attachments

(2 files, 16 obsolete files)

2.16 KB, patch
florian
: review+
Details | Diff | Splinter Review
99.02 KB, image/png
Details
*** Original post on bio 1112 at 2011-10-23 15:00:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached patch Patch for conversation.xml (obsolete) — Splinter Review
*** Original post on bio 1112 as attmnt 922 at 2011-10-23 15:00:00 UTC ***

This patch sets the nick color of all MUC participants to gray until a participant actually participates (i.e. sends a message).

I'm not super happy about hardcoding the color used for "inactive participants" but 1) gray is never used for nicks and 2) I can't see anything in the message style that would be appropriate, so I think it should be OK in this case.

Possible improvements/changes might include
- set nick color back to inactive after a certain timeout and/or if the participant sets their status to 'away' (not sure if this info is pushed to IB though)
- option to hide the inactive nicks altogether (for channels with thousands of participants)
Attachment #8352664 - Flags: review?
*** Original post on bio 1112 at 2011-10-23 15:10:33 UTC ***

(In reply to comment #0)
> Created an attachment (id=922) [details]
> Patch for conversation.xml
> 
> This patch sets the nick color of all MUC participants to gray until a
> participant actually participates (i.e. sends a message).
Would it make more sense to do the same thing we do to context messages, that way they still retain their original color, but it's washed out? (This would, in particular, be useful if you'll again mark participants as "inactive" at some point).

> Possible improvements/changes might include
> - set nick color back to inactive after a certain timeout and/or if the
> participant sets their status to 'away' (not sure if this info is pushed to IB
> though)
It doesn't seem like we have this information [1] although I'd really like to provide it in at least our JS protocols.

> - option to hide the inactive nicks altogether (for channels with thousands of
> participants)
Alternately sorting the active above the inactive might also be an option. (See bug 953656 (bio 210) about sorting participants in MUCs).

Could you provide a screenshot please? Thanks.

[1] http://lxr.instantbird.org/instantbird/source/purple/purplexpcom/public/purpleIConversation.idl#159
Attached image Screenshot (obsolete) —
*** Original post on bio 1112 as attmnt 926 at 2011-10-23 21:41:00 UTC ***

(In reply to comment #1)
> Would it make more sense to do the same thing we do to context messages, that
> way they still retain their original color, but it's washed out? (This would,
> in particular, be useful if you'll again mark participants as "inactive" at
> some point).

I considered it, but it's much harder to distinguish between desaturated colors and active colors when there is no background and you are scrolling through a list of 1000 participants. Also, participants of context messages may still be active (you may just have put the conversation on hold for a few minutes), so the two are not the same thing.

> Alternately sorting the active above the inactive might also be an option. (See
> bug 953656 (bio 210) about sorting participants in MUCs).

Yes, but like you say I think that's part of the larger sorting issue. It will be no problem at a later stage to treat nicks with color==0 differently.
Attached patch Patch for conversation.xml (obsolete) — Splinter Review
*** Original post on bio 1112 as attmnt 927 at 2011-10-23 21:41:00 UTC ***

Fixes incorrect bubble color on a participant's first post.

There is one problem remaining with this patch: The user's nick's color is not set correctly in the participant list. However, it is *also* not set correctly in the present IB code: the color in the participant list and that of the outgoing messages does not match. This is because the participant list color is calculated algorithmically, while the bubble color is set by message style to whatever is coded there for outgoing messages (usually variant-dependent). 

I don't know how to 1) obtain this color or 2) get at the user's nick/alias without waiting for him to post an outgoing message. So the user himself currently appears inactive in the participant list, which is not good.

Asking for review/help with this last issue.
Attachment #8352669 - Flags: review?(clokep)
Comment on attachment 8352664 [details] [diff] [review]
Patch for conversation.xml

*** Original change on bio 1112 attmnt 922 at 2011-10-23 21:41:33 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352664 - Attachment is obsolete: true
Attachment #8352664 - Flags: review?
*** Original post on bio 1112 at 2011-10-23 22:15:07 UTC ***

(In reply to comment #2)
> Created an attachment (id=926) [details]
> Screenshot
> 
> (In reply to comment #1)
> > Would it make more sense to do the same thing we do to context messages, that
> > way they still retain their original color, but it's washed out? (This would,
> > in particular, be useful if you'll again mark participants as "inactive" at
> > some point).
> 
> I considered it, but it's much harder to distinguish between desaturated colors
> and active colors when there is no background and you are scrolling through a
> list of 1000 participants. Also, participants of context messages may still be
> active (you may just have put the conversation on hold for a few minutes), so
> the two are not the same thing.
I understand they're not the same thing, but the colors are there to link everyone together, by possibly marking people gray after a time of not talking, you lose that...
*** Original post on bio 1112 at 2011-10-23 22:24:18 UTC ***

(In reply to comment #4)
> I understand they're not the same thing, but the colors are there to link
> everyone together, by possibly marking people gray after a time of not talking,
> you lose that...

I agree, personally I don't like the idea of setting people back to inactive after a time of not talking. I only mentioned it because it was discussed at one point.

What you might be after would be an _additional_ differentiation: set nick color to desaturated if the last message by the nick was a context message. I kind of think that's unneccessary.
*** Original post on bio 1112 as attmnt 928 at 2011-10-23 22:36:00 UTC ***

(In reply to comment #3)
> There is one problem remaining with this patch: The user's nick's color is not
> set correctly in the participant list. However, it is *also* not set correctly
> in the present IB code: the color in the participant list and that of the
> outgoing messages does not match. This is because the participant list color is
> calculated algorithmically, while the bubble color is set by message style to
> whatever is coded there for outgoing messages (usually variant-dependent). 
I've changed it so outgoing messages are set to the participant's color. Attaching a diff so that someone else can easily apply the patch.
*** Original post on bio 1112 at 2011-10-23 22:44:26 UTC ***

(In reply to comment #6)
That change will make nick and the bubble color match, but it modifies the color of outgoing bubbles compared to normal conversations. This then means that the user will have outgoing messages in a different color in MUCs than in normal conversations.

If this is not a problem, that's indeed all that's needed. Kind of an IB UI policy question...
*** Original post on bio 1112 at 2011-10-24 11:54:38 UTC ***

I wonder if this is taking the right direction?

As far as I see being 'active' means that the participant has talked atleast once since I've been in the channel, no matter how long ago this was.

What about the following:

* Calculate and set the color for each participant P
* Toggle the flag from inactive to active when P sends a message [1].
* Reset to inactive after a while [2].
* Apply a filter or different color using a CSS rule [3].



[1] Write the time of the message into an attribute of the participant's node and use it to:
[2] reset the flag to inactive if the participant hasn't been active for <defined number of minutes>. A timer would call a method that updates the list maybe once a minute?
[3] Maybe this would need a !important somewhere if it is in a CSS file: styling via the style-attribute (as it happens at the moment) has the highest selectivity
*** Original post on bio 1112 at 2011-10-24 11:57:17 UTC ***

(In reply to comment #8)
> I wonder if this is taking the right direction?
> 
> As far as I see being 'active' means that the participant has talked atleast
> once since I've been in the channel, no matter how long ago this was.
> 
> What about the following:
> 
> * Calculate and set the color for each participant P
> * Toggle the flag from inactive to active when P sends a message [1].
> * Reset to inactive after a while [2].
> * Apply a filter or different color using a CSS rule [3].
> 
> 
> 
> [1] Write the time of the message into an attribute of the participant's node
> and use it to:
> [2] reset the flag to inactive if the participant hasn't been active for
> <defined number of minutes>. A timer would call a method that updates the list
> maybe once a minute?
> [3] Maybe this would need a !important somewhere if it is in a CSS file:
> styling via the style-attribute (as it happens at the moment) has the highest
> selectivity

Doing it right on restored conversations (put on hold and then opened again) might require to go through the list of old messages to figure out the time of the last message. Atleast I guess the whole visible conversation including the participant list is actually destroyed and loses all attached data?
*** Original post on bio 1112 at 2011-10-24 21:34:24 UTC ***

(In reply to comment #8)
A couple of independent issues mentioned in the comments here
and on IRC:

- Using a CSS rule to style inactive nicks: This sounds like a more elegant way to implement how inactive nicks look that allows for theming. Good idea, but I don't know how to do it ;)

- Inactive/active nicks vs context messages and their styling: Imho the two are orthogonal. Context messages denote "You've read this already" and their styling
must be such that different speakers, i.e. different nick colors, remain easily *distinguishable*. Conversely, inactive participants should look visually *indistinguishable* in the list so that the active nicks stand out at a glance. (At least, that's the usability enhancement I wrote the patch for.) This means styling inactive nicks using desaturation or opacity while retaining color is not a good idea imho.

- Styling of the op/voice/bot icon for inactive nicks: Mic suggested using desaturation so the icons don't stand out so much from the grey inactive nicks. I'm a little hesitant as I think it's quite important that the user can quickly pick out the ops (e.g.) by scrolling the participant list without having to wonder whether "dirty yellow star" means the same as "bright yellow star". If this is desired, I'd suggest using opacity which doesn't change the tone of the colour so much.

- Returning nicks to 'inactive' after a period of silence: I think this can be added as a separate patch after the exact way it would happen, and how it interacts with putting conversations on hold etc, has been decided after more discussion. (At present, leaving an IRC tab open for days won't happen much
anyway because it takes so long to reopen a chat from hold after it has grown
sizeable.) Nothing is lost by implementing this "half" first.

- There's still the minor but annoying issue of getting hold of the outgoing message color from the message style.
*** Original post on bio 1112 at 2011-10-24 21:51:54 UTC ***

Regarding the CSS rule idea: could it really be as simple as buddies[name].setAttribute("class","inactive")? #stilllearningXUL
*** Original post on bio 1112 at 2011-10-24 22:14:10 UTC ***

- I agree that "said something only in context messages" isn't equal to "inactive".

- I think in the future we will want old (*read*) messages to turn automatically into context messages after some time. Once we have that, participants having talked only in old messages should become inactive, and we can probably get the "context" and "inactive" concepts closer to each other. This doesn't need to happen in this bug. :-)

- I also think we need to do something with the op/half-op/... icons when the participant is inactive. Not sure if it's about the saturation, the opacity or whatever. Try it, we will see what works :-).

- Please, go ahead in the CSS direction rather than hacking the style="color: ..." attribute :-).
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1112 as attmnt 931 at 2011-10-24 23:26:00 UTC ***

Better!

Thanks Mic for the tip.
Attachment #8352673 - Flags: review?(florian)
Comment on attachment 8352669 [details] [diff] [review]
Patch for conversation.xml

*** Original change on bio 1112 attmnt 927 at 2011-10-24 23:26:48 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352669 - Attachment is obsolete: true
Attachment #8352669 - Flags: review?(clokep)
Comment on attachment 8352670 [details] [diff] [review]
Patch for conversation.xml clokep's version

*** Original change on bio 1112 attmnt 928 at 2011-10-24 23:26:48 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352670 - Attachment is obsolete: true
Attached image New screenshot (obsolete) —
*** Original post on bio 1112 as attmnt 932 at 2011-10-24 23:27:00 UTC ***

Screenshot
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1112 as attmnt 933 at 2011-10-24 23:35:00 UTC ***

(Tweaked the color a little.)

Hope this is OK.
Attachment #8352675 - Flags: review?(florian)
Comment on attachment 8352673 [details] [diff] [review]
Patch

*** Original change on bio 1112 attmnt 931 at 2011-10-24 23:35:34 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352673 - Attachment is obsolete: true
Attachment #8352673 - Flags: review?(florian)
Attached image screenshot (obsolete) —
*** Original post on bio 1112 as attmnt 934 at 2011-10-24 23:36:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Comment on attachment 8352674 [details]
New screenshot

*** Original change on bio 1112 attmnt 932 at 2011-10-24 23:36:24 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352674 - Attachment is obsolete: true
Attached image Alternative screenshot (obsolete) —
*** Original post on bio 1112 as attmnt 935 at 2011-10-24 23:52:00 UTC ***

One could go even paler with the inactive color (see screenshot).
This one has rgb(130,130,130) in the CSS.
*** Original post on bio 1112 at 2011-10-25 09:44:57 UTC ***

(In reply to comment #17)
> Created an attachment (id=935) [details]
> Alternative screenshot
> 
> One could go even paler with the inactive color (see screenshot).
> This one has rgb(130,130,130) in the CSS.

Looks really good :)

I didn't meant to imply any connection between context messages and inactive participants when suggesting the filter. My concern was only the bright icons. Now that the style is moved to CSS it should be easy to experiment with it :)
Attached patch Variant of patch (obsolete) — Splinter Review
*** Original post on bio 1112 as attmnt 936 at 2011-10-25 12:33:00 UTC ***

One more, lighter, variant (see following screenshot)
Attached image Variant screenshot (obsolete) —
*** Original post on bio 1112 as attmnt 937 at 2011-10-25 12:34:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
*** Original post on bio 1112 at 2011-10-25 13:28:16 UTC ***

(In reply to comment #19)

>+            // Remove inactive tag if present
>+            if (this.buddies[name].classList.contains("nick-inactive"))
>+              this.buddies[name].classList.remove("nick-inactive");

I think you don't have to check for existence here, just call remove and it should be fine.

>
>+.conv-nicklist .nick-inactive {
>+  opacity: 0.3;
>+  color: rgb(80,80,80) !important;
>+}

I liked the previous values better, this is too faint in my opinion. There's also the problem that the background color of selected inactive nicks is affected by the opacity change (it should have the normal color imo).
In combination with the blue background of selected items on Windows, the grey text of the nicks is hard to read :(

You can try to use :not([selected]) on your rule to apply it only if the nick is both inactive and not selected. This has the advantage that you can easily look up the color of someone by selecting his nick.
Comment on attachment 8352675 [details] [diff] [review]
Patch

*** Original change on bio 1112 attmnt 933 at 2011-10-25 16:01:21 UTC ***

>diff -r -u chrome/instantbird/content/instantbird/conversation.xml chrome/instantbird/content/instantbird/conversation.xml
>--- chrome/instantbird/content/instantbird/conversation.xml	2010-01-01 00:00:00.000000000 +0100
>+++ chrome/instantbird/content/instantbird/conversation.xml	2011-10-25 00:57:59.000000000 +0200
>@@ -218,13 +218,19 @@
>         }
>
>         // Ugly hack... :(
>-        if (!aMsg.system && aMsg.incoming && conv.isChat) {
>+        if (!aMsg.system && conv.isChat) {
>           let name = aMsg.alias || aMsg.who;
>           let color;
>-          if (this.buddies.hasOwnProperty(name))
>+          if (this.buddies.hasOwnProperty(name)) {
>             color = this.buddies[name].color;
>-          else
>+            // Remove inactive tag if present
>+            if (this.buddies[name].classList.contains("nick-inactive"))
>+              this.buddies[name].classList.remove("nick-inactive");

Mic is right in comment 21 saying you can call remove even when the class isn't in the list. (If like me you like to be really sure for that kind of things, the code you will want to look at is http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDOMTokenList.cpp#267).

>+          }
>+          else {
>+            // Buddy no longer in the room
>             color = this._computeColor(name);
>+          }
>           aMsg.color = "color: hsl(" + color + ", 100%, 40%);";
>         }
>
>@@ -890,6 +896,7 @@
>          var color = this._computeColor(name);
>          var style = "color: hsl(" + color + ", 100%, 40%);";
>          item.setAttribute("style", style);
>+         item.classList.add("nick-inactive");
>          item.color = color;
>          this.buddies[name] = item;
>

>diff -r -u chrome/instantbird/skin/classic/instantbird/conversation.css chrome/instantbird/skin/classic/instantbird/conversation.css

I don't think the changes in this file will apply correctly, as it has lots of ifdefs.

>@@ -278,3 +278,7 @@
>   display: none;
> }
>
>+.conv-nicklist .nick-inactive {

Using only .nick-inactive is faster (If you want to understand the details of why, you can read https://developer.mozilla.org/en/Writing_Efficient_CSS - it's a bit long though ;)).

>+  opacity: 0.45;
>+  color: rgb(90,90,90) !important;
>+}

Mic pointed out on IRC that this opacity change may have a surprising effect on the background color of selected rows.

Maybe we should theme separately the label and the icon? If you want to do that, you can use these selectors:
for the icon: .nick-inactive > .listcell-iconic > .listcell-icon
for the label: .nick-inactive > .listcell-iconic > .listcell-label

For the color/opacity, what would you think of reusing the values used in the account manager for disconnected account? That could maybe improve consistency across the application a bit:
http://lxr.instantbird.org/instantbird/source/instantbird/themes/accounts.css#186

That would give for the icon:
opacity: 0.3 (and opacity: 0.7 when the item is selected).
For the text: color: GrayText; (and the default value when selected? That default value is HighlightText when the listbox is focused, and -moz-cellhighlighttext otherwise on Windows[1]/Linux[2] (-moz-DialogText on Mac[3]))

[1] http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/listbox.css#78
[2] http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/gnomestripe/global/listbox.css#78
[3] http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/pinstripe/global/listbox.css#62
Attachment #8352675 - Flags: review?(florian) → review-
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1112 as attmnt 939 at 2011-10-25 22:43:00 UTC ***

(In reply to comment #22)
Thanks for your detailed review!

- I changed the styling to what you suggested. However, I think the inactive text color is too dark here. The difference to the account manager is that there, the contrast is between gray and black; here, it's between gray and a range of colors. I wouldn't go for something as light as the "variant screenshot" above, but lighter than this.

- At clokep's request, I changed the implementation of "inactive" from a CSS class to an attribute.
Attachment #8352681 - Flags: review?(florian)
Comment on attachment 8352675 [details] [diff] [review]
Patch

*** Original change on bio 1112 attmnt 933 at 2011-10-25 22:43:14 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352675 - Attachment is obsolete: true
Comment on attachment 8352678 [details] [diff] [review]
Variant of patch

*** Original change on bio 1112 attmnt 936 at 2011-10-25 22:43:14 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352678 - Attachment is obsolete: true
Attached image Screenshot of 939 (obsolete) —
*** Original post on bio 1112 as attmnt 940 at 2011-10-25 22:46:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1112 as attmnt 941 at 2011-10-25 22:50:00 UTC ***

Fix typo
Attachment #8352683 - Flags: review?(florian)
Comment on attachment 8352681 [details] [diff] [review]
Patch

*** Original change on bio 1112 attmnt 939 at 2011-10-25 22:50:03 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352681 - Attachment is obsolete: true
Attachment #8352681 - Flags: review?(florian)
Attached image Screenshot of 941 with modified colors (obsolete) —
*** Original post on bio 1112 as attmnt 946 at 2011-10-26 19:08:00 UTC ***

I prefer this one - text color is hsl(0, 0%, 60%) (taken from the Bubbles system messages actually) and icon opacity is .45.
*** Original post on bio 1112 as attmnt 947 at 2011-10-26 19:33:00 UTC ***

Alright, this one actually has some inactive people with icons!
color is GrayText and opacity is .45
Comment on attachment 8352683 [details] [diff] [review]
Patch

*** Original change on bio 1112 attmnt 941 at 2011-10-26 22:59:21 UTC ***

>diff -r -u chrome/instantbird/skin/classic/instantbird/conversation.css chrome/instantbird/skin/classic/instantbird/conversation.css
>--- chrome/instantbird/skin/classic/instantbird/conversation.css  2010-01-01 00:00:00.000000000 +0100
>+++ chrome/instantbird/skin/classic/instantbird/conversation.css  2011-10-25 23:29:03.000000000 +0200
>@@ -402,0 +405,25
>+.listitem-iconic[inactive] .listcell-icon {

Insert this before .listcell-{icon,label}:
 > .listcell-iconic >

The selectors will be much faster with this.

>+%ifdef MACOSX
>+.listitem-iconic[inactive][selected] .listcell-label {
>+  color: -moz-DialogText !important;
>+}
>+%else
>+.listitem-iconic[inactive][selected] .listcell-label {
>+  color: -moz-cellhighlighttext !important;
>+}
>+%endif

ifdef only the line that actually changes.

>+.conv-nicklist:focus .listitem-iconic[inactive][selected] .listcell-label {
>+  color: HighlightText !important;
>+}

Insert > between .conv-nicklist:focus and .listitem-iconic

It's not clear at all to me which of the last few variations you talked about in the few previous comments you prefer/would like us to try on nightlies.
Attachment #8352683 - Flags: review?(florian) → review-
Attached patch PatchSplinter Review
*** Original post on bio 1112 as attmnt 952 at 2011-10-28 19:21:00 UTC ***

Hope I haven't forgotten anything...

Font on inactive nicks now normal weight; they become bold when active.
Attachment #8352694 - Flags: review?(florian)
Comment on attachment 8352683 [details] [diff] [review]
Patch

*** Original change on bio 1112 attmnt 941 at 2011-10-28 19:21:55 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352683 - Attachment is obsolete: true
Attached image Screenshot of 952
*** Original post on bio 1112 as attmnt 953 at 2011-10-28 19:22:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Comment on attachment 8352668 [details]
Screenshot

*** Original change on bio 1112 attmnt 926 at 2011-10-28 19:22:47 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352668 - Attachment is obsolete: true
Comment on attachment 8352676 [details]
screenshot

*** Original change on bio 1112 attmnt 934 at 2011-10-28 19:22:47 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352676 - Attachment is obsolete: true
Comment on attachment 8352677 [details]
Alternative screenshot

*** Original change on bio 1112 attmnt 935 at 2011-10-28 19:22:47 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352677 - Attachment is obsolete: true
Comment on attachment 8352679 [details]
Variant screenshot

*** Original change on bio 1112 attmnt 937 at 2011-10-28 19:22:47 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352679 - Attachment is obsolete: true
Comment on attachment 8352682 [details]
Screenshot of 939

*** Original change on bio 1112 attmnt 940 at 2011-10-28 19:22:47 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352682 - Attachment is obsolete: true
Comment on attachment 8352688 [details]
Screenshot of 941 with modified colors

*** Original change on bio 1112 attmnt 946 at 2011-10-28 19:22:47 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352688 - Attachment is obsolete: true
Comment on attachment 8352689 [details]
Screenshot of 942, yet another permutation

*** Original change on bio 1112 attmnt 947 at 2011-10-28 19:22:47 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352689 - Attachment is obsolete: true
Comment on attachment 8352694 [details] [diff] [review]
Patch

*** Original change on bio 1112 attmnt 952 at 2011-11-09 22:45:32 UTC ***

>diff -r -u chrome/instantbird/content/instantbird/conversation.xml chrome/instantbird/content/instantbird/conversation.xml

>+          if (this.buddies.hasOwnProperty(name)) {
>             color = this.buddies[name].color;
>+            this.buddies[name].removeAttribute("inactive");

I'm adding a |let buddy = this.buddies[name];| variable here to avoid a tiny code duplication.



>diff -r -u chrome/instantbird/skin/classic/instantbird/conversation.css chrome/instantbird/skin/classic/instantbird/conversation.css
>--- chrome/instantbird/skin/classic/instantbird/conversation.css  2010-01-01 00:00:00.000000000 +0100
>+++ chrome/instantbird/skin/classic/instantbird/conversation.css  2011-10-27 23:29:03.000000000 +0200
>@@ -402,0 +405,25

If you are hand editing the diff, please ensure you do it correctly. The missing @@ at the end of this line caused the patch to be rejected as malformed by the patch command, I had to fix it before testing your changes.

And by the way, if you are already hand editing your diff, maybe you could also edit the path on the --- lines, so that they match what we have in the hg repository? :)


Anyway, I've just tested this and it looks like a nice improvement. Thanks for working on this, and for your patience! :-)
Attachment #8352694 - Flags: review?(florian) → review+
*** Original post on bio 1112 at 2011-11-10 00:53:00 UTC ***

Thanks for working on this! I'm really excited to try this in day-to-day usage!

Checked in as http://hg.instantbird.org/instantbird/rev/ba4e219b3d55
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
You need to log in before you can comment on or make changes to this bug.