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 657249 - duplicate chat ui
duplicate chat ui
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: message-tray
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-08-24 14:10 UTC by Matthias Clasen
Modified: 2011-09-19 14:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
a log (70.56 KB, text/plain)
2011-08-24 16:08 UTC, Matthias Clasen
  Details
Silently add chat source in the MessageTray (2.04 KB, patch)
2011-09-15 09:28 UTC, Xavier Claessens
reviewed Details | Review
Silently add chat source in the MessageTray (1.30 KB, patch)
2011-09-15 12:25 UTC, Xavier Claessens
reviewed Details | Review

Description Matthias Clasen 2011-08-24 14:10:24 UTC
When I click on the chat button in gnome-contacts, I get both an empathy window and a gnome-shell 'chat bubble'. I think we only need one of these, not both. 

This is probably some telepathy issue, so cc'ing some telepathy/empathy people.
Comment 1 Guillaume Desmottes 2011-08-24 14:15:42 UTC
Could you please attach dbus-monitor logs?
Comment 2 Matthias Clasen 2011-08-24 16:08:50 UTC
Created attachment 194638 [details]
a log

Here is a log from briefly before I clicked the chat button to until the empathy window and the shell bubble appeared.

empathy was already running.
Comment 3 Guillaume Desmottes 2011-08-25 07:53:07 UTC
That's the same problem when starting a chat from empathy's contact list.

The Shell should create the source when observing outgoing channels but not display them right away to user.
Comment 4 Owen Taylor 2011-08-25 15:01:52 UTC
Seems important, but taking off the ff tracker, since it is apparently just a bug
Comment 5 Xavier Claessens 2011-09-15 08:53:04 UTC
Ok, this is because when I do Main.messageTray.add(source); it briefly display the bottom notification bar, instead of silently add the icon.

Is it possible to create a MessageTray source without any visual effect?
Comment 6 Xavier Claessens 2011-09-15 09:28:27 UTC
Created attachment 196598 [details] [review]
Silently add chat source in the MessageTray

We don't want the tray bar to open/close quickly when adding a chat.
Comment 7 Xavier Claessens 2011-09-15 09:33:33 UTC
Note that there is still a weird issue:

1) Open a chat window from empathy (tray does not open, cool!)
2) close the empathy window
3) re-open chat for that contact => tray opens quickly.

I traced that it shows the tray from onWindowDemandsAttention(). There it also call Main.messageTray.add(). But the empathy window was closed, so why is there that demandsAttention?
Comment 8 Florian Müllner 2011-09-15 10:03:26 UTC
Review of attachment 196598 [details] [review]:

Untested, but looks like it should work.

::: js/ui/messageTray.js
@@ +1465,3 @@
     },
 
+    add: function(source, silent) {

Mmmh, not sure I like this ad-hoc API addition - in general, boolean parameters don't read well ("what does doFoo(false) do?"), and in this particular case, you also rely on the fact that the parameter evaluates to false if not specified.
We already special case chat sources (source.isChat), so maybe just use that?
(Anyway, if Marina is OK with the change, I won't object ;-)

@@ +1496,3 @@
         // not removed fast enough because of the callbacks to avoid the summary popping up.
         // So we just don't add transient sources to this._newSummaryItems .
+        if (!source.isTransient && !silent)

In either case, the comment above will need some adjustments.
Comment 9 Florian Müllner 2011-09-15 10:09:47 UTC
(In reply to comment #7)
> I traced that it shows the tray from onWindowDemandsAttention(). There it also
> call Main.messageTray.add(). But the empathy window was closed, so why is there
> that demandsAttention?

demandsAttention is either set by the application (which I assume is not the case here) or when focus stealing prevention kicks in - the latter has regressed quite a bit in comparison to metacity, so I suspect the error there.
Comment 10 Xavier Claessens 2011-09-15 12:25:39 UTC
Created attachment 196616 [details] [review]
Silently add chat source in the MessageTray

We don't want the tray bar to open/close quickly when adding a chat.
Comment 11 Xavier Claessens 2011-09-15 12:27:18 UTC
Changed the patch, it is much simpler like that, since we already special case chat sources there anyway.
Comment 12 Florian Müllner 2011-09-15 12:41:08 UTC
Review of attachment 196616 [details] [review]:

Yeah, I'd prefer that patch to the previous one. I'd still like to see the comment improved (and while at it, the commit message should mention the reason for the change as well)

::: js/ui/messageTray.js
@@ +1496,3 @@
         // not removed fast enough because of the callbacks to avoid the summary popping up.
+        // So we just don't add transient sources to this._newSummaryItems.
+        // We don't want that to happen for chat sources neither.

That's already obvious from the code, however the reason is not - chat sources are not removed when its notifications are done showing, so the reasoning for transient sources does not apply. (I don't think you need to be as verbose as the existing comment :-)
Comment 13 Xavier Claessens 2011-09-15 12:57:32 UTC
Added that, looks good?

        // We don't want that to happen for chat sources neither, because they
        // can be added when the user starts a chat from Empathy and they are not transient.
        // The notification will popup on incoming message anyway. See bug #657249.
Comment 14 Xavier Claessens 2011-09-19 14:22:39 UTC
pushed