Merge lp:~unity-team/unity/places-fixes-2010-09-22 into lp:unity

Proposed by Neil J. Patel
Status: Merged
Merged at revision: 544
Proposed branch: lp:~unity-team/unity/places-fixes-2010-09-22
Merge into: lp:unity
Diff against target: 493 lines (+155/-26)
13 files modified
targets/mutter/plugin.vala (+4/-4)
unity-private/launcher/application-controller.vala (+9/-4)
unity-private/places/places-controller.vala (+1/-0)
unity-private/places/places-default-renderer-group.vala (+1/-1)
unity-private/places/places-default-renderer.vala (+7/-1)
unity-private/places/places-place-home-renderer.vala (+2/-1)
unity-private/places/places-place-home.vala (+61/-1)
unity-private/places/places-place-search-bar.vala (+28/-2)
unity-private/places/places-place-search-navigation.vala (+9/-0)
unity-private/places/places-place.vala (+3/-3)
unity-private/places/places-view.vala (+28/-8)
unity-private/unity-utils.c (+1/-0)
unity/theme.vala (+1/-1)
To merge this branch: bzr merge lp:~unity-team/unity/places-fixes-2010-09-22
Reviewer Review Type Date Requested Status
Mikkel Kamstrup Erlandsen (community) Approve
Review via email: mp+36332@code.launchpad.net

Description of the change

All bug reports linked

To post a comment you must log in.
Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

CODE REVIEW:
unity-private/places/places-place-home.vala:
 You are using ObjectRegistry to look up the view. This seems like a pretty big hack, abusing the testing infrastructure..?

FUNCTIONAL REVIEW:

Bug #607821: Search Field does not behave as specified when the search fails
 - Fix confirmed

Bug #607829: Clicking a Place icon while viewing the same place in the Dash should return to the Home screen of that place and clear the search
 - Fix confirmed

Bug #612562: URI activation in global view
 - Fix confirmed

Bug #634364: unity crashed with SIGSEGV in unity_places_place_entry_remote_set_active()
 - I could reproduce this now. See http://paste.ubuntu.com/498597/. I can't exactly figure out where to insert the null checks. And frankly I am more concerned why u-p-a fails to come up...

Bug #637136: launchers should act like if the application was not focussed in the place views
 - Fix confirmed

Bug #638402: clicking on a category from CoFs does not directly take you to the desired category
 - Fix confirmed

Bug #638857: dash initial icons states
 - Fix confirmed

Bug #641554: cropping an image in shotwell crashes unity
 - Not tested

Bug #641669: Launcher loads 48 px icons instead of 32 px
 - Fix confirmed

Bug #641894: Clicking on the "Files & Folders" icon while a folder is already displayed in the overlay view causes unity to crash and re-start
 - I get a crash here still. 100% reproducible.

Bug #642669: mutter crashes when closing pop-up dialog
 - I still get very rare crashes when I close the 'bzr gcommit' dialog, in maximized state, when clicking the commit button

Bug #643627: mutter crashed with SIGSEGV in unity_places_place_entry_get_parent()
 -

Bug #644215: "Applications" and "Files & Folders" tooltips are not translatable
 - Fix confirmed

CONCLUSION
 * Can you recheck the fix for bug #641894?
 * Bug #634364 scares me a bit... I don't have a ready fix right now.

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Bugs #634364, #641894, and #643627 have been removed from the fixes-list of this merge proposal and re-opened as agreed with Neil.

The ObjectRegistry hack was is acknowledged as a hack. We'll fix it properly for Natty.

