Merge lp:~osomon/unity/4.0-dash-custom-home-screen into lp:unity/4.0

Proposed by Olivier Tilloy
Status: Rejected
Rejected by: Gord Allott
Proposed branch: lp:~osomon/unity/4.0-dash-custom-home-screen
Merge into: lp:unity/4.0
Diff against target: 72 lines (+28/-4)
1 file modified
plugins/unityshell/src/PlacesHomeView.cpp (+28/-4)
To merge this branch: bzr merge lp:~osomon/unity/4.0-dash-custom-home-screen
Reviewer Review Type Date Requested Status
Gord Allott (community) Approve
Review via email: mp+88017@code.launchpad.net

Commit message

Do not rely solely on the executable name to launch an application, this doesn’t work for applications that are launched with parameters.

Use g_app_info_launch(…) instead.

To post a comment you must log in.
Revision history for this message
Gord Allott (gordallott) wrote :

On Tue 10 Jan 2012 08:52:40 GMT, Olivier Tilloy wrote:
> Olivier Tilloy has proposed merging lp:~osomon/unity/4.0-dash-custom-home-screen into lp:unity/4.0.
>
> Requested reviews:
> Unity Team (unity-team)
> Related bugs:
> Bug #913885 in unity (Ubuntu): "Custom dash shortcut to desktop file drops Exec args"
> https://bugs.launchpad.net/ubuntu/+source/unity/+bug/913885
>
> For more details, see:
> https://code.launchpad.net/~osomon/unity/4.0-dash-custom-home-screen/+merge/88017

Looks good to me, desktop files are great ;) approving

 review approve
 status approved

--
Gordon Allott
Canonical Ltd.
27 Floor, Millbank Tower
London SW1P 4QP
www.canonical.com

review: Approve

Unmerged revisions

1723. By Olivier Tilloy

Do not rely solely on the executable name to launch an application, this doesn’t work for applications that are launched with parameters.

Use g_app_info_launch(…) instead.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/unityshell/src/PlacesHomeView.cpp'
2--- plugins/unityshell/src/PlacesHomeView.cpp 2011-12-15 12:15:00 +0000
3+++ plugins/unityshell/src/PlacesHomeView.cpp 2012-01-10 08:51:27 +0000
4@@ -75,7 +75,8 @@
5 _id(0),
6 _place_id(NULL),
7 _place_section(0),
8- _exec(NULL)
9+ _exec(NULL),
10+ _desktop_filename(NULL)
11 {
12 SetDndEnabled(false, false);
13 }
14@@ -84,12 +85,14 @@
15 {
16 g_free(_place_id);
17 g_free(_exec);
18+ g_free(_desktop_filename);
19 }
20
21 int _id;
22 gchar* _place_id;
23 guint32 _place_section;
24 char* _exec;
25+ char* _desktop_filename;
26 };
27
28 PlacesHomeView::PlacesHomeView()
29@@ -344,11 +347,10 @@
30 icon = cicon;
31 g_free(cicon);
32 }
33- gchar* exec = g_strdup(g_app_info_get_executable(G_APP_INFO(info)));
34 markup = g_strdup_printf(temp, name.c_str());
35 shortcut = new Shortcut(icon.c_str(), markup, style->GetHomeTileIconSize());
36 shortcut->_id = TYPE_EXEC;
37- shortcut->_exec = exec;
38+ shortcut->_desktop_filename = g_strdup(g_desktop_app_info_get_filename(info));
39 _layout->AddView(shortcut, 1, nux::eLeft, nux::eFull);
40 shortcut->sigClick.connect(sigc::mem_fun(this, &PlacesHomeView::OnShortcutClicked));
41 g_free(markup);
42@@ -526,7 +528,29 @@
43 {
44 GError* error = NULL;
45
46- if (!g_spawn_command_line_async(shortcut->_exec, &error))
47+ if (shortcut->_desktop_filename != NULL)
48+ {
49+ GDesktopAppInfo* info = g_desktop_app_info_new_from_filename(shortcut->_desktop_filename);
50+ if (G_IS_DESKTOP_APP_INFO(info))
51+ {
52+ if (!g_app_info_launch(G_APP_INFO(info), NULL, NULL, &error))
53+ {
54+ g_warning("%s: Unable to launch %s: %s",
55+ G_STRFUNC,
56+ shortcut->_desktop_filename,
57+ error->message);
58+ g_error_free(error);
59+ }
60+ g_object_unref(info);
61+ }
62+ else
63+ {
64+ g_warning("%s: Unable to build desktop file info from %s",
65+ G_STRFUNC,
66+ shortcut->_desktop_filename);
67+ }
68+ }
69+ else if (!g_spawn_command_line_async(shortcut->_exec, &error))
70 {
71 g_warning("%s: Unable to launch %s: %s",
72 G_STRFUNC,

Subscribers

People subscribed via source and target branches

to all changes: