GNOME Bugzilla – Bug 615976
empathy_dispatcher_handle_channels ignores timestamp
Last modified: 2011-08-29 10:12:37 UTC
empathy_dispatcher_handle_channels() ignores the timestamp it's given, which means that if you try to use EnsureChannel() from another process to bring forward an empathy chat window (qv bug 611610), that window will generally get focus-stealing-prevented.
Could you try with this branch please? http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/timestamp-615976
that fixes it. (would be nice if this could end up in 2.30.x too...)
Created attachment 159313 [details] [review] http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/timestamp-615976 libempathy-gtk/empathy-chat.c | 6 +++- libempathy-gtk/empathy-contact-list-view.c | 3 +- libempathy-gtk/empathy-contact-menu.c | 3 +- libempathy-gtk/empathy-new-message-dialog.c | 3 +- libempathy-gtk/empathy-ui-utils.c | 14 ++++++---- libempathy-gtk/empathy-ui-utils.h | 3 +- libempathy/empathy-call-handler.c | 5 ++- libempathy/empathy-call-handler.h | 3 +- libempathy/empathy-dispatch-operation.c | 36 +++++++++++++++++++++++++- libempathy/empathy-dispatch-operation.h | 10 ++++++- libempathy/empathy-dispatcher.c | 36 ++++++++++++++++++--------- libempathy/empathy-dispatcher.h | 19 ++++++++++--- libempathy/empathy-ft-handler.c | 3 +- libempathy/empathy-tp-chat.c | 2 +- src/empathy-call-window.c | 3 +- src/empathy-chat-manager.c | 4 +- src/empathy-chat-window.c | 9 +++--- src/empathy-chat-window.h | 3 +- src/empathy-main-window.c | 5 ++- src/empathy-map-view.c | 4 +- src/empathy-new-chatroom-dialog.c | 3 +- src/empathy-status-icon.c | 2 +- src/empathy.c | 7 +++-- tests/interactive/empetit.c | 3 +- 24 files changed, 133 insertions(+), 56 deletions(-)
Cool thanks. Branch is waiting for review but changes are too intrusive for 2.30.
Review of attachment 159313 [details] [review]: One big ::: libempathy-gtk/empathy-chat.c @@ +263,3 @@ case TP_HANDLE_TYPE_CONTACT: empathy_dispatcher_chat_with_contact_id ( + connection, priv->id, 0, Please use a #define AUTOMATED_ACTION or somesuch, just using 0 is confusing (especially since in timestamp related to the UI it means current time) ::: libempathy-gtk/empathy-ui-utils.c @@ +1469,3 @@ + if (timestamp == 0) { + timestamp = gtk_get_current_event_time (); + if (timestamp == 0) as before s/0/GDK_CURRENT_TIME/ (or after depending on splinters ordering_ :) ::: libempathy-gtk/empathy-ui-utils.h @@ +106,3 @@ gboolean empathy_window_get_is_visible (GtkWindow *window); +void empathy_window_present (GtkWindow *window, + guint32 timestamp); I don't like this API change. It means you had to sprinkle 0 as timestamp in various location and only in one location used the actual value. It's probably better to add a empathy_window_present_with_time similar to gtk_window_present_with_time? Also, instead of 0 use GDK_CURRENT_TIME for code clarity ? ::: src/empathy.c @@ +138,1 @@ Doesn't work this way, you're converting from UserActionTime to ``gdk time'' here implictely. 0 in UserActionTime means it wasn't a user action, 0 in ``gdk time'' means current time, seems there is a mismatch here :).. empathy_chat_window_present_chat should probably take a UserActionTime and not call _window_present if it's not a user action (0 or some #define). This will also allow it to do more cunning thing like only switching tabs if needed :)
Created attachment 159480 [details] [review] http://git.collabora.co.uk/?p=user/sjoerd/empathy.git;a=shortlog;h=refs/heads/timestamp-615976
Review of attachment 159480 [details] [review]: Looks good; 2 smal comments. ::: src/empathy-chat-window.c @@ +109,3 @@ GtkAction *menu_tabs_detach; + + guint32 x_user_action_time; Please add a comment explaining the semantic of this variable. @@ +2312,3 @@ + + /* Don't present or switch tab if the action was earlier then the + last actions X time, accounting for overflow and the first ever Alig is wrong here.
Guillaume: is there any way to get the fixes for the focus-stealing issue split, so that bug 588955 can be fixed in 2.30.x? Or can we find a temporary hack for this series? I don't think it's reasonable to keep this annoying issue in distribution like Ubuntu Lucid for several years...
Maybe. Current plan is to merge this branch to master once it's ready (very soon hopefully) and then consider to backport it to 2.30 after some testing if it didn't introduce any regression.
(In reply to comment #7) > Review of attachment 159480 [details] [review]: > > Looks good; 2 smal comments. > > ::: src/empathy-chat-window.c > @@ +109,3 @@ > GtkAction *menu_tabs_detach; > + > + guint32 x_user_action_time; > > Please add a comment explaining the semantic of this variable. fixed > @@ +2312,3 @@ > + > + /* Don't present or switch tab if the action was earlier then the > + last actions X time, accounting for overflow and the first ever > > Alig is wrong here. Alig isn't a word. I've fixed the alignment though :p
+ /* Don't present or switch tab if the action was earlier then the *than Maybe define a constant rather than using G_MAXINT64? That's easier to understand when reading code.
Fixed in master. Will be in 2.31.1 This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.