After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 443884 - red-line indicating last time I saw the given MUC/IRC channel/etc.
red-line indicating last time I saw the given MUC/IRC channel/etc.
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Chat
unspecified
Other All
: Normal enhancement
: ---
Assigned To: empathy-maint
: 586617 601103 648575 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2007-06-04 11:24 UTC by Matěj Cepl
Modified: 2011-04-27 11:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
redline in ongoing messages (11.46 KB, patch)
2011-04-25 13:47 UTC, Chandni Verma
none Details | Review
Shot in a one-to-one chat (51.46 KB, image/png)
2011-04-25 14:10 UTC, Chandni Verma
  Details
Sticker Style adium theme (32.45 KB, image/png)
2011-04-25 14:18 UTC, Chandni Verma
  Details
Ubuntu adium theme (41.65 KB, image/png)
2011-04-25 14:26 UTC, Chandni Verma
  Details
attaching diff for easier review (13.31 KB, patch)
2011-04-27 10:57 UTC, Guillaume Desmottes
reviewed Details | Review

Description Matěj Cepl 2007-06-04 11:24:27 UTC
Like in xchat-gnome, a red line across your screen to indicate last messages you read.

If it matters (or if it is possible to steal a code) it has been added to xchat-gnome in the release 0.13 (http://xchat-gnome.navi.cx/?p=20).
Comment 1 Guillaume Desmottes 2007-06-04 11:39:40 UTC
This code is in xchat core, not xchat-gnome.
Comment 2 semente 2009-03-12 15:55:52 UTC
Pidgin have a similar feature as plugin[1] and works with private conversations too (even in Jabber conversations). That's a useful feature.

[1] http://plugins.guifications.org/trac/wiki/markerline

Comment 3 Frederic Peters 2009-06-22 10:53:48 UTC
*** Bug 586617 has been marked as a duplicate of this bug. ***
Comment 4 Praveen Thirukonda 2009-06-22 11:12:35 UTC
+1.

it's features like this that will go a long way in improving the overall MUC usability.
Comment 5 Xavier Claessens 2009-11-08 09:50:31 UTC
*** Bug 601103 has been marked as a duplicate of this bug. ***
Comment 6 Jeremy Nickurak 2009-11-09 05:48:12 UTC
This should apply to all chats, not just multiuser chat.

To quote my other bug (marked as dupe of this):

Looking at a large chat history, it's not visually obvious, (except by reading
the dates, which is a distraction from which is a distraction from content)
that messages are old, or read previously.

Under Gossip, I usually used the Conversation:Clear option to clear things I
read, so if i wanted to read them again, I'd go to the log. Messages in the
window were always things I hadn't read yet.

Xchat uses a red marker line to distinguish between the last time you looked at
the window from newer content.

Gajim has a nice feature where the color/text-size of old chats are
dimmed/shrunk, so you can read them, but they're again clearly distinct from
the new content.

Any of these stratagies would make empathy feel more comfortable to use.
Comment 7 Praveen Thirukonda 2009-11-09 09:21:01 UTC
(In reply to comment #6)
> This should apply to all chats, not just multiuser chat.
> 
> To quote my other bug (marked as dupe of this):
> 
> Looking at a large chat history, it's not visually obvious, (except by reading
> the dates, which is a distraction from which is a distraction from content)
> that messages are old, or read previously.
> 
> Under Gossip, I usually used the Conversation:Clear option to clear things I
> read, so if i wanted to read them again, I'd go to the log. Messages in the
> window were always things I hadn't read yet.
> 
> Xchat uses a red marker line to distinguish between the last time you looked at
> the window from newer content.
> 
> Gajim has a nice feature where the color/text-size of old chats are
> dimmed/shrunk, so you can read them, but they're again clearly distinct from
> the new content.
> 
> Any of these stratagies would make empathy feel more comfortable to use.

+1 on having this for single chat too.
when i have even say 3-4 conversations going on at the same time, then switching between the tabs,windows,and other applications often makes me forget where the last conversation was.
Comment 8 Guillaume Desmottes 2009-11-27 16:01:01 UTC
Any recommandation about the best way to drawk such line? xchat-gnome uses gdk_draw_line which is pretty low level...
Comment 9 Xavier Claessens 2009-11-30 08:51:09 UTC
There are 2 cases here: GtkTextView based themes, and WebkikWebView adium themes.
Comment 10 Mike Ruprecht 2010-04-13 01:28:20 UTC
Pidgin also uses gdk_draw_line :/

