Merge lp:~jeremywootten/pantheon-files/fix-1626240-breadcrumb-name-with-hash into lp:~elementary-apps/pantheon-files/trunk

[Jeremy Wootten]

enable paths containing folders starting with a '#'

The changes made enable paths containing folders starting with a '#' character to be handled correctly both on local and remote (smb) file systems when creating and restoring views.

Also, the breadcrumb tooltip now displays the unescaped text.

Views are now only created via the Window.uri_path_change_request () function; the corresponding file_path_change_request function being made private. This is to ensure consistent treatment of the uri before creating the view.

(This branch is the basis for a more extensive reworking to make this kind of bug less likely and easier to fix - see lp:~jeremywootten/pantheon-files/refactor-path-change-requests)

TO TEST: See bug report.

[lp:~jeremywootten/pantheon-files/fix-1626240-breadcrumb-name-with-hash]

2391. By Jeremy Wootten on 2017-01-14

Merge trunk to r2444

2392. By Jeremy Wootten on 2017-02-04

Merge trunk to r2470

[Dieter Debast (ddieter)]

When launching files from the command line with an URI containing a #, it won't navigate to the correct location but has the same behaviour like a bookmark (before the fix of course).

Example:
pantheon-files "smb://localhost/share/#test/second/"
Will navigate to "smb://localhost/share/"

Otherwise seems to work as intended.

review: Needs Fixing

[Jeremy Wootten (jeremywootten)]

Hi Dieter, thanks very much for reviewing this branch. Strictly speaking the issue you raise is a separate bug from the one linked to this branch (which only refers to the headerbar/breadcrumbs). However, I agree it is closely related and can probably be fixed in a similar way. I'll check whether it can be included in this branch with minimal extra code.

[Dieter Debast (ddieter)]

Hello Jeremy, if this is too much work to also fix in this branch, let me know and I will accept it. It can be put in a different issue then.

[lp:~jeremywootten/pantheon-files/fix-1626240-breadcrumb-name-with-hash]

2393. By Jeremy Wootten on 2017-06-03

Merge trunk to r2570 and fix conflict

2394. By Jeremy Wootten on 2017-06-03

Fix hash in commandline uri

[Jeremy Wootten (jeremywootten)]

Hello Dieter, I have pushed a relatively simple fix to your issue. Ultimately, I would like to refactor the code so that path sanitization for new tabs/windows occurs in only one place but that can wait for another branch.

I also updated the branch to be compatible with the latest trunk.

[Dieter Debast (ddieter)]

Thanks Jeremy, it fixes the issue and there are no obvious code issues so I'll accept this.

review: Approve

=== modified file 'libcore/FileUtils.vala'
--- libcore/FileUtils.vala	2017-03-10 17:31:17 +0000
+++ libcore/FileUtils.vala	2017-06-03 12:06:40 +0000
@@ -162,7 +162,9 @@
         return Uri.escape_string ((Uri.unescape_string (uri) ?? uri), rc , allow_utf8);
     }
 