Approved!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'targets/mutter/plugin.vala'
2--- targets/mutter/plugin.vala 2010-09-22 04:36:17 +0000
3+++ targets/mutter/plugin.vala 2010-09-22 16:03:41 +0000
4@@ -844,10 +844,10 @@
5 {
6 window = actor as Mutter.Window;
7
8- if (start_pan_window.get_window_type () != Mutter.MetaCompWindowType.NORMAL &&
9- start_pan_window.get_window_type () != Mutter.MetaCompWindowType.DIALOG &&
10- start_pan_window.get_window_type () != Mutter.MetaCompWindowType.MODAL_DIALOG &&
11- start_pan_window.get_window_type () != Mutter.MetaCompWindowType.UTILITY)
12+ if (window.get_window_type () != Mutter.MetaCompWindowType.NORMAL &&
13+ window.get_window_type () != Mutter.MetaCompWindowType.DIALOG &&
14+ window.get_window_type () != Mutter.MetaCompWindowType.MODAL_DIALOG &&
15+ window.get_window_type () != Mutter.MetaCompWindowType.UTILITY)
16 window = null;
17 }
18
19
20=== modified file 'unity-private/launcher/application-controller.vala'
21--- unity-private/launcher/application-controller.vala 2010-09-02 10:59:56 +0000
22+++ unity-private/launcher/application-controller.vala 2010-09-22 16:03:41 +0000
23@@ -349,14 +349,18 @@
24
25 public override void activate ()
26 {
27- global_shell.hide_unity ();
28-
29 if (app is Bamf.Application)
30 {
31 if (app.is_active ())
32 {
33- Array<uint32> xids = app.get_xids ();
34- global_shell.expose_xids (xids);
35+ /* We only want to do expose if the window was _actually_
36+ * active i.e. the dash wasn't showing.
37+ */
38+ if (global_shell.get_mode () == ShellMode.MINIMIZED)
39+ {
40+ Array<uint32> xids = app.get_xids ();
41+ global_shell.expose_xids (xids);
42+ }
43 }
44 else if (app.is_running ())
45 {
46@@ -384,6 +388,7 @@
47 warning (e.message);
48 }
49 }
50+ global_shell.hide_unity ();
51 }
52
53 private bool on_launch_timeout ()
54
55=== modified file 'unity-private/places/places-controller.vala'
56--- unity-private/places/places-controller.vala 2010-08-18 22:04:31 +0000
57+++ unity-private/places/places-controller.vala 2010-09-22 16:03:41 +0000
58@@ -119,6 +119,7 @@
59 shell.show_unity ();
60 }
61 view.on_entry_view_activated (cont.entry, section_id);
62+ view.search_bar.reset ();
63 }
64 }
65 }
66
67=== modified file 'unity-private/places/places-default-renderer-group.vala'
68--- unity-private/places/places-default-renderer-group.vala 2010-09-22 09:51:13 +0000
69+++ unity-private/places/places-default-renderer-group.vala 2010-09-22 16:03:41 +0000
70@@ -72,7 +72,7 @@
71 private bool last_result_timeout = false;
72
73 public signal void activated (string uri, string mimetype);
74-
75+
76 private GLib.List<Tile?> cleanup_tiles = new GLib.List<Tile?> ();
77 private uint cleanup_operation = 0;
78
79
80=== modified file 'unity-private/places/places-default-renderer.vala'
81--- unity-private/places/places-default-renderer.vala 2010-09-17 17:22:27 +0000
82+++ unity-private/places/places-default-renderer.vala 2010-09-22 16:03:41 +0000
83@@ -17,6 +17,8 @@
84 *
85 */
86
87+using Unity.Testing;
88+
89 namespace Unity.Places
90 {
91 public class DefaultRenderer : LayeredBin, Unity.Place.Renderer
92@@ -40,6 +42,7 @@
93 private Ctk.VBox box;
94 private Dee.Model groups_model;
95 private Dee.Model results_model;
96+ private Places.View place_view;
97
98 private string[] expanded = null;
99
100@@ -285,6 +288,8 @@
101 box.homogeneous = false;
102 scroll.add_actor (box);
103 box.show ();
104+
105+ place_view = ObjectRegistry.get_default ().lookup ("UnityPlacesView")[0] as View;
106 }
107
108 /*
109@@ -342,6 +347,8 @@
110 300,
111 "opacity", groups_box_opacity);
112
113+ place_view.search_bar.search_fail = section_empty.active
114+ || search_empty.active;
115 }
116
117 private void activate_default ()
118@@ -687,4 +694,3 @@
119 }
120 }
121 }
122-
123
124=== modified file 'unity-private/places/places-place-home-renderer.vala'
125--- unity-private/places/places-place-home-renderer.vala 2010-09-14 08:50:05 +0000
126+++ unity-private/places/places-place-home-renderer.vala 2010-09-22 16:03:41 +0000
127@@ -183,7 +183,7 @@
128 }
129 }
130
131- public class HomeButton : Ctk.Button
132+ public class HomeButton : Button
133 {
134 static const int ICON_SIZE = 128;
135
136@@ -201,6 +201,7 @@
137
138 construct
139 {
140+ padding = { 0.0f, 0.0f, 8.0f, 0.0f };
141 get_image ().size = 128;
142 get_image ().filename = icon;
143 get_text ().text = name;
144
145=== modified file 'unity-private/places/places-place-home.vala'
146--- unity-private/places/places-place-home.vala 2010-08-26 11:14:00 +0000
147+++ unity-private/places/places-place-home.vala 2010-09-22 16:03:41 +0000
148@@ -17,6 +17,7 @@
149 *
150 */
151 using Unity;
152+using Unity.Testing;
153
154 namespace Unity.Places
155 {
156@@ -62,6 +63,7 @@
157 public PlaceModel place_model { get; set construct; }
158
159 private Gee.HashMap<PlaceEntry?, uint> entry_group_map;
160+ private Places.View place_view = null;
161
162 public PlaceHomeEntry (Shell shell, PlaceModel model)
163 {
164@@ -131,6 +133,7 @@
165 5, _model.get_string (it, 5),
166 -1);
167
168+ update_search_failed ();
169 });
170
171 entry.global_results_model.row_removed.connect ((it) => {
172@@ -149,11 +152,28 @@
173
174 i = _model.next (i);
175 }
176+
177+ update_search_failed ();
178 });
179 });
180 }
181 }
182
183+ private void update_search_failed ()
184+ {
185+ if (place_view == null)
186+ place_view = ObjectRegistry.get_default ().lookup ("UnityPlacesView")[0] as View;
187+
188+ if (entry_results_model.get_n_rows () > 0)
189+ {
190+ place_view.search_bar.search_fail = false;
191+ }
192+ else
193+ {
194+ place_view.search_bar.search_fail = true;
195+ }
196+ }
197+
198 /*
199 * Public Methods
200 */
201@@ -168,7 +188,10 @@
202
203 if (search == "" || search == null)
204 {
205+ if (place_view == null)
206+ place_view = ObjectRegistry.get_default ().lookup ("UnityPlacesView")[0] as View;
207 entry_renderer_name = "UnityHomeScreen";
208+ place_view.search_bar.search_fail = false;
209 }
210 else
211 {
212@@ -202,6 +225,43 @@
213 {
214
215 }
216+
217+ public unowned PlaceEntry? get_entry_for_uri (string uri)
218+ {
219+ int entry_id = -1;
220+
221+ unowned Dee.ModelIter iter = entry_results_model.get_first_iter ();
222+ while (!entry_results_model.is_last (iter))
223+ {
224+ if (uri == entry_results_model.get_string (iter, 0))
225+ {
226+ entry_id = (int) entry_results_model.get_uint (iter, 2);
227+ break;
228+ }
229+
230+ iter = entry_results_model.next (iter);
231+ }
232+
233+ if (entry_id >= 0)
234+ {
235+ unowned PlaceEntry? ent = null;
236+
237+ foreach (Gee.Map.Entry<PlaceEntry, uint> e in entry_group_map.entries)
238+ {
239+ PlaceEntry? entry = e.key;
240+
241+ if (entry != null && entry_id == e.value)
242+ {
243+ ent = entry;
244+ break;
245+ }
246+ }
247+
248+ if (ent is PlaceEntry)
249+ return ent;
250+ }
251+
252+ return null;
253+ }
254 }
255 }
256-
257
258=== modified file 'unity-private/places/places-place-search-bar.vala'
259--- unity-private/places/places-place-search-bar.vala 2010-09-16 14:35:22 +0000
260+++ unity-private/places/places-place-search-bar.vala 2010-09-22 16:03:41 +0000
261@@ -34,6 +34,19 @@
262 public PlaceSearchSectionsBar sections;
263 public PlaceSearchExtraAction extra_action;
264
265+ private bool _search_fail = false;
266+ public bool search_fail {
267+ get { return _search_fail; }
268+ set {
269+ if (_search_fail != value)
270+ {
271+ _search_fail = value;
272+ bg.search_empty = _search_fail;
273+ bg.update_background ();
274+ }
275+ }
276+ }
277+
278 public PlaceSearchBar ()
279 {
280 Object (orientation:Ctk.Orientation.HORIZONTAL,
281@@ -83,6 +96,7 @@
282
283 public void reset ()
284 {
285+ search_fail = false;
286 entry.reset ();
287 }
288
289@@ -170,9 +184,10 @@
290 active_entry = entry;
291 bg.entry_position = x;
292 sections.set_active_entry (entry);
293- if (section != 0)
294+
295+ if (!(sections.style == SectionStyle.BREADCRUMB && section ==0))
296 {
297- Idle.add (() => {
298+ Timeout.add (0, () => {
299 sections.set_active_section (section);
300 return false;
301 });
302@@ -224,6 +239,8 @@
303 private Clutter.CairoTexture texture;
304 private Ctk.EffectGlow glow;
305
306+ public bool search_empty { get; set; }
307+
308 public PlaceSearchNavigation navigation { get; construct; }
309 public PlaceSearchEntry search_entry { get; construct; }
310
311@@ -257,6 +274,8 @@
312 glow.set_factor (1.0f);
313 glow.set_margin (5);
314 add_effect (glow);
315+
316+ search_empty = false;
317 }
318
319 private override void allocate (Clutter.ActorBox box,
320@@ -407,6 +426,13 @@
321 cr.fill_preserve ();
322
323 cr.set_operator (Cairo.Operator.OVER);
324+
325+ if (search_empty)
326+ {
327+ cr.set_source_rgba (1.0f, 0.0f, 0.0f, 0.3f);
328+ cr.fill_preserve ();
329+ }
330+
331 cr.set_source_rgba (1.0f, 1.0f, 1.0f, 0.6f);
332 cr.stroke ();
333
334
335=== modified file 'unity-private/places/places-place-search-navigation.vala'
336--- unity-private/places/places-place-search-navigation.vala 2010-09-21 15:39:09 +0000
337+++ unity-private/places/places-place-search-navigation.vala 2010-09-22 16:03:41 +0000
338@@ -138,6 +138,9 @@
339
340 private async void refresh_states ()
341 {
342+ if (remote == null)
343+ return;
344+
345 try {
346 var states = yield remote.get_state ();
347 back_sensitive = states[0].sensitive;
348@@ -150,6 +153,9 @@
349
350 private async void go_forward ()
351 {
352+ if (remote == null)
353+ return;
354+
355 try {
356 var states = yield remote.go_forward ();
357 back_sensitive = states[0].sensitive;
358@@ -161,6 +167,9 @@
359
360 private async void go_back ()
361 {
362+ if (remote == null)
363+ return;
364+
365 try {
366 var states = yield remote.go_back ();
367 back_sensitive = states[0].sensitive;
368
369=== modified file 'unity-private/places/places-place.vala'
370--- unity-private/places/places-place.vala 2010-09-16 12:42:01 +0000
371+++ unity-private/places/places-place.vala 2010-09-22 16:03:41 +0000
372@@ -377,9 +377,9 @@
373 return null;
374 }
375
376- name = file.get_string (group, "Name");
377- icon = file.get_string (group, "Icon");
378- desc = file.get_string (group, "Description");
379+ name = file.get_locale_string (group, "Name", null);
380+ icon = file.get_locale_string (group, "Icon", null);
381+ desc = file.get_locale_string (group, "Description", null);
382 } catch (Error e) {
383 warning (@"Cannot load entry '$group': %s", e.message);
384 return null;
385
386=== modified file 'unity-private/places/places-view.vala'
387--- unity-private/places/places-view.vala 2010-09-14 08:50:05 +0000
388+++ unity-private/places/places-view.vala 2010-09-22 16:03:41 +0000
389@@ -18,6 +18,7 @@
390 *
391 */
392 using Unity;
393+using Unity.Testing;
394
395 namespace Unity.Places
396 {
397@@ -42,6 +43,8 @@
398 public View (Shell shell, PlaceModel model)
399 {
400 Object (shell:shell, orientation:Ctk.Orientation.VERTICAL, model:model);
401+
402+ Testing.ObjectRegistry.get_default ().register ("UnityPlacesView", this);
403 }
404
405 construct
406@@ -49,6 +52,8 @@
407 about_to_show ();
408
409 shell.get_stage ().captured_event.connect (on_stage_event_captured);
410+
411+
412 }
413
414 public void about_to_show ()
415@@ -123,6 +128,8 @@
416
417 private void update_views (PlaceEntry entry, uint section_id=0)
418 {
419+ search_bar.set_active_entry_view (entry, 0, section_id);
420+
421 /* Create the correct results view */
422 if (renderer is Clutter.Actor)
423 {
424@@ -148,8 +155,6 @@
425 entry.entry_renderer_hints);
426 renderer.show ();
427 renderer.activated.connect (on_result_activated);
428-
429- search_bar.set_active_entry_view (entry, 0, section_id);
430 }
431
432 private void on_entry_renderer_info_changed (PlaceEntry entry)
433@@ -183,14 +188,29 @@
434
435 private void on_result_activated (string uri, string mimetype)
436 {
437- if (active_entry.parent is Place == false)
438+ ActivationStatus result;
439+
440+ if (active_entry is PlaceHomeEntry)
441+ {
442+ var e = (active_entry as PlaceHomeEntry).get_entry_for_uri (uri);
443+
444+ if (e is PlaceEntry)
445+ result = e.parent.activate (uri, mimetype);
446+ else
447+ {
448+ Place.activate_fallback.begin (uri);
449+ result = ActivationStatus.ACTIVATED_FALLBACK;
450+ }
451+ }
452+ else if (active_entry == null || active_entry.parent == null)
453 {
454 Place.activate_fallback.begin (uri);
455- global_shell.hide_unity ();
456- return;
457- }
458-
459- ActivationStatus result = active_entry.parent.activate (uri, mimetype);
460+ result = ActivationStatus.ACTIVATED_FALLBACK;
461+ }
462+ else
463+ {
464+ result = active_entry.parent.activate (uri, mimetype);
465+ }
466
467 switch (result)
468 {
469
470=== modified file 'unity-private/unity-utils.c'
471--- unity-private/unity-utils.c 2010-08-21 11:26:06 +0000
472+++ unity-private/unity-utils.c 2010-09-22 16:03:41 +0000
473@@ -240,6 +240,7 @@
474 XChangeProperty (display, window, net_wm_strut_partial,
475 XA_CARDINAL, 32, PropModeReplace,
476 (guchar *) &struts, 12);
477+ gdk_flush ();
478 gdk_error_trap_pop ();
479 }
480
481
482=== modified file 'unity/theme.vala'
483--- unity/theme.vala 2010-07-29 11:48:06 +0000
484+++ unity/theme.vala 2010-09-22 16:03:41 +0000
485@@ -66,7 +66,7 @@
486 if (theme.has_icon (icon_name))
487 {
488 var info = theme.lookup_icon (icon_name,
489- 48,
490+ 32,
491 0);
492 if (info != null)
493 {