rename-file-popover-controller: Make popover recyclable

Creating and destroying the controller each time we want to rename
something results in not being able to pop down the popover, which leads
to an inconsistent look.

The “containing-directory” in the file name widget controller parent
class needed to be made non-construct-only, as otherwise it would be
impossible to reuse the same controller.

Fixes https://gitlab.gnome.org/GNOME/nautilus/issues/260

5 files changed, 190 insertions, 102 deletions

diff --git a/src/nautilus-file-name-widget-controller.c b/src/nautilus-file-name-widget-controller.c
index 0deff902b..9b49b0f4c 100644
--- a/src/nautilus-file-name-widget-controller.c
+++ b/src/nautilus-file-name-widget-controller.c
@@ -63,6 +63,15 @@ nautilus_file_name_widget_controller_get_new_name (NautilusFileNameWidgetControl
     return NAUTILUS_FILE_NAME_WIDGET_CONTROLLER_GET_CLASS (self)->get_new_name (self);
 }
 
+void
+nautilus_file_name_widget_controller_set_containing_directory (NautilusFileNameWidgetController *self,
+                                                               NautilusDirectory                *directory)
+{
+    g_assert (NAUTILUS_IS_DIRECTORY (directory));
+
+    g_object_set (self, "containing-directory", directory, NULL);
+}
+
 static gboolean
 nautilus_file_name_widget_controller_name_is_valid (NautilusFileNameWidgetController  *self,
                                                     gchar                             *name,
@@ -160,6 +169,8 @@ file_name_widget_controller_process_new_name (NautilusFileNameWidgetController *
     NautilusFile *existing_file;
     priv = nautilus_file_name_widget_controller_get_instance_private (controller);
 
+    g_return_if_fail (NAUTILUS_IS_DIRECTORY (priv->containing_directory));
+
     name = nautilus_file_name_widget_controller_get_new_name (controller);
     *valid_name = nautilus_file_name_widget_controller_name_is_valid (controller,
                                                                       name,
@@ -289,6 +300,11 @@ file_name_widget_controller_on_activate (gpointer user_data)
 static void
 nautilus_file_name_widget_controller_init (NautilusFileNameWidgetController *self)
 {
+    NautilusFileNameWidgetControllerPrivate *priv;
+
+    priv = nautilus_file_name_widget_controller_get_instance_private (self);
+
+    priv->containing_directory = NULL;
 }
 
 static void
@@ -345,8 +361,9 @@ nautilus_file_name_widget_controller_set_property (GObject      *object,
 
         case PROP_CONTAINING_DIRECTORY:
         {
-            priv->containing_directory = NAUTILUS_DIRECTORY (g_value_get_object (value));
-            nautilus_directory_ref (priv->containing_directory);
+            g_clear_object (&priv->containing_directory);
+
+            priv->containing_directory = NAUTILUS_DIRECTORY (g_value_dup_object (value));
         }
         break;
 
@@ -460,6 +477,5 @@ nautilus_file_name_widget_controller_class_init (NautilusFileNameWidgetControlle
                              "Containing Directory",
                              "The directory used to check for duplicate names",
                              NAUTILUS_TYPE_DIRECTORY,
-                             G_PARAM_WRITABLE |
-                             G_PARAM_CONSTRUCT_ONLY));
+                             G_PARAM_WRITABLE));
 }
diff --git a/src/nautilus-file-name-widget-controller.h b/src/nautilus-file-name-widget-controller.h
index 94b23b880..1bd481c43 100644
--- a/src/nautilus-file-name-widget-controller.h
+++ b/src/nautilus-file-name-widget-controller.h
@@ -45,3 +45,6 @@ struct _NautilusFileNameWidgetControllerClass
 };
 
 gchar * nautilus_file_name_widget_controller_get_new_name (NautilusFileNameWidgetController *controller);
