Merge lp:~ris/unity-2d/fix-692444 into lp:unity-2d/3.0
- fix-692444
- Merge into natty
Status: | Merged |
---|---|
Merge reported by: | Florian Boucault |
Merged at revision: | not available |
Proposed branch: | lp:~ris/unity-2d/fix-692444 |
Merge into: | lp:unity-2d/3.0 |
Diff against target: |
327 lines (+148/-62) 7 files modified
launcher/UnityApplications/CMakeLists.txt (+2/-0) launcher/UnityApplications/launcherapplication.cpp (+2/-56) launcher/UnityApplications/launcherapplication.h (+0/-2) launcher/UnityApplications/launcherutility.cpp (+58/-0) launcher/UnityApplications/launcherutility.h (+16/-0) launcher/UnityApplications/trash.cpp (+66/-2) launcher/UnityApplications/trash.h (+4/-2) |
To merge this branch: | bzr merge lp:~ris/unity-2d/fix-692444 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Florian Boucault (community) | code | Needs Fixing | |
Review via email: mp+57944@code.launchpad.net |
Commit message
Description of the change
[launcher] Match trash nautilus window with the trash item in the launcher.
Florian Boucault (fboucault) wrote : | # |
Florian Boucault (fboucault) wrote : | # |
I confirm this previous statement: it fails because the window name is translated. But that's not the only reason, a pointer to a QString is created and is then passed to QString::compare which should fail:
54 + QString *windowName = new QString(
55 +
56 + int found = QString:
The following should work better:
QString windowName = QString(
Florian Boucault (fboucault) wrote : | # |
Code in Trash::running() and Trash::show() is mostly duplicated. Code duplication could be avoided by creating a utility method that returns the first BamfWindow corresponding to the trash if it exists:
BamfWindow* firstWindow();
Florian Boucault (fboucault) wrote : | # |
Duplicating LauncherApplica
Robert Sajdok (ris) wrote : | # |
> I confirm this previous statement: it fails because the window name is
> translated. But that's not the only reason, a pointer to a QString is created
> and is then passed to QString::compare which should fail:
>
> 54 + QString *windowName = new QString(
> 55 +
> 56 + int found = QString:
> Qt::CaseSensitive);
>
>
> The following should work better:
>
> QString windowName = QString(
Try this version it should work fine.
Robert Sajdok (ris) wrote : | # |
> Code in Trash::running() and Trash::show() is mostly duplicated. Code
> duplication could be avoided by creating a utility method that returns the
> first BamfWindow corresponding to the trash if it exists:
>
> BamfWindow* firstWindow();
Yes I know, but please note that the method "show ()" does not stop after finding the first window: "Trash". This method "show ()" shows all the windows, whose name is "Trash"
Robert Sajdok (ris) wrote : | # |
> Duplicating LauncherApplica
> something that should be avoided as well. Same for
> LauncherApplica
> utility class/namespace where these 2 would go but for now it is just simpler
> to call LauncherApplica
Done.
Florian Boucault (fboucault) wrote : | # |
Thanks a lot for the fixes Robert. Sorry for having taken so long to reply. The Ubuntu Developer Summit and related meetings took all my attention away.
I added more fixes to your code in lp:~fboucault/unity-2d/trash_singleton
If you can have a look and check that you are happy with that, we should merge it.
Robert Sajdok (ris) wrote : | # |
> Thanks a lot for the fixes Robert. Sorry for having taken so long to reply.
> The Ubuntu Developer Summit and related meetings took all my attention away.
No problem. I understand.
> I added more fixes to your code in lp:~fboucault/unity-2d/trash_singleton
> If you can have a look and check that you are happy with that, we should merge
> it.
I looked at it. It looks good. You can merge :)
Preview Diff
1 | === modified file 'launcher/UnityApplications/CMakeLists.txt' |
2 | --- launcher/UnityApplications/CMakeLists.txt 2011-04-27 16:10:32 +0000 |
3 | +++ launcher/UnityApplications/CMakeLists.txt 2011-05-02 11:09:31 +0000 |
4 | @@ -20,6 +20,7 @@ |
5 | launcherapplicationslistdbus.cpp |
6 | launcherdevice.cpp |
7 | launcherdeviceslist.cpp |
8 | + launcherutility.cpp |
9 | placeentry.cpp |
10 | place.cpp |
11 | launcherplaceslist.cpp |
12 | @@ -38,6 +39,7 @@ |
13 | launcherapplicationslistdbus.h |
14 | launcherdevice.h |
15 | launcherdeviceslist.h |
16 | + launcherutility.h |
17 | placeentry.h |
18 | place.h |
19 | launcherplaceslist.h |
20 | |
21 | === modified file 'launcher/UnityApplications/launcherapplication.cpp' |
22 | --- launcher/UnityApplications/launcherapplication.cpp 2011-04-28 13:09:25 +0000 |
23 | +++ launcher/UnityApplications/launcherapplication.cpp 2011-05-02 11:09:31 +0000 |
24 | @@ -31,13 +31,10 @@ |
25 | via QML’s import statement (`import Unity2d 1.0`), so there is no need to |
26 | set it again here. |
27 | */ |
28 | -extern "C" { |
29 | -#include <libwnck/libwnck.h> |
30 | -#include <libwnck/util.h> |
31 | -} |
32 | |
33 | #include "launcherapplication.h" |
34 | #include "launchermenu.h" |
35 | +#include "launcherutility.h" |
36 | #include "bamf-matcher.h" |
37 | #include "bamf-indicator.h" |
38 | |
39 | @@ -710,58 +707,7 @@ |
40 | wnck_screen_force_update(screen); |
41 | |
42 | WnckWindow* window = wnck_window_get(important->xid()); |
43 | - showWindow(window); |
44 | -} |
45 | - |
46 | -void |
47 | -LauncherApplication::showWindow(WnckWindow* window) |
48 | -{ |
49 | - WnckWorkspace* workspace = wnck_window_get_workspace(window); |
50 | - |
51 | - /* Using X.h's CurrentTime (= 0) */ |
52 | - wnck_workspace_activate(workspace, CurrentTime); |
53 | - |
54 | - /* If the workspace contains a viewport then move the viewport so |
55 | - that the window is visible. |
56 | - Compiz for example uses only one workspace with a desktop larger |
57 | - than the screen size which means that a viewport is used to |
58 | - determine what part of the desktop is visible. |
59 | - |
60 | - Reference: |
61 | - http://standards.freedesktop.org/wm-spec/wm-spec-latest.html#largedesks |
62 | - */ |
63 | - if (wnck_workspace_is_virtual(workspace)) { |
64 | - moveViewportToWindow(window); |
65 | - } |
66 | - |
67 | - /* Using X.h's CurrentTime (= 0) */ |
68 | - wnck_window_activate(window, CurrentTime); |
69 | -} |
70 | - |
71 | -void |
72 | -LauncherApplication::moveViewportToWindow(WnckWindow* window) |
73 | -{ |
74 | - WnckWorkspace* workspace = wnck_window_get_workspace(window); |
75 | - WnckScreen* screen = wnck_window_get_screen(window); |
76 | - |
77 | - int screen_width = wnck_screen_get_width(screen); |
78 | - int screen_height = wnck_screen_get_height(screen); |
79 | - int viewport_x = wnck_workspace_get_viewport_x(workspace); |
80 | - int viewport_y = wnck_workspace_get_viewport_y(workspace); |
81 | - |
82 | - int window_x, window_y, window_width, window_height; |
83 | - wnck_window_get_geometry(window, &window_x, &window_y, |
84 | - &window_width, &window_height); |
85 | - |
86 | - /* Compute the row and column of the "virtual workspace" that contains |
87 | - the window. A "virtual workspace" is a portion of the desktop of the |
88 | - size of the screen. |
89 | - */ |
90 | - int viewport_column = (viewport_x + window_x) / screen_width; |
91 | - int viewport_row = (viewport_y + window_y) / screen_height; |
92 | - |
93 | - wnck_screen_move_viewport(screen, viewport_column * screen_width, |
94 | - viewport_row * screen_height); |
95 | + LauncherUtility::showWindow(window); |
96 | } |
97 | |
98 | void |
99 | |
100 | === modified file 'launcher/UnityApplications/launcherapplication.h' |
101 | --- launcher/UnityApplications/launcherapplication.h 2011-04-28 06:59:39 +0000 |
102 | +++ launcher/UnityApplications/launcherapplication.h 2011-05-02 11:09:31 +0000 |
103 | @@ -108,8 +108,6 @@ |
104 | |
105 | Q_INVOKABLE virtual void createMenuActions(); |
106 | |
107 | - static void showWindow(WnckWindow* window); |
108 | - static void moveViewportToWindow(WnckWindow* window); |
109 | void updateOverlaysState(QMap<QString, QVariant> properties); |
110 | |
111 | Q_SIGNALS: |
112 | |
113 | === added file 'launcher/UnityApplications/launcherutility.cpp' |
114 | --- launcher/UnityApplications/launcherutility.cpp 1970-01-01 00:00:00 +0000 |
115 | +++ launcher/UnityApplications/launcherutility.cpp 2011-05-02 11:09:31 +0000 |
116 | @@ -0,0 +1,58 @@ |
117 | + |
118 | +extern "C" { |
119 | +#include <libsn/sn.h> |
120 | +} |
121 | + |
122 | +#include "launcherutility.h" |
123 | + |
124 | +void |
125 | +LauncherUtility::moveViewportToWindow(WnckWindow* window) |
126 | +{ |
127 | + WnckWorkspace* workspace = wnck_window_get_workspace(window); |
128 | + WnckScreen* screen = wnck_window_get_screen(window); |
129 | + |
130 | + int screen_width = wnck_screen_get_width(screen); |
131 | + int screen_height = wnck_screen_get_height(screen); |
132 | + int viewport_x = wnck_workspace_get_viewport_x(workspace); |
133 | + int viewport_y = wnck_workspace_get_viewport_y(workspace); |
134 | + |
135 | + int window_x, window_y, window_width, window_height; |
136 | + wnck_window_get_geometry(window, &window_x, &window_y, |
137 | + &window_width, &window_height); |
138 | + |
139 | + /* Compute the row and column of the "virtual workspace" that contains |
140 | + the window. A "virtual workspace" is a portion of the desktop of the |
141 | + size of the screen. |
142 | + */ |
143 | + int viewport_column = (viewport_x + window_x) / screen_width; |
144 | + int viewport_row = (viewport_y + window_y) / screen_height; |
145 | + |
146 | + wnck_screen_move_viewport(screen, viewport_column * screen_width, |
147 | + viewport_row * screen_height); |
148 | +} |
149 | + |
150 | +void |
151 | +LauncherUtility::showWindow(WnckWindow* window) |
152 | +{ |
153 | + WnckWorkspace* workspace = wnck_window_get_workspace(window); |
154 | + |
155 | + /* Using X.h's CurrentTime (= 0) */ |
156 | + wnck_workspace_activate(workspace, CurrentTime); |
157 | + |
158 | + /* If the workspace contains a viewport then move the viewport so |
159 | + that the window is visible. |
160 | + Compiz for example uses only one workspace with a desktop larger |
161 | + than the screen size which means that a viewport is used to |
162 | + determine what part of the desktop is visible. |
163 | + |
164 | + Reference: |
165 | + http://standards.freedesktop.org/wm-spec/wm-spec-latest.html#largedesks |
166 | + */ |
167 | + if (wnck_workspace_is_virtual(workspace)) { |
168 | + moveViewportToWindow(window); |
169 | + } |
170 | + |
171 | + /* Using X.h's CurrentTime (= 0) */ |
172 | + wnck_window_activate(window, CurrentTime); |
173 | +} |
174 | + |
175 | |
176 | === added file 'launcher/UnityApplications/launcherutility.h' |
177 | --- launcher/UnityApplications/launcherutility.h 1970-01-01 00:00:00 +0000 |
178 | +++ launcher/UnityApplications/launcherutility.h 2011-05-02 11:09:31 +0000 |
179 | @@ -0,0 +1,16 @@ |
180 | + |
181 | +#ifndef LAUNCHERUTILITY_H |
182 | +#define LAUNCHERUTILITY_H |
183 | + |
184 | +#include <gio/gdesktopappinfo.h> |
185 | +#include <libwnck/libwnck.h> |
186 | + |
187 | +class LauncherUtility { |
188 | + |
189 | +public: |
190 | + |
191 | + static void showWindow(WnckWindow* window); |
192 | + static void moveViewportToWindow(WnckWindow* window); |
193 | +}; |
194 | + |
195 | +#endif // LAUNCHERUTILITY_H |
196 | |
197 | === modified file 'launcher/UnityApplications/trash.cpp' |
198 | --- launcher/UnityApplications/trash.cpp 2011-04-28 13:09:25 +0000 |
199 | +++ launcher/UnityApplications/trash.cpp 2011-05-02 11:09:31 +0000 |
200 | @@ -19,6 +19,11 @@ |
201 | |
202 | #include "trash.h" |
203 | #include "launchermenu.h" |
204 | +#include "launcherutility.h" |
205 | + |
206 | +#include "bamf-application.h" |
207 | +#include "bamf-window.h" |
208 | +#include "bamf-matcher.h" |
209 | |
210 | #include "config.h" |
211 | |
212 | @@ -28,6 +33,7 @@ |
213 | |
214 | #include <QAction> |
215 | |
216 | + |
217 | #define TRASH_URI "trash://" |
218 | |
219 | Trash::Trash() |
220 | @@ -53,7 +59,61 @@ |
221 | bool |
222 | Trash::running() const |
223 | { |
224 | - return false; |
225 | + BamfMatcher& matcher = BamfMatcher::get_default(); |
226 | + QScopedPointer<BamfApplicationList> running_applications(matcher.running_applications()); |
227 | + BamfApplication* bamfApplication; |
228 | + |
229 | + for(int i=0; i<running_applications->size(); i++) { |
230 | + bamfApplication = running_applications->at(i); |
231 | + |
232 | + QScopedPointer<BamfWindowList> windowApplications(bamfApplication->windows()); |
233 | + |
234 | + for (int j=0; j < windowApplications->size(); j++) { |
235 | + |
236 | + BamfWindow *bamfWindow = windowApplications->at(j); |
237 | + |
238 | + WnckWindow* wnckWindow = wnck_window_get(bamfWindow->xid()); |
239 | + QString windowName = QString(wnck_window_get_name(wnckWindow)); |
240 | + |
241 | + int found = QString::compare(u2dTr("Trash", "nautilus"), windowName, Qt::CaseSensitive); |
242 | + |
243 | + if (found == 0) { |
244 | + |
245 | + return true; |
246 | + } |
247 | + } |
248 | + } |
249 | + |
250 | + return false; |
251 | +} |
252 | + |
253 | +void |
254 | +Trash::show() |
255 | +{ |
256 | + BamfMatcher& matcher = BamfMatcher::get_default(); |
257 | + QScopedPointer<BamfApplicationList> running_applications(matcher.running_applications()); |
258 | + BamfApplication* bamfApplication; |
259 | + |
260 | + for(int i=0; i<running_applications->size(); i++) { |
261 | + bamfApplication = running_applications->at(i); |
262 | + |
263 | + QScopedPointer<BamfWindowList> windowApplications(bamfApplication->windows()); |
264 | + |
265 | + for (int j=0; j < windowApplications->size(); j++) { |
266 | + |
267 | + BamfWindow *bamfWindow = windowApplications->at(j); |
268 | + |
269 | + WnckWindow* wnckWindow = wnck_window_get(bamfWindow->xid()); |
270 | + QString windowName = QString(wnck_window_get_name(wnckWindow)); |
271 | + |
272 | + int found = QString::compare(u2dTr("Trash", "nautilus"), windowName, Qt::CaseSensitive); |
273 | + |
274 | + if (found == 0) { |
275 | + LauncherUtility::showWindow(wnckWindow); |
276 | + } |
277 | + } |
278 | + } |
279 | + |
280 | } |
281 | |
282 | int |
283 | @@ -91,7 +151,11 @@ |
284 | void |
285 | Trash::activate() |
286 | { |
287 | - open(); |
288 | + if (running()) { |
289 | + show(); |
290 | + } else { |
291 | + open(); |
292 | + } |
293 | } |
294 | |
295 | void |
296 | |
297 | === modified file 'launcher/UnityApplications/trash.h' |
298 | --- launcher/UnityApplications/trash.h 2011-03-31 08:12:28 +0000 |
299 | +++ launcher/UnityApplications/trash.h 2011-05-02 11:09:31 +0000 |
300 | @@ -20,9 +20,10 @@ |
301 | #ifndef TRASH_H |
302 | #define TRASH_H |
303 | |
304 | -#include <gio/gio.h> |
305 | +#include <libwnck/libwnck.h> |
306 | |
307 | #include "launcheritem.h" |
308 | +#include "launcherutility.h" |
309 | |
310 | #include <QAbstractListModel> |
311 | #include <QMetaType> |
312 | @@ -58,6 +59,7 @@ |
313 | |
314 | private Q_SLOTS: |
315 | void onEmptyTriggered(); |
316 | + void show(); |
317 | |
318 | private: |
319 | void open() const; |
320 | @@ -65,7 +67,7 @@ |
321 | int count() const; |
322 | |
323 | static void recursive_delete(GFile* dir); |
324 | - |
325 | + |
326 | GFile* m_trash; |
327 | }; |
328 |
Thanks a lot for the work Robert. The technique you are using (similar to the one proposed for Unity 3D) looks pretty good. Unfortunately it does not work on my computer. I suspect this is due to the fact that I am running Ubuntu in French and the window name returned by wnck_window_ get_name might be translated to "Corbeille" instead of "Trash".
I still have to do a deeper code review.