After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 652724 - prevent nautilus "close all" from closing the Desktop window
prevent nautilus "close all" from closing the Desktop window
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: Desktop
0.x.x [obsolete]
Other All
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-06-16 12:43 UTC by Antoine Jacoutot
Modified: 2011-07-25 10:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Prevent nautilus_application_close_all_windows to close the Desktop window (1.30 KB, patch)
2011-06-16 12:43 UTC, Antoine Jacoutot
needs-work Details | Review
turn nautilus_window_close() into a virtual method (3.97 KB, patch)
2011-06-20 15:30 UTC, Antoine Jacoutot
reviewed Details | Review
turn nautilus_window_close() into a virtual method v2 (3.94 KB, patch)
2011-06-20 16:37 UTC, Antoine Jacoutot
committed Details | Review

Description Antoine Jacoutot 2011-06-16 12:43:27 UTC
Created attachment 190037 [details] [review]
Prevent nautilus_application_close_all_windows to close the  Desktop window

Hi.

By default, nautilus_application_close_all_windows() would close all
nautilus windows regardless of their type. If we configured nautilus to
manage the Desktop (e.g. in fallback mode), we want to make sure
NAUTILUS_IS_DESKTOP_WINDOW is not closed.

Thoughts?
Comment 1 Cosimo Cecchi 2011-06-16 16:03:33 UTC
Hi Antoine, yes, this is a bug, thanks for the patch.

I think NautilusApplication and NautilusWindow code already has too many of these special cases, and I would like to try making that code a bit more clean instead. 

1. You can turn nautilus_window_close() into a virtual method on NautilusWindowClass, with a default implementation in NautilusWindow that does what the current method does, and an implementation in NautilusDesktopWindow that doesn't do anything.

2. This clashes a bit with the hide-and-then-destroy dance we're trying to do in _close_all_windows(). I don't think gaining a few ms is worth complicating that code, so you can remove the double loop there, and just call nautilus_window_close() on each window instead.

Does that sound OK to you?
Comment 2 Antoine Jacoutot 2011-06-16 18:19:25 UTC
(In reply to comment #1)
> Does that sound OK to you?

Sure I'll give it a shot. Just give me several days as I'm leaving for the week-end ;-) I'll keep you posted.
Comment 3 Cosimo Cecchi 2011-06-16 22:38:47 UTC
(In reply to comment #2)

> Sure I'll give it a shot. Just give me several days as I'm leaving for the
> week-end ;-) I'll keep you posted.

Yep, no hurries! Have a good weekend :)
Comment 4 Antoine Jacoutot 2011-06-20 15:29:31 UTC
Hi Cosimo.

Thanks again for the offline help.
Hopefully this patch does what you requested. It "works for me" (tm) ;-)
Comment 5 Antoine Jacoutot 2011-06-20 15:30:10 UTC
Created attachment 190283 [details] [review]
turn nautilus_window_close() into a virtual method
Comment 6 Cosimo Cecchi 2011-06-20 15:38:24 UTC
Review of attachment 190283 [details] [review]:

Hi Antoine, the patch looks good! Just a couple of coding style details and we're good to go; feel free to push to git master with these fixed.

::: src/nautilus-window.c
@@ +864,3 @@
 nautilus_window_close (NautilusWindow *window)
 {
+	NAUTILUS_WINDOW_CLASS (G_OBJECT_GET_CLASS(window))->window_close(window);

Coding style: make sure to put spaces before braces here. </nitpick>

::: src/nautilus-window.h
@@ +94,2 @@
         void   (* prompt_for_location) (NautilusWindow *window, const char *initial);
+        void   (* window_close) (NautilusWindow *window);

You don't usually repeat the class name in the virtual method names, so this should be just (* close).
Comment 7 Antoine Jacoutot 2011-06-20 16:37:10 UTC
Created attachment 190288 [details] [review]
turn nautilus_window_close() into a virtual method v2
Comment 8 Antoine Jacoutot 2011-06-20 16:37:32 UTC
Fixed patch.
Regarding the 'window_close' vs 'close' I was just mimicking window_type ;-)
As for "pushing to git master", you're not the first one to tell me to
do it but I have no commit bit so feel free to do it for me.
Thanks, it's been a pleasure working with you on this!
Comment 9 Antoine Jacoutot 2011-07-09 01:13:21 UTC
Hi Cosimo.

Any news about this being committed?
Thanks.
Comment 10 Cosimo Cecchi 2011-07-25 10:35:03 UTC
Thanks again for the patch; I just pushed this to git master now.