+
+void    nautilus_file_name_widget_controller_set_containing_directory (NautilusFileNameWidgetController *controller,
+                                                                       NautilusDirectory                *directory);
diff --git a/src/nautilus-files-view.c b/src/nautilus-files-view.c
index fe276e277..a1cd2c9cd 100644
--- a/src/nautilus-files-view.c
+++ b/src/nautilus-files-view.c
@@ -269,6 +269,9 @@ typedef struct
 
     GCancellable *starred_cancellable;
     NautilusTagManager *tag_manager;
+
+    gint name_accepted_handler_id;
+    gint cancelled_handler_id;
 } NautilusFilesViewPrivate;
 
 typedef struct
@@ -1885,6 +1888,29 @@ nautilus_files_view_compute_rename_popover_pointing_to (NautilusFilesView *view)
 }
 
 static void
+disconnect_rename_controller_signals (NautilusFilesView *self)
+{
+    NautilusFilesViewPrivate *priv;
+
+    g_assert (NAUTILUS_IS_FILES_VIEW (self));
+
+    priv = nautilus_files_view_get_instance_private (self);
+
+    if (priv->name_accepted_handler_id != 0)
+    {
+        g_signal_handler_disconnect (priv->rename_file_controller, priv->name_accepted_handler_id);
+        priv->name_accepted_handler_id = 0;
+    }
+
+    if (priv->cancelled_handler_id != 0)
+    {
+        g_signal_handler_disconnect (priv->rename_file_controller,
+                                     priv->cancelled_handler_id);
+        priv->cancelled_handler_id = 0;
+    }
+}
+
+static void
 rename_file_popover_controller_on_name_accepted (NautilusFileNameWidgetController *controller,
                                                  gpointer                          user_data)
 {
@@ -1908,7 +1934,7 @@ rename_file_popover_controller_on_name_accepted (NautilusFileNameWidgetControlle
 
     nautilus_rename_file (target_file, name, NULL, NULL);
 
-    g_clear_object (&priv->rename_file_controller);
+    disconnect_rename_controller_signals (view);
 }
 
 static void
@@ -1916,12 +1942,10 @@ rename_file_popover_controller_on_cancelled (NautilusFileNameWidgetController *c
                                              gpointer                          user_data)
 {
     NautilusFilesView *view;
-    NautilusFilesViewPrivate *priv;
 
     view = NAUTILUS_FILES_VIEW (user_data);
-    priv = nautilus_files_view_get_instance_private (view);
 
-    g_clear_object (&priv->rename_file_controller);
+    disconnect_rename_controller_signals (view);
 }
 
 static void
@@ -1933,11 +1957,6 @@ nautilus_files_view_rename_file_popover_new (NautilusFilesView *view,
 
     priv = nautilus_files_view_get_instance_private (view);
 
-    if (priv->rename_file_controller != NULL)
-    {
-        return;
-    }
-
     /* Make sure the whole item is visible. The selection is a single item, the
      * one to rename with the popover, so we can use reveal_selection() for this.
      */
@@ -1945,19 +1964,19 @@ nautilus_files_view_rename_file_popover_new (NautilusFilesView *view,
 
     pointing_to = nautilus_files_view_compute_rename_popover_pointing_to (view);
 
-    priv->rename_file_controller =
-        nautilus_rename_file_popover_controller_new (target_file,
-                                                     pointing_to,
-                                                     GTK_WIDGET (view));
+    nautilus_rename_file_popover_controller_show_for_file (priv->rename_file_controller,
+                                                           target_file,
+                                                           pointing_to,
+                                                           GTK_WIDGET (view));
 
-    g_signal_connect (priv->rename_file_controller,
-                      "name-accepted",
-                      (GCallback) rename_file_popover_controller_on_name_accepted,
-                      view);
-    g_signal_connect (priv->rename_file_controller,
-                      "cancelled",
-                      (GCallback) rename_file_popover_controller_on_cancelled,
-                      view);
+    priv->name_accepted_handler_id = g_signal_connect (priv->rename_file_controller,
+                                                       "name-accepted",
+                                                       G_CALLBACK (rename_file_popover_controller_on_name_accepted),
+                                                       view);
+    priv->cancelled_handler_id = g_signal_connect (priv->rename_file_controller,
+                                                   "cancelled",
+                                                   G_CALLBACK (rename_file_popover_controller_on_cancelled),
+                                                   view);
 }
 
 static void
@@ -9649,6 +9668,8 @@ nautilus_files_view_init (NautilusFilesView *view)
     priv->starred_cancellable = g_cancellable_new ();
     priv->tag_manager = nautilus_tag_manager_get ();
 
+    priv->rename_file_controller = nautilus_rename_file_popover_controller_new ();
+
     nautilus_profile_end (NULL);
 }
 
diff --git a/src/nautilus-rename-file-popover-controller.c b/src/nautilus-rename-file-popover-controller.c
index 5806315fb..2c10e07e2 100644
--- a/src/nautilus-rename-file-popover-controller.c
+++ b/src/nautilus-rename-file-popover-controller.c
@@ -38,14 +38,53 @@ struct _NautilusRenameFilePopoverController
     gboolean target_is_folder;
 
     GtkWidget *rename_file_popover;
+    GtkWidget *name_entry;
+    GtkWidget *name_label;
 
     gint closed_handler_id;
     gint file_changed_handler_id;
+    gint key_press_event_handler_id;
 };
 
 G_DEFINE_TYPE (NautilusRenameFilePopoverController, nautilus_rename_file_popover_controller, NAUTILUS_TYPE_FILE_NAME_WIDGET_CONTROLLER)
 
 static void
