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 561136 - Undo Close Tabs
Undo Close Tabs
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: Tabs
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
: 750227 757067 (view as bug list)
Depends on:
Blocks: 771089
 
 
Reported: 2008-11-17 01:16 UTC by Ben Grimes
Modified: 2017-05-31 13:02 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
Adds ability to reopen a closed tab with Ctrl+Shift+T (4.40 KB, patch)
2009-10-11 20:45 UTC, Marcus Carlson
reviewed Details | Review
add "restore tab" action (5.50 KB, patch)
2017-03-09 22:16 UTC, Alexandru Pandelea
none Details | Review
Remove Shift+Control+T accelerator for open in new tab (1.98 KB, patch)
2017-03-27 20:02 UTC, Alexandru Pandelea
accepted-commit_now Details | Review
add "restore tab" action (4.91 KB, patch)
2017-03-27 20:02 UTC, Alexandru Pandelea
none Details | Review
add "restore tab" action (5.66 KB, patch)
2017-03-28 08:54 UTC, Alexandru Pandelea
none Details | Review
add "restore tab" action (9.25 KB, patch)
2017-04-12 15:45 UTC, Alexandru Pandelea
none Details | Review
add "restore tab" action (10.41 KB, patch)
2017-05-09 12:13 UTC, Alexandru Pandelea
none Details | Review
add "restore tab" action (10.98 KB, patch)
2017-05-09 12:51 UTC, Alexandru Pandelea
none Details | Review
add "restore tab" action (10.40 KB, patch)
2017-05-30 19:18 UTC, Alexandru Pandelea
none Details | Review
add "restore tab" action (10.31 KB, patch)
2017-05-31 10:34 UTC, Alexandru Pandelea
none Details | Review
nautilus-window: add wrapper for creating slot (3.08 KB, patch)
2017-05-31 10:35 UTC, Alexandru Pandelea
none Details | Review
nautilus-window: add wrapper for creating slot (3.08 KB, patch)
2017-05-31 12:06 UTC, Alexandru Pandelea
committed Details | Review
add "restore tab" action (11.21 KB, patch)
2017-05-31 12:28 UTC, Alexandru Pandelea
committed Details | Review

Description Ben Grimes 2008-11-17 01:16:29 UTC
I like the tabbed file browsing in Nautilus, it makes life easier. I would love the tabbed browsing if there were an "Undo Close Tab". As a bonus, I would also love to middle click the tab to close it.
Comment 1 EvilGnome 2009-01-21 03:11:21 UTC
I second this request. This would provide an intuitive model for those of us who are accustomed to other tabbed browsers such as Mozilla Firefox. Ideally the keyboard shortcuts would match, too, so I recommend Shift+Ctrl+T for Undo Close Tab.
Comment 2 Marcus Carlson 2009-10-11 20:45:19 UTC
Created attachment 145255 [details] [review]
Adds ability to reopen a closed tab with Ctrl+Shift+T
Comment 3 Marcus Carlson 2009-10-11 20:46:50 UTC
I'm not sure I've done this correctly but it seems to be working. Also, should we have a menu item for this or just a keyboard shortcut? 
Please try it out and comment.
Comment 4 A. Walton 2009-10-11 22:53:45 UTC
Comment on attachment 145255 [details] [review]
Adds ability to reopen a closed tab with Ctrl+Shift+T

Couple of minor fixes: you're leaking the list of closed locations, and g_file_get_uri_scheme() then strcmp() then g_free() is wasteful, just use g_file_has_uri_scheme().

