Merge lp:~cando/unity/fix_692444 into lp:unity

Proposed by Stefano Candori
Status: Rejected
Rejected by: Brandon Schaefer
Proposed branch: lp:~cando/unity/fix_692444
Merge into: lp:unity
Diff against target: 178 lines (+65/-15)
3 files modified
src/LauncherController.cpp (+2/-2)
src/TrashLauncherIcon.cpp (+53/-11)
src/TrashLauncherIcon.h (+10/-2)
To merge this branch: bzr merge lp:~cando/unity/fix_692444
Reviewer Review Type Date Requested Status
Alex Launi (community) Needs Fixing
Mirco Müller (community) Approve
Jason Smith (community) Approve
Review via email: mp+48767@code.launchpad.net

Description of the change

In this branch i've fixed the problem with the multiple click on the trash icon.
Here's how it works:
1)when the user clicks on the trash icon, i connect to the "ViewOpened" signal emitted by the BamfMatcher.
2) In the callback i check if the name of the opened view is equal to the localized version of "Trash": if so i connect to the "Closed" signal of this view and set QUIRK_RUNNING to true.
3)when the window is closed i simply put the QUIRK_RUNNING to false.

It works flawlessy, but i don't like the point 2)...it's just an hack and it works fine with Nautilus but i really don't know if others FileManager nominate their windows with the directory's name they are opening. Finally, i know that's almost impossible that, in the very small time range from the user click on the trashbin and the opening of the window, the user opens another window named "Trash", but i think that there could be a better solution.
Maybe modifying BAMF : we could expose the get_window_pid method from BamfLegacyWindow and then compare this pid to the one obtained from g_spawn_process("trash://).

Anyway, maybe you like this solution....

To post a comment you must log in.
lp:~cando/unity/fix_692444 updated
824. By Stefano Candori on 2011-02-07

Quick Fix:removed unused #include

825. By Stefano Candori on 2011-02-07

Quick Fix:removed unused variable

Mirco Müller (macslow) wrote :

Overall this looks ok. But I agree that point 2.) is a hack. Your suggestion with the process-id sounds better. I'd like to hear form Jason, who knows bamf much better then me, what's his view on this. I'll wait with approving until he had a look over it.

Stefano Candori (cando) wrote :

Awesome Mirco, thanks! Let's wait for Jason...

2011/2/17 Mirco Müller <email address hidden>

> Overall this looks ok. But I agree that point 2.) is a hack. Your
> suggestion with the process-id sounds better. I'd like to hear form Jason,
> who knows bamf much better then me, what's his view on this. I'll wait with
> approving until he had a look over it.
> --
> https://code.launchpad.net/~cando/unity/fix_692444/+merge/48767
> You are the owner of lp:~cando/unity/fix_692444.
>

Jason Smith (jassmith) wrote :

+1 it looks good
I am going to try to improve it in the coming days

review: Approve
Mirco Müller (macslow) :
review: Approve
Jason Smith (jassmith) wrote :

merge with trunk is not so happy on this branch, Im more than happy to update it myself (since there was a 10 day delay since its proposal) but it will take a bit of time.

Stefano Candori (cando) wrote :

Sure, no problem...thanks Jason.

2011/2/17 Jason Smith <email address hidden>

> merge with trunk is not so happy on this branch, Im more than happy to
> update it myself (since there was a 10 day delay since its proposal) but it
> will take a bit of time.
> --
> https://code.launchpad.net/~cando/unity/fix_692444/+merge/48767
> You are the owner of lp:~cando/unity/fix_692444.
>

Alex Launi (alexlauni) wrote :

Stefano, I fixed the conflicts on trunk and was testing the fix, but there are some problems with your solution. This disables the ability to accidentally open the trash a ton of times, but when the trash is open you can't click the trash to focus the window or even open a new instance. It's very frustrating.

review: Needs Fixing
Alex Launi (alexlauni) wrote :