Also, I peeked at the Adium theme reference and it looks like there's not a definition for theming such an indicator either.. at least from my brief skim of it.
Comment 11 Chandni Verma 2011-04-25 01:46:44 UTC
(In reply to comment #9)
> There are 2 cases here: GtkTextView based themes, and WebkikWebView adium
> themes.

I opened separate bug #648575 for this feature to be implemented in HTML based chats.
Comment 12 Guillaume Desmottes 2011-04-25 07:27:43 UTC
(In reply to comment #11)
> (In reply to comment #9)
> > There are 2 cases here: GtkTextView based themes, and WebkikWebView adium
> > themes.
> 
> I opened separate bug #648575 for this feature to be implemented in HTML based
> chats.

We are considering moving away from the old themes (bug #645921) so I think that's fine if this feature is only implemented in HTML themes.
Comment 13 Guillaume Desmottes 2011-04-25 07:28:09 UTC
*** Bug 648575 has been marked as a duplicate of this bug. ***
Comment 14 Chandni Verma 2011-04-25 13:47:13 UTC
Created attachment 186590 [details] [review]
redline in ongoing messages

(In reply to comment #3)
> Review of attachment 186573 [details] [review]:
> 
> I'm reviewing here because of the reviewing tool but please use bug #443884
> from now.

ok, I attached the same patch you reviewed here too.

my public branch for this is https://www.gitorious.org/glassrose-gnome/empathy/commits/redline-demarcating-read-and-unread-mesages-adium

All errors pointed by Guillaume have been corrected except:

> Did you check if Adium has a built-in system to display unread messages?
> 
not one which I found in the theme structure here: http://trac.adium.im/wiki/CreatingMessageStyles/Tutorial

> 
> @@ +1252,3 @@
>          empathy_contact_get_handle (sender));
> 
> +    if (priv->is_seen &&
> 
> This logic doesn't seem to work here, the line is never displayed.

This works exactly as how the red line in xchat works.
i.e. Whenever the chat is un-focussed and we have a message the line gets updated. It doesn't update when the chat is focussed and there are new messages.
I can additionally see if I can get it updated for the current chat when the full notebook is un-focussed.

> ::: libempathy-gtk/empathy-chat.h
> @@ +97,3 @@
> +void               empathy_chat_set_last_viewed_at (EmpathyChat *chat,
> +                              time_t view_time);
> +time_t             empathy_chat_get_last_viewed_at (EmpathyChat *chat);
> 
> Why is this function public? Actually this function doesn't really buy us
> anything as it's only called in empathy-chat, which could use
> priv->last_viewed_at directly.

empathy_chat_set_last_viewed_at() is used in empathy-chat-window.c and 
empathy_chat_get_last_viewed_at() can be used in empathy-log-window.c if we are willing to have the red-line shown in backlog conversations too (if yes then how can we save the last time a chat was un-focussed? logging it too?)

> ::: src/empathy-chat-window.c
> @@ +1540,3 @@
> +    if (priv->current_chat != NULL)
> +            empathy_chat_set_last_viewed_at (priv->current_chat,
> +                empathy_time_get_current ());
> 
> This look wrong. Why not do this directly in empathy_chat_messages_read()?

This is intentionally done and works fine for me!
Comment 15 Xavier Claessens 2011-04-25 13:58:18 UTC
Just a small comment: when redline is drawn, it should force split message bubble, even if next message is from the same contact. Empathy already has code to force the split if next message comes after more than 5min for example. This is to avoid redline inside the bubble.
Comment 16 Chandni Verma 2011-04-25 14:10:16 UTC
Created attachment 186595 [details]
Shot in a one-to-one chat
Comment 17 Chandni Verma 2011-04-25 14:18:05 UTC
Created attachment 186597 [details]
Sticker Style adium theme
Comment 18 Chandni Verma 2011-04-25 14:26:13 UTC
Created attachment 186600 [details]
Ubuntu adium theme
Comment 19 Xavier Claessens 2011-04-25 14:35:06 UTC
Suggestion: create a demarcator.html, check if it is shipped by the theme, and fallback to one shipped by empathy that display that red line. That html+JS function could be proposed to adium theme people for standardization.

Also: be careful in case the theme ships its own Template.html, because in that case Empathy won't load the JS you wrote for the fallback Template.html. We should check if it's possible to load that JS function directly using Webkit API instead of shipping it in the Template.html, otherwise some themes won't remove the red line and you'll end with lines everywhere.
Comment 20 Guillaume Desmottes 2011-04-25 15:14:25 UTC
It seems that Adium does support this feature. See the 'focus' and
'firstFocus' message classes on [0].

I guess that's what is used by the Stockholm theme [1] do display the small
blue marker on [2].

I quickly looked at Adium's source code and they seem to use Webkit's API to
manipulate the DOM and directly remove these classes when the window is
focused.

[0] http://trac.adium.im/wiki/CreatingMessageStyles
[1] http://www.adiumxtras.com/index.php?a=xtras&xtra_id=1760
[2] http://willthompson.co.uk/misc/adium-unread-messages.png
Comment 21 Guillaume Desmottes 2011-04-25 15:35:18 UTC
(In reply to comment #20)
> [2] http://willthompson.co.uk/misc/adium-unread-messages.png

<wjt> cassidy: there are three different styles of message in that screenshot
<wjt> cassidy: the blue markers at the end of the line mean “unread message in this conversation”
<wjt> cassidy: the slightly greyed out messages at the tops of each windows mean “messages from the logs”
<wjt> cassidy: the stuff in between (full colour, no blue marker) means “read message in this conversation”
Comment 22 Xavier Claessens 2011-04-26 09:58:31 UTC
I've experimenting the adium way for doing this[1]. Known issues:

a) Themes on adiumxtra are outdated and does not support those new adium features.
b) Adium tarball[2] ships some themes that support focus feature, but webkit does not render them correctly for me. I tested Stockholm theme from the adium tarball, and the avatar box is not displayed correctly on the left of the message. But that works enough to test focus at least.
c) My code has a FIXME, code to unmark messages is missing there.


[1] http://cgit.collabora.co.uk/git/user/xclaesse/empathy.git/log/?h=focus
[2] http://download.adium.im/adium-1.4.1.tgz
Comment 23 Xavier Claessens 2011-04-26 14:57:46 UTC
Updated my branch with b and c fixed. However it has another issue: only one op 2 message gets updated to remove focus mark. This is because webkit_dom_html_element_set_class_name() internally remove the node from nodes list and append a new node. So the code iterates on a list and the list that gets modified.

This looks like a webkit bug... API has not doc... need the advice of webkit devs to know what's the proper way.

Note that adium itself does exactly the same loop, so maybe it is binding issue.
Comment 24 Xavier Claessens 2011-04-26 15:30:26 UTC
Ok, turns out that using webkit_dom_document_query_selector_all() fix the above issue. I've reported that issue against webkit though: https://bugs.webkit.org/show_bug.cgi?id=59461

So my branch is ready for review: http://cgit.collabora.co.uk/git/user/xclaesse/empathy.git/log/?h=focus
Comment 25 Xavier Claessens 2011-04-27 07:21:41 UTC
Hm, actually it doesn't work properly when switching tabs. Seems that emits focus in-out-in whens selecting a tab which makes all mark being removed...
Comment 26 Xavier Claessens 2011-04-27 08:41:50 UTC
fixed, branch updates, should be really ready for review now :)
Comment 27 Guillaume Desmottes 2011-04-27 10:57:29 UTC
Created attachment 186725 [details] [review]
attaching diff for easier review
Comment 28 Guillaume Desmottes 2011-04-27 11:13:28 UTC
Review of attachment 186725 [details] [review]:

::: libempathy-gtk/empathy-chat.c
@@ +2719,3 @@
 	/* Add input GtkTextView */
 	chat->input_text_view = empathy_input_text_view_new ();
+	g_signal_connect (chat->input_text_view, "notify::has-focus",

Any reason to use the GtkEntry rather than the EmpathyChat widget itself?

::: libempathy-gtk/empathy-theme-adium.c
@@ +320,3 @@
 }
 
+static gboolean

A small description of this function would be nice.

@@ +321,3 @@
 
+static gboolean
+theme_adium_match_with_format (const gchar **str, const gchar *match,

one arg per line

@@ +514,3 @@
+	/* Remove focus and firstFocus class in all nodes having it */
+	dom = webkit_web_view_get_dom_document (WEBKIT_WEB_VIEW (theme));
+	WebKitDOMNodeList *nodes;

this function can't return NULL?

@@ +637,3 @@
+		if (!priv->has_unread_message) {
+			/* This is the first message we receive since we lost
+			 * focus; remove redline from previous position. */

Which red line? :)
Comment 29 Xavier Claessens 2011-04-27 11:32:01 UTC
Review of attachment 186725 [details] [review]:

::: libempathy-gtk/empathy-chat.c
@@ +2719,3 @@
 	/* Add input GtkTextView */
 	chat->input_text_view = empathy_input_text_view_new ();
+	g_signal_connect (chat->input_text_view, "notify::has-focus",

because only entries can focus, afaik.
Comment 30 Xavier Claessens 2011-04-27 11:52:48 UTC
Comments fixed, branch merged, bug solved \o/