Unity

Merge lp:~rodrigo-moya/unity/fix-740360 into lp:unity

  1. fix-740360
  2. Merge into trunk
Proposed by Rodrigo Moya
Status: Merged
Approved by: Rodrigo Moya
Approved revision: no longer in the source branch.
Merged at revision: 1039
Proposed branch: lp:~rodrigo-moya/unity/fix-740360
Merge into: lp:unity
Diff against target: 66 lines (+27/-9)
1 file modified
services/panel-service.c (+27/-9)
To merge this branch: bzr merge lp:~rodrigo-moya/unity/fix-740360
Reviewer Review Type Date Requested Status
Rodrigo Moya (community) Approve
Alejandro Piñeiro (community) Approve
Review via email: mp+55319@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Alejandro Piñeiro (apinheiro) wrote :

The code seems ok to me, so I will approve it.

Anyway, I still notice the crash. I will upload a backtrace log to the bug

review: Approve
Revision history for this message
Alejandro Piñeiro (apinheiro) wrote :

Reviewing the backtrace, the crash happens in a line added by this branch, so I think that it would be better to disapprove the merge.

Sorry for the noise

review: Disapprove
Revision history for this message
Rodrigo Moya (rodrigo-moya) wrote :

Can you provide a gdb's backtrace to see where it crashes?

Revision history for this message
Alejandro Piñeiro (apinheiro) wrote :

> Can you provide a gdb's backtrace to see where it crashes?

Yes, it is on the bug itself, I didn't find a way to upload the backtrace on the merge proposal.

BTW: moving from disapprove to need fixing.

review: Needs Fixing
Revision history for this message
Alejandro Piñeiro (apinheiro) wrote :

Ok, this seems to work now, as I was not able to reproduce the bug. Some brief comments:

* Little nitpick on lines 41-44: it seems that the indentation are not homogeneous

* Some things to take into account (for other bug, but writing here to not forget that)
   * When you select the volume indicator it exposes the level
      * But if you move down, when the volume bar is selected no sound is exposed
      * The same when you select the play buttons
      * In general: some of those entries are not exposing
   * On the calendar item: if you move down, and enter the calendar, I didn't find any way to close that window. Moving left-right moves on the calendar, and press Esc doesnt close it. In the same way you can't press alt+f1 or other unity keybinding.
      * In summary you "get trapped" on the calendar

But as I said, those are issues for other bugs. Approving the branch

review: Approve
Revision history for this message
Rodrigo Moya (rodrigo-moya) wrote :

Neil gave his +1 on IRC

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'services/panel-service.c'
--- services/panel-service.c 2011-03-17 21:32:51 +0000
+++ services/panel-service.c 2011-03-29 15:28:47 +0000
@@ -923,13 +923,22 @@
923static gboolean923static gboolean
924should_skip_menu (IndicatorObjectEntry *entry)924should_skip_menu (IndicatorObjectEntry *entry)
925{925{
926 gboolean label_ok;926 gboolean label_ok = FALSE;
927 gboolean image_ok;927 gboolean image_ok = FALSE;
928928
929 label_ok = gtk_widget_get_visible (GTK_WIDGET (entry->label))929 g_return_val_if_fail (entry != NULL, TRUE);
930 && gtk_widget_is_sensitive (GTK_WIDGET (entry->label));930
931 image_ok = gtk_widget_get_visible (GTK_WIDGET (entry->image))931 if (GTK_IS_LABEL (entry->label))
932 && gtk_widget_is_sensitive (GTK_WIDGET (entry->image));932 {
933 label_ok = gtk_widget_get_visible (GTK_WIDGET (entry->label))
934 && gtk_widget_is_sensitive (GTK_WIDGET (entry->label));
935 }
936
937 if (GTK_IS_IMAGE (entry->image))
938 {
939 image_ok = gtk_widget_get_visible (GTK_WIDGET (entry->image))
940 && gtk_widget_is_sensitive (GTK_WIDGET (entry->image));
941 }
933942
934 return !label_ok && !image_ok;943 return !label_ok && !image_ok;
935}944}
@@ -979,13 +988,22 @@
979 }988 }
980989
981 new_entries = indicator_object_get_entries (new_object);990 new_entries = indicator_object_get_entries (new_object);
991 // if the indicator has no entries, move to the next/prev one until we find one with entries
992 while (new_entries == NULL)
993 {
994 gint cur_object_index = g_slist_index (indicators, new_object);
995 gint new_object_index = cur_object_index + (direction == GTK_MENU_DIR_CHILD ? 1 : -1);
996 new_object = g_slist_nth_data (indicators, new_object_index);
997 new_entries = indicator_object_get_entries (new_object);
998 }
999
982 new_entry = g_list_nth_data (new_entries, direction == GTK_MENU_DIR_PARENT ? g_list_length (new_entries) - 1 : 0);1000 new_entry = g_list_nth_data (new_entries, direction == GTK_MENU_DIR_PARENT ? g_list_length (new_entries) - 1 : 0);
9831001
984 g_list_free (entries);1002 g_list_free (entries);
985 g_list_free (new_entries);1003 g_list_free (new_entries);
9861004
987 if (should_skip_menu (new_entry))1005 if (should_skip_menu (new_entry))
988 { 1006 {
989 activate_next_prev_menu (self, new_object, new_entry, direction);1007 activate_next_prev_menu (self, new_object, new_entry, direction);
990 return;1008 return;
991 }1009 }
@@ -997,7 +1015,7 @@
997 g_list_free (entries);1015 g_list_free (entries);
9981016
999 if (should_skip_menu (new_entry))1017 if (should_skip_menu (new_entry))
1000 { 1018 {
1001 activate_next_prev_menu (self, object, new_entry, direction);1019 activate_next_prev_menu (self, object, new_entry, direction);
1002 return;1020 return;
1003 }1021 }