Would probably make sense not to even store the closed locations if tabs are off (not familiar enough with the slot code to know if this already happens) and to sanity check the length of that list to some number of items so we're not blowing up in RAM usage for windows that stay open for a very long time.
Comment 5 Marcus Carlson 2010-08-04 20:38:54 UTC
Hmm.. this got more complex now with panes. I think we need to keep a list of list of locations. Right?
Comment 6 Holger Berndt 2010-08-25 16:04:54 UTC
(In reply to comment #5)
> Hmm.. this got more complex now with panes. I think we need to keep a list of
> list of locations. Right?

No, I don't think so. Just like some other formerly window specific items (the list of slots, the pointer to the active slot, ...), it would move from NautilusWindow to NautilusWindowPane. So tab history would be something pane specific, not window specific.
Comment 7 Cosimo Cecchi 2015-10-24 16:35:50 UTC
*** Bug 757067 has been marked as a duplicate of this bug. ***
Comment 8 Electric Prism 2016-11-21 08:40:53 UTC
What is the current status of this? I would also like to have a undo close tab as sometimes I accidentally close the wrong work tab and have it ingrained in my Ctrl + Shift + T reopens closed tabs as has been defacto in Google Chrome, Firefox & other browsers for years.
Comment 9 Carlos Soriano 2016-11-21 09:33:18 UTC
(In reply to Electric Prism from comment #8)
> What is the current status of this? I would also like to have a undo close
> tab as sometimes I accidentally close the wrong work tab and have it
> ingrained in my Ctrl + Shift + T reopens closed tabs as has been defacto in
> Google Chrome, Firefox & other browsers for years.

Status is what you can see here, nothing happened. But this looks like an interesting addition, hope someone can make the time to post a patch.
Marking for 3.24 for now.
Comment 10 Alexandru Pandelea 2017-03-09 22:16:47 UTC
Created attachment 347584 [details] [review]
add "restore tab" action

Adds option to reopen closed tabs with Ctrl+Shift+T. This is enabled
when there are no selected folders in the active slot, as in this case
this shortcut opens the selected folders in new tabs.
Comment 11 Carlos Soriano 2017-03-27 17:12:55 UTC
Review of attachment 347584 [details] [review]:

Heya, code looks mostly good! But, the shrotcut is already taken... we should find a new one

::: src/nautilus-files-view.c
@@ +7516,3 @@
     g_simple_action_set_enabled (G_SIMPLE_ACTION (action), item_opens_in_view);
+    action = g_action_map_lookup_action (G_ACTION_MAP (window),
+                                         "restore-tab");

why do you need to deactivate it? You are trying to avoid the shortcut collision? That's not the way... this is really confusing.

::: src/nautilus-window.c
@@ +1284,3 @@
+    NautilusWindow *window = NAUTILUS_WINDOW (user_data);
+    NautilusWindowOpenFlags flags;
+    GFile *location;

auto

@@ +1298,3 @@
+    location = window->priv->closed_locations->data;
+
+    {

Why this? We can actually store the "search location", similar to what we do with history in the back/forward buttons in the toolbar.

@@ +2190,3 @@
     nautilus_application_set_accelerator (app, "win.prompt-home-location", "asciitilde");
     nautilus_application_set_accelerator (app, "win.view-menu", "F10");
+    nautilus_application_set_accelerator (app, "win.restore-tab", "<shift><control>t");

this shortcut is already taken :( I vote to change the shortcut in files-view so we have similar shortcut as chrome for this, but we need a new one for opening a file in a new tab.
Comment 12 Alexandru Pandelea 2017-03-27 20:02:17 UTC
Created attachment 348831 [details] [review]
Remove Shift+Control+T accelerator for open in new tab

Opening in a new tab can be done with Shift+Return.

This shortcut is removed so that it can be used for reopening closed
tabs.
Comment 13 Alexandru Pandelea 2017-03-27 20:02:41 UTC
Created attachment 348832 [details] [review]
add "restore tab" action

Adds option to reopen closed tabs with Ctrl+Shift+T.
Comment 14 Carlos Soriano 2017-03-27 20:11:44 UTC
Review of attachment 348831 [details] [review]:

+1
Comment 15 Carlos Soriano 2017-03-27 20:36:36 UTC
Review of attachment 348832 [details] [review]:

as mentioned in IRC search tabs doesn't work, and I think we can do something similar as window-slot where it works (hope that doesn't make the code much ugly though).
Comment 16 Alexandru Pandelea 2017-03-28 08:54:09 UTC
Created attachment 348869 [details] [review]
add "restore tab" action

Adds option to reopen closed tabs with Ctrl+Shift+T.
Comment 17 Carlos Soriano 2017-03-29 10:14:45 UTC
Review of attachment 348869 [details] [review]:

Now! This is the kind of approach I would like to see more... not trying to workaround but fixing the underlying issue, nice!

There are just some details that still doesn't work, but now the logic is correct.
Search -> close tab -> restore tab -> click one of the directories. The view uses list-view. I saw this in the code, tried it, and effectively that happens. Keep in mind that now in your code you don't save the "previous_view_if_search", this is fine, but what you do afterwards for this case ^ it misbehaves.

Also, the comments should be updated to reflect this new use case.

And the commit message, something happened with it that now is wrong when it was right before.
Comment 18 Ernestas Kulik 2017-04-07 13:47:50 UTC
*** Bug 750227 has been marked as a duplicate of this bug. ***
Comment 19 Alexandru Pandelea 2017-04-12 15:45:58 UTC
Created attachment 349739 [details] [review]
add "restore tab" action

Adds option to reopen closed tabs with Ctrl+Shift+T.

In order to do so, keep a list with data needed to restore closed
tabs. So, this list keeps the location bookmark, the view id before
search, which is needed in case the closed tab was a search and
the back/forward history.
Comment 20 Carlos Soriano 2017-04-20 18:03:25 UTC
Review of attachment 349739 [details] [review]:

Looking good, and it works great, nice solution with the history handling.

::: src/nautilus-window-slot.h
@@ +111,3 @@
                                                 GFile              *location);
 
+void nautilus_window_slot_set_history (NautilusWindowSlot *self,

maybe we should have a single function for just getting a struct with all the history data, instead of this specific functions.
It's already a little ugly we have to peek on the private data of the slot, so if we can isolate that and not really peek into it individually would be better.
So something like:
nautilus_window_slot_get_restore_data.
nautilus_window_slot_restore_from_data.

::: src/nautilus-window.c
@@ +1308,3 @@
+    data = window->priv->tab_data_list->data;
+
+    if (!window->priv->tab_data_list)

cannot we use just a GFile? Why using the bookmark?

@@ +1317,3 @@
+    nautilus_window_slot_set_view_before_search (slot, data->view_before_search);
+
+    flags = g_settings_get_enum (nautilus_preferences, NAUTILUS_PREFERENCES_NEW_TAB_POSITION);

this already initializes the slot. Couldn't we omite the previous call to window_iniaitlize_slot and the creation of the slot and instead just open a new locaiton in a new tab with this call? then we can set the history and the set_view_before_search.

@@ +1320,3 @@
+
+    window->priv->tab_data_list = g_list_delete_link (window->priv->tab_data_list,
+

missaligment

@@ +1323,3 @@
+
+    g_object_unref (data->location_bookmark);
+    flags |= NAUTILUS_WINDOW_OPEN_FLAG_DONT_MAKE_ACTIVE;

do a method free_restore_tab_data and do these things in there. Also you are missing to unref the back_list and forward_list (and ref them inside set_history). The ownership should be clear.

@@ +1324,3 @@
+    g_object_unref (data->location_bookmark);
+    g_free (data);
+    flags |= NAUTILUS_WINDOW_OPEN_FLAG_DONT_MAKE_ACTIVE;

spurious line

@@ +1515,3 @@
 {
     NautilusWindowSlot *next_slot;
+    GList *back_list, * forward_list;

one per line pls

@@ +1539,3 @@
+    data->view_before_search = nautilus_window_slot_get_view_before_search (slot);
+
+    data->back_list = back_list;

can we use a gqueue? We are trying to use it more and more so we can push from both ends and query the lenght in O(1)

@@ +2423,3 @@
 
+    for (l = window->priv->tab_data_list; l != NULL; l = l->next)
+    {

ideally this will just be a g_list_foreach (list, free_restore_data, NULL)
Comment 21 Alexandru Pandelea 2017-05-09 12:13:00 UTC
Created attachment 351426 [details] [review]
add "restore tab" action

Adds option to reopen closed tabs with Ctrl+Shift+T.

In order to do so, keep a list with data needed to restore closed
tabs. So, this list keeps the location bookmark, the view id before
search, which is needed in case the closed tab was a search and
the back/forward history.
Comment 22 Alexandru Pandelea 2017-05-09 12:51:02 UTC
Created attachment 351431 [details] [review]
add "restore tab" action

Adds option to reopen closed tabs with Ctrl+Shift+T.

In order to do so, keep a list with data needed to restore closed
tabs. So, this list keeps the location bookmark, the view id before
search, which is needed in case the closed tab was a search and
the back/forward history.
Comment 23 Carlos Soriano 2017-05-30 15:26:48 UTC
Review of attachment 351431 [details] [review]:

Looking better and better, still two details to take care about.

::: src/nautilus-window-slot.c
@@ +124,3 @@
     gint view_mode_before_search;
+
+    gboolean restored_slot;

Oh this is pretty ugly, and I believe not necesary.
I guess you want to avoid the forward list to be cleaned, but doesn't NAUTILUS_LOCATION_CHANGE_RELOAD preciselly exists for this situation, when you are kinda "reloading" rather than opening a new location?
The nautilus_window_slot_restore_from_data should do the work already.

@@ +156,3 @@
                                     gpointer              user_data);
 
+void nautilus_window_slot_restore_from_data (NautilusWindowSlot *self,

new line

@@ +171,3 @@
+}
+
+    priv->back_list = g_list_copy_deep (data->back_list, (GCopyFunc) g_object_ref, NULL);

new line

::: src/nautilus-window.c
@@ +1376,3 @@
+
+    slot = nautilus_window_create_slot (window, location);
+    if (g_queue_get_length (priv->tab_data_queue) == 0)

is this still necesary? This seems to be handled in nautilus_window_open_location_full if NAUTILUS_WINDOW_OPEN_FLAG_NEW_TAB is there.
This should go away in one way or another...

@@ +1379,3 @@
+
+    nautilus_window_slot_open_location_full (slot, location, flags, NULL);
+        return;

detail, but this looks better withouth a new line here, as in opening the location and restoring are close related.

@@ +1382,3 @@
+    nautilus_window_slot_restore_from_data (slot, data);
+
+    }

use free_restore_tab_data

@@ +2531,3 @@
 static void
+free_restore_tab_data (gpointer data,
+                          gpointer user_data)

wrong identation
Comment 24 Alexandru Pandelea 2017-05-30 19:18:31 UTC
Created attachment 352900 [details] [review]
add "restore tab" action

Adds option to reopen closed tabs with Ctrl+Shift+T.

In order to do so, keep a list with data needed to restore closed
tabs. So, this list keeps the location bookmark, the view id before
search, which is needed in case the closed tab was a search and
the back/forward history.
Comment 25 Carlos Soriano 2017-05-30 19:47:01 UTC
Review of attachment 352900 [details] [review]:

::: src/nautilus-window.c
@@ +144,3 @@
+
+    GQueue *tab_data_queue;
+

spurious line

@@ +1375,3 @@
+    data = g_queue_pop_head (priv->tab_data_queue);
+
+

GFile, bookmarks are not necesary
Comment 26 Alexandru Pandelea 2017-05-31 10:34:50 UTC
Created attachment 352936 [details] [review]
add "restore tab" action

Adds option to reopen closed tabs with Ctrl+Shift+T.

In order to do so, keep a list with data needed to restore closed
tabs. So, this list keeps the location bookmark, the view id before
search, which is needed in case the closed tab was a search and
the back/forward history.
Comment 27 Alexandru Pandelea 2017-05-31 10:35:18 UTC
Created attachment 352938 [details] [review]
nautilus-window: add wrapper for creating slot

Always when creating a slot in NautilusWindow, it is also initialized,
so create a wrapper which does both the creating and initializing.
Comment 28 Carlos Soriano 2017-05-31 11:28:39 UTC
Review of attachment 352936 [details] [review]:

Overall looks good now, just a nit

::: src/nautilus-window-slot.c
@@ +191,3 @@
+    data->back_list = back_list;
+    data->forward_list = forward_list;
+nautilus_window_slot_get_restore_tab_data (NautilusWindowSlot *self)

Add a clear comment here why this is necessary. When investigating leaks I'm pretty sure this will be necessary.
Also update OVERVIEW.md to reflect the saved tabs own a reference to previous files.
Comment 29 Alexandru Pandelea 2017-05-31 12:06:40 UTC
Created attachment 352945 [details] [review]
nautilus-window: add wrapper for creating slot

Always when creating a slot in NautilusWindow, it is also initialized,
so create a wrapper which does both the creating and initializing.
Comment 30 Alexandru Pandelea 2017-05-31 12:28:01 UTC
Created attachment 352946 [details] [review]
add "restore tab" action

Adds option to reopen closed tabs with Ctrl+Shift+T.

In order to do so, keep a list with data needed to restore closed
tabs. So, this list keeps the location bookmark, the view id before
search, which is needed in case the closed tab was a search and
the back/forward history.
Comment 31 Carlos Soriano 2017-05-31 12:31:00 UTC
Review of attachment 352945 [details] [review]:

+1
Comment 32 Carlos Soriano 2017-05-31 12:31:40 UTC
Review of attachment 352946 [details] [review]:

LGTM!!
Comment 33 Alexandru Pandelea 2017-05-31 13:02:14 UTC
Attachment 352945 [details] pushed as 9ab9de9 - nautilus-window: add wrapper for creating slot
Attachment 352946 [details] pushed as 6e16bc3 - add "restore tab" action