-    /** Produce a valid unescaped path **/
+    /** Produce a valid unescaped path.  A current path can be provided and is used to get the scheme and
+      * to interpret relative paths where necessary.
+      **/
     public string sanitize_path (string? p, string? cp = null) {
         string path = "";
         string scheme = "";
 
=== modified file 'libcore/gof-directory-async.vala'
--- libcore/gof-directory-async.vala	2017-05-08 10:39:31 +0000
+++ libcore/gof-directory-async.vala	2017-06-03 12:06:40 +0000
@@ -107,8 +107,14 @@
     public string last_error_message {get; private set; default = "";}
 
     private Async (GLib.File _file) {
-        /* Ensure uri is correctly escaped */
-        location = GLib.File.new_for_uri (PF.FileUtils.escape_uri (_file.get_uri ()));
+        /* Ensure uri is correctly escaped and has scheme */
+        var escaped_uri = PF.FileUtils.escape_uri (_file.get_uri ());
+        scheme = Uri.parse_scheme (escaped_uri);
+        if (scheme == null) {
+            scheme = Marlin.ROOT_FS_URI;
+            escaped_uri = scheme + escaped_uri;
+        }
+        location = GLib.File.new_for_uri (escaped_uri);
         file = GOF.File.get (location);
         selected_file = null;
 
 
=== modified file 'libwidgets/Chrome/BasicBreadcrumbsEntry.vala'
--- libwidgets/Chrome/BasicBreadcrumbsEntry.vala	2017-02-28 18:55:07 +0000
+++ libwidgets/Chrome/BasicBreadcrumbsEntry.vala	2017-06-03 12:06:40 +0000
@@ -284,7 +284,7 @@
             set_tooltip_text ("");
             var el = get_element_from_coordinates ((int)event.x, (int)event.y);
             if (el != null) {
-                set_tooltip_text (_("Go to %s").printf (el.text));
+                set_tooltip_text (_("Go to %s").printf (el.text_for_display));
                 set_entry_cursor (new Gdk.Cursor.from_name (Gdk.Display.get_default (), "default"));
             } else {
                 set_entry_cursor (null);
@@ -476,6 +476,7 @@
                     if (el != null && element == el)
                         break;
             }
+
             return PF.FileUtils.sanitize_path (newpath);
         }
 
 
=== modified file 'src/Application.vala'
--- src/Application.vala	2017-05-08 10:39:31 +0000
+++ src/Application.vala	2017-06-03 12:06:40 +0000
@@ -200,10 +200,16 @@
 
         /* Convert remaining arguments to GFiles */
         foreach (string filepath in remaining) {
-            var file = File.new_for_commandline_arg (filepath);
-
-            if (file != null)
+            string path = PF.FileUtils.sanitize_path (filepath, null);
+            GLib.File? file = null;
+
+            if (path.length > 0) {
+                file = File.new_for_uri (PF.FileUtils.escape_uri (path));
+            }
+
+            if (file != null) {
                 files += (file);
+            }
         }
         /* Open application */
         if (create_new_window) {
 
=== modified file 'src/View/Sidebar.vala'
--- src/View/Sidebar.vala	2017-05-16 19:50:22 +0000
+++ src/View/Sidebar.vala	2017-06-03 12:06:40 +0000
@@ -1288,7 +1288,7 @@
                 } else if (flags == Marlin.OpenFlag.NEW_TAB) {
                     window.add_tab (location, Marlin.ViewMode.CURRENT);
                 } else {
-                    window.file_path_change_request (location);
+                    window.uri_path_change_request (uri);
                 }
             } else if (f != null) {
                 f (this);
@@ -1328,7 +1328,7 @@
                         } else if (flags == Marlin.OpenFlag.NEW_TAB) {
                             window.add_tab (location, Marlin.ViewMode.CURRENT);
                         } else {
-                            window.file_path_change_request (location);
+                            window.uri_path_change_request (location.get_uri ());
                         }
                     }
                 }
 