My fixed branch is available at lp:~alexlauni/unity/fix-stefanos-conflicts. I would finish the work off of that! Good luck and thanks for your effort/patience. I'll make sure that your updates don't get lost in the shuffle again.

Omer Akram (om26er) wrote :

Its almost a year old branch, reject it maybe?

Aditya V (kroq-gar78) wrote :

Maybe add a quicklist entry for trash saying "Open in New Window"?

Brandon Schaefer (brandontschaefer) wrote :

This branch is out-dated, and a new isher version is here:

https://code.launchpad.net/~brandontschaefer/unity/one-trash-can

Though it has conflicts.

Unmerged revisions

825. By Stefano Candori on 2011-02-07

Quick Fix:removed unused variable

824. By Stefano Candori on 2011-02-07

Quick Fix:removed unused #include

823. By Stefano Candori on 2011-02-07

Fix #692444

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/LauncherController.cpp'
2--- src/LauncherController.cpp 2011-02-02 23:14:25 +0000
3+++ src/LauncherController.cpp 2011-02-07 10:37:22 +0000
4@@ -47,7 +47,6 @@
5 _device_section->IconAdded.connect (sigc::mem_fun (this, &LauncherController::OnIconAdded));
6
7 InsertExpoAction ();
8- InsertTrash ();
9
10 g_timeout_add (500, (GSourceFunc) &LauncherController::BamfTimerCallback, this);
11
12@@ -231,7 +230,7 @@
13 LauncherController::InsertTrash ()
14 {
15 TrashLauncherIcon *icon;
16- icon = new TrashLauncherIcon (_launcher);
17+ icon = new TrashLauncherIcon (_launcher, _matcher);
18 RegisterIcon (icon);
19 }
20
21@@ -286,6 +285,7 @@
22 LauncherController *self = (LauncherController*) data;
23
24 self->SetupBamf ();
25+ self->InsertTrash ();
26
27 return false;
28 }
29
30=== modified file 'src/TrashLauncherIcon.cpp'
31--- src/TrashLauncherIcon.cpp 2011-01-31 21:00:08 +0000
32+++ src/TrashLauncherIcon.cpp 2011-02-07 10:37:22 +0000
33@@ -27,7 +27,7 @@
34 #include <gio/gio.h>
35 #include <glib/gi18n-lib.h>
36
37-TrashLauncherIcon::TrashLauncherIcon (Launcher* IconManager)
38+TrashLauncherIcon::TrashLauncherIcon (Launcher* IconManager, BamfMatcher *matcher)
39 : SimpleLauncherIcon(IconManager)
40 {
41 SetTooltipText (_("Trash"));
42@@ -35,6 +35,8 @@
43 SetQuirk (QUIRK_VISIBLE, true);
44 SetQuirk (QUIRK_RUNNING, false);
45 SetIconType (TYPE_TRASH);
46+
47+ m_matcher = matcher;
48
49 m_TrashMonitor = g_file_monitor_directory (g_file_new_for_uri("trash:///"),
50 G_FILE_MONITOR_NONE,
51@@ -67,10 +69,30 @@
52 }
53
54 void
55+TrashLauncherIcon::Activate ()
56+{
57+ GError *error = NULL;
58+
59+ if (GetQuirk (QUIRK_STARTING) || GetQuirk (QUIRK_RUNNING))
60+ return;
61+
62+ g_signal_connect(m_matcher,
63+ "view-opened",
64+ G_CALLBACK (&TrashLauncherIcon::OnViewOpened),
65+ this);
66+ SetQuirk (QUIRK_STARTING, true);
67+ g_app_info_launch_default_for_uri ("trash://", NULL, &error);
68+
69+ if (error)
70+ g_error_free (error);
71+
72+}
73+
74+void
75 TrashLauncherIcon::EnsureMenuItemsReady ()
76 {
77 DbusmenuMenuitem *menu_item;
78-
79+
80 /* Empty Trash */
81 if (_menu_items.find ("Empty") == _menu_items.end ())
82 {
83@@ -94,15 +116,7 @@
84 {
85 SimpleLauncherIcon::OnMouseClick (button);
86
87- if (button == 1)
88- {
89- GError *error = NULL;
90-
91- g_spawn_command_line_async ("xdg-open trash://", &error);
92-
93- if (error)
94- g_error_free (error);
95- }
96+ if (button == 1) Activate ();
97 else if (button == 3 && _empty == FALSE)
98 {
99 EnsureMenuItemsReady ();
100@@ -230,6 +244,34 @@
101 }
102 }
103
104+void TrashLauncherIcon::OnViewOpened (BamfMatcher *matcher, BamfView *view, gpointer data)
105+{
106+ TrashLauncherIcon *self = (TrashLauncherIcon*) data;
107+ if (g_strcmp0 (_("Trash"), bamf_view_get_name (BAMF_VIEW(view))) == 0 )
108+ {
109+ self->SetQuirk (QUIRK_RUNNING, true);
110+ self->m_view = view;
111+
112+ g_signal_connect (self->m_view, "closed", (GCallback) &TrashLauncherIcon::OnClosed, self);
113+ /*let's disconnect this signal...we need it no more */
114+ g_signal_handlers_disconnect_by_func (matcher,
115+ (void*) (&TrashLauncherIcon::OnViewOpened),
116+ self);
117+ }
118+
119+}
120+
121+void
122+TrashLauncherIcon::OnClosed (BamfView *view, gpointer data)
123+{
124+ TrashLauncherIcon *self = (TrashLauncherIcon *) data;
125+ self->SetQuirk (QUIRK_RUNNING, false);
126+ /*let's disconnect this signal...we need it no more */
127+ g_signal_handlers_disconnect_by_func (self->m_view,
128+ (void*) (&TrashLauncherIcon::OnClosed),
129+ self);
130+}
131+
132 void
133 TrashLauncherIcon::OnTrashChanged (GFileMonitor *monitor,
134 GFile *file,
135
136=== modified file 'src/TrashLauncherIcon.h'
137--- src/TrashLauncherIcon.h 2011-01-28 10:09:52 +0000
138+++ src/TrashLauncherIcon.h 2011-02-07 10:37:22 +0000
139@@ -21,12 +21,13 @@
140 #define TRASHLAUNCHERICON_H
141
142 #include "SimpleLauncherIcon.h"
143+#include <libbamf/libbamf.h>
144
145 class TrashLauncherIcon : public SimpleLauncherIcon
146 {
147
148 public:
149- TrashLauncherIcon (Launcher *launcher);
150+ TrashLauncherIcon (Launcher *launcher, BamfMatcher *matcher);
151 ~TrashLauncherIcon ();
152
153 virtual nux::Color BackgroundColor ();
154@@ -40,8 +41,12 @@
155 std::map<std::string, DbusmenuMenuitem *> _menu_items;
156 GFileMonitor *m_TrashMonitor;
157 gboolean _empty;
158+
159+ BamfMatcher *m_matcher;
160+ BamfView *m_view;
161
162 void EnsureMenuItemsReady ();
163+ void Activate ();
164
165 static void UpdateTrashIconCb (GObject *source, GAsyncResult *res, gpointer data);
166 static void OnTrashChanged (GFileMonitor *monitor, GFile *file, GFile *other_file,
167@@ -49,7 +54,10 @@
168 static void OnEmptyTrash (DbusmenuMenuitem *item, int time, TrashLauncherIcon *self);
169 static void EmptyTrashAction ();
170 static void RecursiveDelete (GFile *location);
171-
172+
173+ static void OnActivate (DbusmenuMenuitem *item, int time, TrashLauncherIcon *self);
174+ static void OnViewOpened (BamfMatcher *matcher, BamfView *view, gpointer data);
175+ static void OnClosed (BamfView *view, gpointer data);
176 };
177
178 #endif // TRASHLAUNCHERICON_H