Merge lp:~slash-m3/unity/fix-bug-730623 into lp:unity

Proposed by Friik on 2011-03-14
Status: Merged
Merged at revision: 1012
Proposed branch: lp:~slash-m3/unity/fix-bug-730623
Merge into: lp:unity
Diff against target: 220 lines (+80/-28) 4 files modified
To merge this branch: bzr merge lp:~slash-m3/unity/fix-bug-730623
Reviewer Review Type Date Requested Status
Gord Allott (community) 2011-03-14 Needs Fixing on 2011-03-22
Jason Smith (community) Needs Information on 2011-03-18
Review via email: mp+53189@code.launchpad.net

Description of the Change

As we don't get notified by GConf about whether or not shotwell is the preferred image viewer (or whether or not it is installed), the check for suitable shortcuts in the dash had to be done every time the dash is opened. I had to create a new UBusMessage to achieve that.
The shortcuts on the HomeView first check if the default application is available. If not (e.g. because the preferred program was just uninstalled), some alternatives are tested. And if no suitable app was found, the shortcut is simply not displayed.

To post a comment you must log in.
Jason Smith (jassmith) wrote :

Why disable the gconf key check. It seems to me that key would also change if the user changed their default. Something we should strive to represent.

review: Needs Information
Friik (slash-m3) wrote :

Yes, it does change when the user changes his or her default. But PlacesHomeView::Refresh() is called everytime the Dash is opened anyway (every time UBUS_DASH_VISIBLE gets triggered) and then also checks the the gconf key. Leaving the gconf key notification enabled is therefore (as far as I can see) pointless when we check the gconf key anyway every time the Dash opens.

Gord Allott (gordallott) wrote :

The gconf key should stay connected, for the case that something changes whilst the dash is open - applications not being removed from the preferred application list on uninstall is a separate bug with packages/gnome that should be fixed and we should be ready to support it.

Apart from that it seems like it makes sense, best of a bad situation

review: Needs Fixing
lp:~slash-m3/unity/fix-bug-730623 updated on 2011-03-23
947. By Friik on 2011-03-23

re-activated gconf key notification

948. By Friik on 2011-03-23

removed pointless PlacesGroup::View::IsViewActive in PlacesHomeView::Refresh

949. By Friik on 2011-03-23

fixed some conflicts

950. By Friik on 2011-03-23

.

951. By Friik on 2011-03-23

resolved conflicts

Preview Diff

1=== modified file 'src/PlacesController.cpp'
2--- src/PlacesController.cpp 2011-03-20 21:55:33 +0000
3+++ src/PlacesController.cpp 2011-03-23 19:03:31 +0000
4@@ -112,7 +112,7 @@
5 {
6 if (_visible)
7 return;
8-
9+
10 _view->AboutToShow ();
11 _window->ShowWindow (true, false);
12 // Raise this window on top of all other BaseWindows
13@@ -149,6 +149,9 @@
14
15 void PlacesController::ToggleShowHide ()
16 {
17+ if (!_visible)
18+ ubus_server_send_message (ubus_server_get_default (), UBUS_DASH_VISIBLE, NULL);
19+
20 _visible ? Hide () : Show ();
21 }
22
23
24=== modified file 'src/PlacesHomeView.cpp'
25--- src/PlacesHomeView.cpp 2011-03-21 07:22:42 +0000
26+++ src/PlacesHomeView.cpp 2011-03-23 19:03:31 +0000
27@@ -43,6 +43,9 @@
28 #include "PlacesSimpleTile.h"
29 #include "PlacesStyle.h"
30
31+#include <string>
32+#include <vector>
33+
34 #define DESKTOP_DIR "/desktop/gnome/applications"
35 #define BROWSER_DIR DESKTOP_DIR"/browser"
36 #define CALENDAR_DIR DESKTOP_DIR"/calendar"
37@@ -128,6 +131,32 @@
38 (GConfClientNotifyFunc)OnKeyChanged,
39 this,
40 NULL, NULL);
41+
42+ UBusServer *ubus = ubus_server_get_default ();
43+ ubus_server_register_interest (ubus, UBUS_DASH_EXTERNAL_ACTIVATION, (UBusCallback)&PlacesHomeView::DashVisible, this);
44+
45+ //In case the GConf key is invalid (e.g. when an app was uninstalled), we
46+ //rely on a fallback "whitelist" mechanism instead of showing nothing at all
47+ _browser_alternatives.push_back("firefox");
48+ _browser_alternatives.push_back("chromium-browser");
49+ _browser_alternatives.push_back("epiphany-browser");
50+ _browser_alternatives.push_back("midori");
51+
52+ _photo_alternatives.push_back("shotwell");
53+ _photo_alternatives.push_back("f-spot");
54+ _photo_alternatives.push_back("gthumb");
55+ _photo_alternatives.push_back("gwenview");
56+ _photo_alternatives.push_back("eog");
57+
58+ _email_alternatives.push_back("evolution");
59+ _email_alternatives.push_back("thunderbird");
60+ _email_alternatives.push_back("claws-mail");
61+ _email_alternatives.push_back("kmail");
62+
63+ _music_alternatives.push_back("banshee-1");
64+ _music_alternatives.push_back("rhythmbox");
65+ _music_alternatives.push_back("totem");
66+ _music_alternatives.push_back("vlc");
67
68 expanded.connect (sigc::mem_fun (this, &PlacesHomeView::Refresh));
69
70@@ -142,6 +171,13 @@
71 }
72
73 void
74+PlacesHomeView::DashVisible (GVariant *data, void *val)
75+{
76+ PlacesHomeView *self = (PlacesHomeView*)val;
77+ self->Refresh ();
78+}
79+
80+void
81 PlacesHomeView::OnKeyChanged (GConfClient *client,
82 guint cnxn_id,
83 GConfEntry *entry,
84@@ -216,21 +252,21 @@
85
86 // Browser
87 markup = gconf_client_get_string (_client, BROWSER_DIR"/exec", NULL);
88- CreateShortcutFromExec (markup, _("Browse the Web"), "firefox");
89+ CreateShortcutFromExec (markup, _("Browse the Web"), _browser_alternatives);
90 g_free (markup);
91
92 // Photos
93 // FIXME: Need to figure out the default
94- CreateShortcutFromExec ("shotwell", _("View Photos"), "shotwell");
95+ CreateShortcutFromExec ("shotwell", _("View Photos"), _photo_alternatives);
96
97 // Email
98 markup = gconf_client_get_string (_client, CALENDAR_DIR"/exec", NULL);
99- CreateShortcutFromExec (markup, _("Check Email"), "evolution");
100+ CreateShortcutFromExec (markup, _("Check Email"), _email_alternatives);
101 g_free (markup);
102
103 // Music
104 markup = gconf_client_get_string (_client, MEDIA_DIR"/exec", NULL);
105- CreateShortcutFromExec (markup, _("Listen to Music"), "banshee-1");
106+ CreateShortcutFromExec (markup, _("Listen to Music"), _music_alternatives);
107 g_free (markup);
108
109 QueueDraw ();
110@@ -241,7 +277,7 @@
111 void
112 PlacesHomeView::CreateShortcutFromExec (const char *exec,
113 const char *name,
114- const char *icon_hint)
115+ std::vector<std::string>& alternatives)
116 {
117 PlacesStyle *style = PlacesStyle::GetDefault ();
118 Shortcut *shortcut = NULL;
119@@ -271,32 +307,39 @@
120 }
121 else
122 {
123- id = g_strdup_printf ("%s.desktop", icon_hint);
124+ id = g_strdup_printf ("%s.desktop", alternatives[0].c_str());
125 }
126
127 info = g_desktop_app_info_new (id);
128- if (G_IS_DESKTOP_APP_INFO (info))
129- {
130- icon = g_icon_to_string (g_app_info_get_icon (G_APP_INFO (info)));
131- real_exec = g_strdup (g_app_info_get_executable (G_APP_INFO (info)));
132-
133- g_object_unref (info);
134- }
135- else
136- {
137- icon = g_strdup (icon_hint);
138- real_exec = g_strdup ("firefox");
139- }
140-
141- shortcut = new Shortcut (icon, markup, style->GetHomeTileIconSize ());
142- shortcut->_id = TYPE_EXEC;
143- shortcut->_exec = real_exec; //shorcut will free
144- _layout->AddView (shortcut, 1, nux::eLeft, nux::eFull);
145- shortcut->sigClick.connect (sigc::mem_fun (this, &PlacesHomeView::OnShortcutClicked));
146-
147+ std::vector<std::string>::iterator iter = alternatives.begin();
148+ while (iter != alternatives.end())
149+ {
150+ if (!G_IS_DESKTOP_APP_INFO (info))
151+ {
152+ id = g_strdup_printf ("%s.desktop", (*iter).c_str());
153+ info = g_desktop_app_info_new (id);
154+ iter++;
155+ }
156+
157+ if (G_IS_DESKTOP_APP_INFO (info))
158+ {
159+ icon = g_icon_to_string (g_app_info_get_icon (G_APP_INFO (info)));
160+ real_exec = g_strdup (g_app_info_get_executable (G_APP_INFO (info)));
161+
162+ shortcut = new Shortcut (icon, markup, style->GetHomeTileIconSize ());
163+ shortcut->_id = TYPE_EXEC;
164+ shortcut->_exec = real_exec;
165+ _layout->AddView (shortcut, 1, nux::eLeft, nux::eFull);
166+ shortcut->sigClick.connect (sigc::mem_fun (this, &PlacesHomeView::OnShortcutClicked));
167+
168+ g_free (icon);
169+
170+ break;
171+ }
172+ }
173+
174 g_free (id);
175 g_free (markup);
176- g_free (icon);
177 }
178
179 void
180
181=== modified file 'src/PlacesHomeView.h'
182--- src/PlacesHomeView.h 2011-02-28 12:58:19 +0000
183+++ src/PlacesHomeView.h 2011-03-23 19:03:31 +0000
184@@ -49,6 +49,7 @@
185 void AddProperties (GVariantBuilder *builder);
186
187 private:
188+ static void DashVisible (GVariant *data, void *val);
189 void OnShortcutClicked (PlacesTile *_tile);
190 static void OnKeyChanged (GConfClient *client,
191 guint cnxn_id,
192@@ -56,11 +57,15 @@
193 PlacesHomeView *self);
194 void CreateShortcutFromExec (const char *exec,
195 const char *name,
196- const char *icon_hint);
197+ std::vector<std::string>& alternatives);
198
199 private:
200 nux::GridHLayout *_layout;
201 GConfClient *_client;
202+ std::vector<std::string> _browser_alternatives;
203+ std::vector<std::string> _photo_alternatives;
204+ std::vector<std::string> _email_alternatives;
205+ std::vector<std::string> _music_alternatives;
206 };
207
208
209
210=== modified file 'src/UBusMessages.h'
211--- src/UBusMessages.h 2011-03-21 13:56:35 +0000
212+++ src/UBusMessages.h 2011-03-23 19:03:31 +0000
213@@ -24,6 +24,7 @@
214
215 #define UBUS_DASH_EXTERNAL_ACTIVATION "DASH_EXTERNAL_ACTIVATION"
216 #define UBUS_HOME_BUTTON_BFB_UPDATE "PANEL_HOME_BUTTON_BFB_UPDATE"
217+#define UBUS_DASH_VISIBLE "DASH_VISIBLE"
218
219 // When other parts of Unity want to close the place view
220 #define UBUS_PLACE_VIEW_CLOSE_REQUEST "PLACE_VIEW_CLOSE_REQUEST"