+disconnect_signal_handlers (NautilusRenameFilePopoverController *self)
+{
+    g_assert (NAUTILUS_IS_RENAME_FILE_POPOVER_CONTROLLER (self));
+
+    if (self->closed_handler_id != 0)
+    {
+        g_signal_handler_disconnect (self->rename_file_popover, self->closed_handler_id);
+        self->closed_handler_id = 0;
+    }
+
+    if (self->file_changed_handler_id != 0)
+    {
+        g_signal_handler_disconnect (self->target_file, self->file_changed_handler_id);
+        self->file_changed_handler_id = 0;
+    }
+
+    if (self->key_press_event_handler_id != 0)
+    {
+        g_signal_handler_disconnect (self->name_entry, self->key_press_event_handler_id);
+        self->key_press_event_handler_id = 0;
+    }
+}
+
+static void
+reset_state (NautilusRenameFilePopoverController *self)
+{
+    g_assert (NAUTILUS_IS_RENAME_FILE_POPOVER_CONTROLLER (self));
+
+    disconnect_signal_handlers (self);
+
+    g_clear_object (&self->target_file);
+
+    gtk_popover_popdown (GTK_POPOVER (self->rename_file_popover));
+}
+
+static void
 rename_file_popover_controller_on_closed (GtkPopover *popover,
                                           gpointer    user_data)
 {
@@ -53,9 +92,7 @@ rename_file_popover_controller_on_closed (GtkPopover *popover,
 
     controller = NAUTILUS_RENAME_FILE_POPOVER_CONTROLLER (user_data);
 
-    g_signal_handler_disconnect (controller->rename_file_popover,
-                                 controller->closed_handler_id);
-    controller->closed_handler_id = 0;
+    reset_state (controller);
 
     g_signal_emit_by_name (controller, "cancelled");
 }
@@ -137,7 +174,7 @@ name_entry_on_f2_pressed (GtkWidget                           *widget,
     text_length = (guint) gtk_entry_get_text_length (GTK_ENTRY (widget));
     if (text_length == 0)
     {
-        return GDK_EVENT_PROPAGATE;
+        return GDK_EVENT_STOP;
     }
 
     gtk_editable_get_selection_bounds (GTK_EDITABLE (widget),
@@ -160,7 +197,7 @@ name_entry_on_f2_pressed (GtkWidget                           *widget,
                                     start_offset, end_offset);
     }
 
-    return GDK_EVENT_PROPAGATE;
+    return GDK_EVENT_STOP;
 }
 
 static gboolean
@@ -212,18 +249,14 @@ target_file_on_changed (NautilusFile *file,
 
     if (nautilus_file_is_gone (file))
     {
-        g_signal_handler_disconnect (controller->target_file,
-                                     controller->file_changed_handler_id);
-        controller->file_changed_handler_id = 0;
+        reset_state (controller);
 
         g_signal_emit_by_name (controller, "cancelled");
     }
 }
 
 NautilusRenameFilePopoverController *
-nautilus_rename_file_popover_controller_new (NautilusFile *target_file,
-                                             GdkRectangle *pointing_to,
-                                             GtkWidget    *relative_to)
+nautilus_rename_file_popover_controller_new (void)
 {
     NautilusRenameFilePopoverController *self;
     g_autoptr (GtkBuilder) builder = NULL;
@@ -233,9 +266,6 @@ nautilus_rename_file_popover_controller_new (NautilusFile *target_file,
     GtkWidget *name_entry;
     GtkWidget *activate_button;
     GtkWidget *name_label;
-    NautilusDirectory *containing_directory;
-    g_autofree char *display_name = NULL;
-    gint n_chars;
 
     builder = gtk_builder_new_from_resource ("/org/gnome/nautilus/ui/nautilus-rename-file-popover.ui");
     rename_file_popover = GTK_WIDGET (gtk_builder_get_object (builder, "rename_file_popover"));
@@ -245,35 +275,67 @@ nautilus_rename_file_popover_controller_new (NautilusFile *target_file,
     activate_button = GTK_WIDGET (gtk_builder_get_object (builder, "rename_button"));
     name_label = GTK_WIDGET (gtk_builder_get_object (builder, "name_label"));
 
-    if (!nautilus_file_is_self_owned (target_file))
-    {
-        NautilusFile *parent_location;
+    self = g_object_new (NAUTILUS_TYPE_RENAME_FILE_POPOVER_CONTROLLER,
+                         "error-revealer", error_revealer,
+                         "error-label", error_label,
+                         "name-entry", name_entry,
+                         "activate-button", activate_button,
+                         NULL);
+
+    self->rename_file_popover = g_object_ref_sink (rename_file_popover);
+    self->name_entry = name_entry;
+    self->name_label = name_label;
+
+    gtk_popover_set_default_widget (GTK_POPOVER (rename_file_popover), name_entry);
+
+    return self;
+}
+
+NautilusFile *
+nautilus_rename_file_popover_controller_get_target_file (NautilusRenameFilePopoverController *self)
+{
+    g_return_val_if_fail (NAUTILUS_IS_RENAME_FILE_POPOVER_CONTROLLER (self), NULL);
+
+    return self->target_file;
+}
+
+void
+nautilus_rename_file_popover_controller_show_for_file   (NautilusRenameFilePopoverController *self,
+                                                         NautilusFile                        *target_file,
+                                                         GdkRectangle                        *pointing_to,
+                                                         GtkWidget                           *relative_to)
+{
+    g_autoptr (NautilusDirectory) containing_directory = NULL;
+    g_autofree gchar *display_name = NULL;
+    gint n_chars;
+
+    g_assert (NAUTILUS_IS_RENAME_FILE_POPOVER_CONTROLLER (self));
+    g_assert (NAUTILUS_IS_FILE (target_file));
 
-        parent_location = nautilus_file_get_parent (target_file);
-        containing_directory = nautilus_directory_get_for_file (parent_location);
+    reset_state (self);
 
-        nautilus_file_unref (parent_location);
+    self->target_file = g_object_ref (target_file);
+
+    if (!nautilus_file_is_self_owned (self->target_file))
+    {
+        g_autoptr (NautilusFile) parent = NULL;
+
+        parent = nautilus_file_get_parent (self->target_file);
+        containing_directory = nautilus_directory_get_for_file (parent);
     }
     else
     {
-        containing_directory = nautilus_directory_get_for_file (target_file);
+        containing_directory = nautilus_directory_get_for_file (self->target_file);
     }
 
-    self = g_object_new (NAUTILUS_TYPE_RENAME_FILE_POPOVER_CONTROLLER,
-                         "error-revealer", error_revealer,
-                         "error-label", error_label,
-                         "name-entry", name_entry,
-                         "activate-button", activate_button,
-                         "containing-directory", containing_directory, NULL);
-
-    self->target_is_folder = nautilus_file_is_directory (target_file);
-    self->target_file = nautilus_file_ref (target_file);
+    nautilus_file_name_widget_controller_set_containing_directory (NAUTILUS_FILE_NAME_WIDGET_CONTROLLER (self),
+                                                                   containing_directory);
 
-    self->rename_file_popover = rename_file_popover;
+    self->target_is_folder = nautilus_file_is_directory (self->target_file);
 
-    self->closed_handler_id = g_signal_connect (rename_file_popover,
+    self->closed_handler_id = g_signal_connect (self->rename_file_popover,
                                                 "closed",
-                                                (GCallback) rename_file_popover_controller_on_closed,
+                                                G_CALLBACK (rename_file_popover_controller_on_closed),
                                                 self);
 
     self->file_changed_handler_id = g_signal_connect (self->target_file,
@@ -281,26 +343,25 @@ nautilus_rename_file_popover_controller_new (NautilusFile *target_file,
                                                       G_CALLBACK (target_file_on_changed),
                                                       self);
 
-    g_signal_connect (name_entry,
-                      "key-press-event",
-                      G_CALLBACK (name_entry_on_key_pressed),
-                      self);
+    self->key_press_event_handler_id = g_signal_connect (self->name_entry,
+                                                         "key-press-event",
+                                                         G_CALLBACK (name_entry_on_key_pressed),
+                                                         self);
 
-    gtk_label_set_text (GTK_LABEL (name_label),
+    gtk_label_set_text (GTK_LABEL (self->name_label),
                         self->target_is_folder ? _("Folder name") :
                         _("File name"));
 
-    display_name = nautilus_file_get_display_name (target_file);
+    display_name = nautilus_file_get_display_name (self->target_file);
 
-    gtk_entry_set_text (GTK_ENTRY (name_entry), display_name);
+    gtk_entry_set_text (GTK_ENTRY (self->name_entry), display_name);
 
-    gtk_popover_set_default_widget (GTK_POPOVER (rename_file_popover), name_entry);
-    gtk_popover_set_pointing_to (GTK_POPOVER (rename_file_popover), pointing_to);
-    gtk_popover_set_relative_to (GTK_POPOVER (rename_file_popover), relative_to);
+    gtk_popover_set_pointing_to (GTK_POPOVER (self->rename_file_popover), pointing_to);
+    gtk_popover_set_relative_to (GTK_POPOVER (self->rename_file_popover), relative_to);
 
-    gtk_popover_popup (GTK_POPOVER (rename_file_popover));
+    gtk_popover_popup (GTK_POPOVER (self->rename_file_popover));
 
-    if (nautilus_file_is_regular_file (target_file))
+    if (nautilus_file_is_regular_file (self->target_file))
     {
         gint start_offset;
         gint end_offset;
@@ -308,31 +369,30 @@ nautilus_rename_file_popover_controller_new (NautilusFile *target_file,
         /* Select the name part without the file extension */
         eel_filename_get_rename_region (display_name,
                                         &start_offset, &end_offset);
-        gtk_editable_select_region (GTK_EDITABLE (name_entry),
+        gtk_editable_select_region (GTK_EDITABLE (self->name_entry),
                                     start_offset, end_offset);
     }
 
     n_chars = g_utf8_strlen (display_name, -1);
-    gtk_entry_set_width_chars (GTK_ENTRY (name_entry),
+    gtk_entry_set_width_chars (GTK_ENTRY (self->name_entry),
                                MIN (MAX (n_chars, RENAME_ENTRY_MIN_CHARS),
                                     RENAME_ENTRY_MAX_CHARS));
-
-    nautilus_directory_unref (containing_directory);
-
-    return self;
 }
 
-NautilusFile *
-nautilus_rename_file_popover_controller_get_target_file (NautilusRenameFilePopoverController *self)
+static void
+on_name_accepted (NautilusFileNameWidgetController *controller)
 {
-    g_return_val_if_fail (NAUTILUS_IS_RENAME_FILE_POPOVER_CONTROLLER (self), NULL);
+    NautilusRenameFilePopoverController *self;
 
-    return self->target_file;
+    self = NAUTILUS_RENAME_FILE_POPOVER_CONTROLLER (controller);
+
+    reset_state (self);
 }
 
 static void
 nautilus_rename_file_popover_controller_init (NautilusRenameFilePopoverController *self)
 {
+    g_signal_connect_after (self, "name-accepted", G_CALLBACK (on_name_accepted), self);
 }
 
 static void
@@ -342,25 +402,10 @@ nautilus_rename_file_popover_controller_finalize (GObject *object)
 
     self = NAUTILUS_RENAME_FILE_POPOVER_CONTROLLER (object);
 
-    if (self->rename_file_popover)
-    {
-        if (self->closed_handler_id)
-        {
-            g_signal_handler_disconnect (self->rename_file_popover,
-                                         self->closed_handler_id);
-            self->closed_handler_id = 0;
-        }
-        gtk_popover_popdown (GTK_POPOVER (self->rename_file_popover));
-        g_clear_pointer (&self->rename_file_popover, gtk_widget_destroy);
-    }
+    reset_state (self);
 
-    if (self->file_changed_handler_id != 0)
-    {
-        g_signal_handler_disconnect (self->target_file,
-                                     self->file_changed_handler_id);
-        self->file_changed_handler_id = 0;
-    }
-    nautilus_file_unref (self->target_file);
+    gtk_widget_destroy (self->rename_file_popover);
+    g_clear_object (&self->rename_file_popover);
 
     G_OBJECT_CLASS (nautilus_rename_file_popover_controller_parent_class)->finalize (object);
 }
diff --git a/src/nautilus-rename-file-popover-controller.h b/src/nautilus-rename-file-popover-controller.h
index 3b55ffdb7..c4627bce5 100644
--- a/src/nautilus-rename-file-popover-controller.h
+++ b/src/nautilus-rename-file-popover-controller.h
@@ -28,8 +28,11 @@
 #define NAUTILUS_TYPE_RENAME_FILE_POPOVER_CONTROLLER nautilus_rename_file_popover_controller_get_type ()
 G_DECLARE_FINAL_TYPE (NautilusRenameFilePopoverController, nautilus_rename_file_popover_controller, NAUTILUS, RENAME_FILE_POPOVER_CONTROLLER, NautilusFileNameWidgetController)
 
-NautilusRenameFilePopoverController * nautilus_rename_file_popover_controller_new (NautilusFile *target_file,
-                                                                                   GdkRectangle *pointing_to,
-                                                                                   GtkWidget    *relative_to);
+NautilusRenameFilePopoverController * nautilus_rename_file_popover_controller_new (void);
 
-NautilusFile * nautilus_rename_file_popover_controller_get_target_file (NautilusRenameFilePopoverController *controller);
\ No newline at end of file
+NautilusFile * nautilus_rename_file_popover_controller_get_target_file (NautilusRenameFilePopoverController *controller);
+
+void           nautilus_rename_file_popover_controller_show_for_file   (NautilusRenameFilePopoverController *controller,
+                                                                        NautilusFile                        *target_file,
+                                                                        GdkRectangle                        *pointing_to,
+                                                                        GtkWidget                           *relative_to);