Merge lp:~jeremywootten/pantheon-files/fix-1643526-block-focus-out-during-rename into lp:~elementary-apps/pantheon-files/trunk

[Jeremy Wootten]

Do not end name editing when focus is lost

This branch fixes the issue where file or bookmark renaming ends prematurely of the Ctrl key is pressed. This only occurs if the accessibility option "Reveal pointer" is active, because the entry loses focus while the animation is shown. It also fixes premature ending of rename if the keyboard layout is switched (Alt +Shift)

A side effect of this branch is that rename does not end if you switch to another app. This allows pasting a name in from elsewhere.

To confirm rename press Enter. To cancel rename press Esc or click on the view widget or navigate away from the folder.

[Jeremy Wootten (jeremywootten)]

This is a better solution than the first attempt - the name of the branch is not quite accurate now.

[Jeremy Wootten (jeremywootten)]

Regression found in autorename new folder.

[Jeremy Wootten (jeremywootten)]

Regression now fixed.

[David Hewitt (davidmhewitt)]

I'm not so sure about this one. It definitely fixes the very annoying bug of renaming being cancelled when you press any of the hotkeys. But I don't like not being able to click in the window to cancel renaming. I feel like this is something that quite a few users would do.

It's especially bad that once you've clicked in the window, it then doesn't cancel when pressing escape or enter until you click back in the text field. How hard would it be to make any window click event cancel editing?

review: Disapprove

[Jeremy Wootten (jeremywootten)]

OK, I'll see whether that can be arranged.

[David Hewitt (davidmhewitt)]

I'm happy with this now, but I won't set the status to approved so that Cody can decide whether to merge before or after his release.

review: Approve (code, function)

[Maxim Taranov (png2378)]

Of course, It would be better to fix the cause (bug #1593358), not the consequence. But I think this MR is good and helpful temporary workaround, because problem with objects renaming in Files is most frequent case of bug with lost focus.

This branch didn't cause me any problems and works well, but I still can't change keyboard layout during the search in Files.

I also want to note that for some reasons Nautilus (I tried 3.18.4 and latest 3.23.90) behaves similarly - rename does not end if you switch to another window.

[Leonardo Lemos (leonardolemos)]

review: Approve (i18n)

[Jeremy Wootten (jeremywootten)]

> Of course, It would be better to fix the cause (bug #1593358), not the
> consequence. But I think this MR is good and helpful temporary workaround,
> because problem with objects renaming in Files is most frequent case of bug
> with lost focus.
>
> This branch didn't cause me any problems and works well, but I still can't
> change keyboard layout during the search in Files.
>
> I also want to note that for some reasons Nautilus (I tried 3.18.4 and latest
> 3.23.90) behaves similarly - rename does not end if you switch to another
> window.

Yes, this branch was only intended to fix renaming. The keyboard layout issue with the pathbar will be dealt with separately.

=== modified file 'src/TextRenderer.vala'
--- src/TextRenderer.vala	2017-01-27 12:16:50 +0000
+++ src/TextRenderer.vala	2017-02-15 19:03:51 +0000
@@ -34,18 +34,20 @@
         int focus_border_width;
         Pango.Layout layout;
         Gtk.Widget widget;
-        public Marlin.AbstractEditableLabel entry;
+        Marlin.AbstractEditableLabel entry;
+
+        construct {
+            this.mode = Gtk.CellRendererMode.EDITABLE;
+        }
 
         public TextRenderer (Marlin.ViewMode viewmode) {
-            this.mode = Gtk.CellRendererMode.EDITABLE;
-
-            if (viewmode == Marlin.ViewMode.ICON)
+            if (viewmode == Marlin.ViewMode.ICON) {
                 entry = new Marlin.MultiLineEditableLabel ();
-            else
+            } else {
                 entry = new Marlin.SingleLineEditableLabel ();
+            }
 
             entry.editing_done.connect (on_entry_editing_done);
-            entry.get_real_editable ().focus_out_event.connect_after (on_entry_focus_out_event);
         }
 
         public override void get_preferred_height_for_width (Gtk.Widget widget, int width, out int minimum_size, out int natural_size) {
@@ -180,6 +182,10 @@
             return entry as Gtk.CellEditable;
         }
 
+        public void end_editing (bool cancel) {
+            entry.end_editing (cancel);
+        }
+
         private void set_widget (Gtk.Widget? _widget) {
             Pango.FontMetrics metrics;
             Pango.Context context;
@@ -246,11 +252,6 @@
             file = null;
         }
 
-        private bool on_entry_focus_out_event (Gdk.Event event) {
-            on_entry_editing_done ();
-            return false;
-        }
-
         private void draw_focus (Cairo.Context cr,
                                  Gdk.Rectangle cell_area,
                                  Gtk.CellRendererState flags,

=== modified file 'src/View/AbstractDirectoryView.vala'
--- src/View/AbstractDirectoryView.vala	2017-02-07 19:38:46 +0000
+++ src/View/AbstractDirectoryView.vala	2017-02-15 19:03:51 +0000
@@ -3110,7 +3110,11 @@
         }
 
         protected virtual bool on_view_button_press_event (Gdk.EventButton event) {
-            grab_focus (); /* cancels any renaming */
+            if (renaming) {
+                /* Cancel renaming */
+                name_renderer.end_editing (true);
+            }
+
             cancel_hover (); /* cancel overlay statusbar cancellables */
 
             /* Ignore if second button pressed before first released - not permitted during rubberbanding.

=== modified file 'src/View/IconView.vala'
--- src/View/IconView.vala	2017-02-04 13:48:29 +0000
+++ src/View/IconView.vala	2017-02-15 19:03:51 +0000
@@ -38,6 +38,8 @@
             name_renderer = new Marlin.TextRenderer (Marlin.ViewMode.ICON);
             set_up_name_renderer ();
 
+            set_up_icon_renderer ();
+
             (tree as Gtk.CellLayout).pack_start (icon_renderer, false);
             (tree as Gtk.CellLayout).pack_end (name_renderer, false);
 
@@ -72,8 +74,6 @@
         protected override Gtk.Widget? create_view () {
             tree = new Gtk.IconView ();
             set_up_view ();
-            set_up_name_renderer ();
-            set_up_icon_renderer ();
 
             return tree as Gtk.Widget;
         }

=== modified file 'src/View/Widgets/AbstractEditableLabel.vala'
--- src/View/Widgets/AbstractEditableLabel.vala	2017-01-26 16:53:06 +0000
+++ src/View/Widgets/AbstractEditableLabel.vala	2017-02-15 19:03:51 +0000
@@ -46,16 +46,14 @@
                      *  to commit Chinese/Japanese characters when using some input methods, without ending rename.
                      */ 
                     if (mods == 0) {
-                        editing_canceled = false;
-                        remove_widget (); /* also causes edited signal to be emitted by CellRenderer */
+                        end_editing (false);
                         return true;
                     }
 
                     break;
 
                 case Gdk.Key.Escape:
-                    editing_canceled = true;
-                    remove_widget (); /* also causes edited signal to be emitted by CellRenderer */
+                    end_editing (true);
                     return true;
 
                 case Gdk.Key.z:
@@ -72,6 +70,11 @@
             return false;
         }
 
+        public void end_editing (bool cancelled) {
+            editing_canceled = cancelled;
+            remove_widget ();
+            editing_done ();
+        }
 
         public virtual void set_text (string text) {
             original_name = text;