=== modified file 'src/View/Window.vala'
--- src/View/Window.vala	2017-05-16 20:04:17 +0000
+++ src/View/Window.vala	2017-06-03 12:06:40 +0000
@@ -324,11 +324,11 @@
             });
 
             tabs.tab_restored.connect ((label, restore_data, icon) => {
-                add_tab (File.new_for_uri (restore_data));
+                add_tab_by_uri (restore_data);
             });
 
             tabs.tab_duplicated.connect ((tab) => {
-                add_tab (File.new_for_uri (((tab.page as ViewContainer).uri)));
+                add_tab_by_uri (((ViewContainer)(tab.page)).uri);
             });
 
             tabs.tab_moved.connect ((tab, x, y) => {
@@ -456,6 +456,15 @@
             top_menu.working = current_tab.is_frozen;
         }
 
+        public void add_tab_by_uri (string uri, Marlin.ViewMode mode = Marlin.ViewMode.PREFERRED) {
+            var file = get_file_from_uri (uri);
+            if (file != null) {
+                add_tab (file, mode);
+            } else {
+                add_tab ();
+            }
+        }
+
         public void add_tab (File location = File.new_for_commandline_arg (Environment.get_home_dir ()),
                              Marlin.ViewMode mode = Marlin.ViewMode.PREFERRED) {
             mode = real_mode (mode);
@@ -978,24 +987,15 @@
                 if (mode < 0 || mode >= Marlin.ViewMode.INVALID || root_uri == null || root_uri == "" || tip_uri == null)
                     continue;
 
-                string? unescaped_root_uri = PF.FileUtils.sanitize_path (root_uri);
-
-                if (unescaped_root_uri == null) {
-                    warning ("Invalid root location for tab");
-                    continue;
-                }
-
-                GLib.File root_location = GLib.File.new_for_commandline_arg (unescaped_root_uri);
-
                 /* We do not check valid location here because it may cause the interface to hang
                  * before the window appears (e.g. if trying to connect to a server that has become unavailable)
                  * Leave it to GOF.Directory.Async to deal with invalid locations asynchronously. 
                  */
 
-                add_tab (root_location, mode);
+                add_tab_by_uri (root_uri, mode);
 
                 if (mode == Marlin.ViewMode.MILLER_COLUMNS && tip_uri != root_uri) {
-                    expand_miller_view (tip_uri, root_location);
+                    expand_miller_view (tip_uri, root_uri);
                 }
 
                 tabs_added++;
@@ -1037,7 +1037,7 @@
             return tabs_added;
         }
 
-        private void expand_miller_view (string tip_uri, GLib.File root_location) {
+        private void expand_miller_view (string tip_uri, string unescaped_root_uri) {
             /* It might be more elegant for Miller.vala to handle this */
             var tab = tabs.current;
             var view = tab.page as ViewContainer;
@@ -1050,6 +1050,7 @@
             }
 
             var tip_location = PF.FileUtils.get_file_for_path (unescaped_tip_uri);
+            var root_location = PF.FileUtils.get_file_for_path (unescaped_root_uri);
             var relative_path = root_location.get_relative_path (tip_location);
             GLib.File gfile;
 
@@ -1059,7 +1060,7 @@
 
                 foreach (string dir in dirs) {
                     uri += (GLib.Path.DIR_SEPARATOR_S + dir);
-                    gfile = PF.FileUtils.get_file_for_path (uri);
+                    gfile = get_file_from_uri (uri);
 
                     mwcols.add_location (gfile, mwcols.current_slot, false); /* Do not scroll at this stage */
                 }
@@ -1111,7 +1112,7 @@
             }
         }
 
-        public void file_path_change_request (GLib.File loc, Marlin.OpenFlag flag = Marlin.OpenFlag.DEFAULT) {
+        private void file_path_change_request (GLib.File loc, Marlin.OpenFlag flag = Marlin.OpenFlag.DEFAULT) {
             /* ViewContainer deals with non-existent or unmounted directories
              * and locations that are not directories */
             if (restoring_tabs) {
@@ -1130,11 +1131,28 @@
         }
 
         public void uri_path_change_request (string p, Marlin.OpenFlag flag = Marlin.OpenFlag.DEFAULT) {
-            string path = PF.FileUtils.sanitize_path (p, current_tab.location.get_path ());
+            var file = get_file_from_uri (p);
+            if (file != null) {
+                /* Have to escape path and use File.new_for_uri () to correctly handle paths with certain characters such as "#" */
+                file_path_change_request (file, flag);
+            } else {
+                warning ("Cannot browse %s", p);
+            }
+        }
+
+        /** Use this function to standardise how locations are generated from uris **/
+        private File? get_file_from_uri (string uri) {
+            /* Sanitize path removes file:// scheme if present, but GOF.Directory.Async will replace it */
+            string? current_uri = null;
+            if (current_tab != null && current_tab.location != null) {
+                current_uri = current_tab.location.get_uri ();
+            }
+
+            string path = PF.FileUtils.sanitize_path (uri, current_uri);
             if (path.length > 0) {
-                file_path_change_request (File.new_for_commandline_arg (path), flag);
+                return File.new_for_uri (PF.FileUtils.escape_uri (path));
             } else {
-                warning ("Cannot browse %s", p);
+                return null;
             }
         }