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 651138 - telepathyClient: Fix presence information
telepathyClient: Fix presence information
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-05-26 11:11 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2011-07-13 18:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
telepathyClient: Fix presence information (4.67 KB, patch)
2011-05-26 11:11 UTC, Jasper St. Pierre (not reading bugmail)
needs-work Details | Review
telepathyContact: Fix presence information (940 bytes, patch)
2011-06-03 20:07 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2011-05-26 11:11:31 UTC
This is just a big messy patch to fix up presence information.

We don't currently check for presence information at the start of a session,
and the way we set the information for display is quite fragile.

Additionally, since cassidy's ShellTpClient landed, there's been an incorrect
signature for 'presenced-changed', making the code there not work at all.

Right now this patches changes behavior. It will take the presence title
completely: the title of the notification will be "Jasper is away" or
"Jasper is online", not "Jasper".

Sidenote: We depend on the side effect of bug #643513 where updating the
title won't update the summary item itself.
Comment 1 Jasper St. Pierre (not reading bugmail) 2011-05-26 11:11:33 UTC
Created attachment 188653 [details] [review]
telepathyClient: Fix presence information

Presence information was quirky: it wasn't loaded after an initial connection,
and it would be cleared if there was any activity other than another presence
change. Additionally, since cassidy's refactoring, an incorrect signal handler
signature broke it completely.
Comment 2 Guillaume Desmottes 2011-05-31 09:56:42 UTC
Review of attachment 188653 [details] [review]:

Looks good from a TP pov.
Comment 3 Jasper St. Pierre (not reading bugmail) 2011-06-02 03:02:38 UTC
Not going to commit yet... this changes behavior, needs an "OK" from designer perspective.
Comment 4 Dan Winship 2011-06-02 13:14:20 UTC
Comment on attachment 188653 [details] [review]
telepathyClient: Fix presence information

>+        this._message = contact.get_presence_message();

"message" is way too overloaded in this file. Call this "_presenceMessage" or something.

>+        if (!presence) presence = this._presence;

2 lines

>+        let message = this._message;

This isn't really necessary... you can just refer to this._presenceMessage later.

> Not going to commit yet... this changes behavior, needs an "OK" from designer
> perspective.

What are the exact behavior changes?
Comment 5 Jasper St. Pierre (not reading bugmail) 2011-06-02 14:12:56 UTC
> What are the exact behavior changes?

The source title always has presence information, like we had "Jasper is offline" before. Now you see "Jasper is online" and "Jasper is away" and all of that in the notification icon.

Right now we're also depending on the fact the SummaryItem's title doesn't update after the Notification is created. (bug 643513)

I'll work a bit harder on doing a patch like this without any of the behavior changes, and I want to see if I can eventually go the way of MessageTray and just implement a big "updateState" method to chop down all of the intricate and fragile moving parts we've concocted.
Comment 6 Dan Winship 2011-06-03 17:21:24 UTC
(In reply to comment #5)
> > What are the exact behavior changes?
> 
> The source title always has presence information, like we had "Jasper is
> offline" before. Now you see "Jasper is online" and "Jasper is away" and all of
> that in the notification icon.

Why would you want to see "Jasper is online" in the title of a notification when receiving an IM? It should be pretty obvious that they're online at that point...

The title should be just the name, unless we're actually notifying a presence change.
Comment 7 Jasper St. Pierre (not reading bugmail) 2011-06-03 20:07:53 UTC
Created attachment 189179 [details] [review]
telepathyContact: Fix presence information

An incorrect signal handler signature broke presence handling in master.
Comment 8 Jasper St. Pierre (not reading bugmail) 2011-06-03 20:31:55 UTC
This just fixes the signal handler, without anything else.

I guess we can do some design work for what messages should appear where and when.
Comment 9 Guillaume Desmottes 2011-06-06 11:29:23 UTC
Review of attachment 189179 [details] [review]:

++
Comment 10 Jasper St. Pierre (not reading bugmail) 2011-06-06 11:58:15 UTC
Comment on attachment 189179 [details] [review]
telepathyContact: Fix presence information

Attachment 189179 [details] pushed as 8e661c3 - telepathyContact: Fix presence information


Whoops, the commit message says "telepathyContact". Only saw it in this git-bz comment thing after pushing