Merge lp:~jeremywootten/pantheon-files/fix-1518692-right-click-samba-breadcrumb into lp:~elementary-apps/pantheon-files/trunk

[Jeremy Wootten]

Fix breadcrumb context menu for uri schemes other than file:// (lp:1518692)

This branch fixes a number of issues relating to right-clicking on breadcrumbs representing remote shares, servers or protocols, including possible crashes.

TO TEST:
Navigate to shares and subfolders for the following protocols:
smb://
ftp://
sftp://
afp://
webdav://
webdavs://
trash://
mtp://
recent://

Right-click on breadcrumbs. There should be no crashes and at least the base context menu should appear and function normally and in most cases a 'folder' list is produced.

=== modified file 'libcore/gof-directory-async.vala'
--- libcore/gof-directory-async.vala	2015-10-30 12:45:44 +0000
+++ libcore/gof-directory-async.vala	2015-11-22 18:20:03 +0000
@@ -874,8 +874,9 @@
             return sorted_dirs;
 
         foreach (var gof in file_hash.get_values()) {
-            if (!gof.is_hidden && gof.is_folder ())
+            if (!gof.is_hidden && (gof.is_folder () || gof.is_smb_server ())) {
                 sorted_dirs.prepend (gof);
+            }
         }
 
         sorted_dirs.sort (GOF.File.compare_by_display_name);

=== modified file 'libwidgets/FileUtils.vala'
--- libwidgets/FileUtils.vala	2015-10-09 11:12:36 +0000
+++ libwidgets/FileUtils.vala	2015-11-22 18:20:03 +0000
@@ -31,23 +31,20 @@
     }
 
     public string get_parent_path_from_path (string path) {
-        File? file = File.new_for_commandline_arg (path);
-        string parent_path = path;
-        if (file != null) {
-            File? parent = file.get_parent ();
-            if (parent != null) {
-                parent_path = parent.get_path ();
-            } else {
-                parent_path = construct_parent_path (path);
-            }
-        }
+        /* We construct the parent path rather than use File.get_parent () as the latter gives odd
+         * results for some gvfs files.
+         */  
+        string parent_path = construct_parent_path (path);
         if (parent_path == Marlin.FTP_URI ||
-            parent_path == Marlin.MTP_URI ||
             parent_path == Marlin.SFTP_URI) {
 
             parent_path = path;
         }
 
+        if (parent_path.has_prefix (Marlin.MTP_URI) && !valid_mtp_uri (parent_path)) {
+            parent_path = path;
+        }
+
         if (parent_path == Marlin.SMB_URI) {
             parent_path = parent_path + Path.DIR_SEPARATOR_S;
         }
@@ -68,7 +65,7 @@
         }
         sb.erase (last_separator, -1);
         string parent_path = sb.str + Path.DIR_SEPARATOR_S;
-        return parent_path;
+        return sanitize_path (parent_path);
     }
 
     public bool path_has_parent (string new_path) {
@@ -145,7 +142,7 @@
     **/
     public void split_protocol_from_path (string path, out string protocol, out string new_path) {
         protocol = "";
-        new_path = path;
+        new_path = path.dup ();
 
         string[] explode_protocol = new_path.split ("://");
         if (explode_protocol.length > 2) {
@@ -155,12 +152,18 @@
         if (explode_protocol.length > 1) {
             if (explode_protocol[0] == "mtp") {
                 string[] explode_path = explode_protocol[1].split ("]", 2);
-                protocol = (explode_protocol[0] + "://" + explode_path[0] + "]").replace ("///", "//");
-                /* If path is being manually edited there may not be "]" so explode_path[1] may be null*/
-                new_path = explode_path [1] ?? "";
+                if (explode_path[0] != null && explode_path[0].has_prefix ("[")) {
+                    protocol = (explode_protocol[0] + "://" + explode_path[0] + "]").replace ("///", "//");
+                    /* If path is being manually edited there may not be "]" so explode_path[1] may be null*/
+                    new_path = explode_path [1] ?? "";
+                } else {
+                    warning ("Invalid mtp path");
+                    protocol = new_path.dup ();
+                    new_path = "/";
+                }
             } else {
                 protocol = explode_protocol[0] + "://";
-                new_path = explode_protocol[1];
+                new_path = explode_protocol[1] ?? "";
             }
         } else {
             protocol = Marlin.ROOT_FS_URI;
@@ -173,4 +176,17 @@
             new_path = Path.DIR_SEPARATOR_S + new_path;
         }
     }
+
+    private bool valid_mtp_uri (string uri) {
+        if (!uri.contains (Marlin.MTP_URI)) {
+            return false;
+        }
+        string[] explode_protocol = uri.split ("://",2);
+        if (explode_protocol.length != 2 ||
+            !explode_protocol[1].has_prefix ("[") ||
+            !explode_protocol[1].contains ("]")) {
+            return false;
+        }
+        return true;
+    }
 }

=== modified file 'libwidgets/MimeActions.vala'
--- libwidgets/MimeActions.vala	2015-10-09 11:12:36 +0000
+++ libwidgets/MimeActions.vala	2015-11-22 18:20:03 +0000
@@ -75,12 +75,13 @@
         return app;
     }
 
-    public static List<AppInfo>? get_applications_for_file (GOF.File file) {
+    public static List<AppInfo> get_applications_for_file (GOF.File file) {
+        List<AppInfo> result = null;
         string? type = file.get_ftype ();
         if (type == null)
-            return null;
+            return result;
 
-        List<AppInfo> result = AppInfo.get_all_for_type (type);
+        result = AppInfo.get_all_for_type (type);
         string uri_scheme = file.location.get_uri_scheme ();
 
         if (uri_scheme != null) {
@@ -98,7 +99,7 @@
         return result;
     }
 
-    public static List<AppInfo>? get_applications_for_folder (GOF.File file) {
+    public static List<AppInfo> get_applications_for_folder (GOF.File file) {
         List<AppInfo> result = AppInfo.get_all_for_type (ContentType.get_mime_type ("inode/directory"));
         string uri_scheme = file.location.get_uri_scheme ();
 
@@ -110,10 +111,9 @@
         }
 
         if (!file_has_local_path (file))
-            filter_non_uri_apps (result);
+            result = filter_non_uri_apps (result);
 
         result.sort (application_compare_by_name);
-
         return result;
     }
 
@@ -189,17 +189,14 @@
         return strcmp (a.get_id (), b.get_id ());
     }
 
-    private static void filter_non_uri_apps (List<AppInfo> apps) {
-         List<AppInfo> non_uri_apps = null;
-
+    private static List<AppInfo> filter_non_uri_apps (List<AppInfo> apps) {
+         List<AppInfo> uri_apps = null;
         foreach (var app in apps) {
-            if (!app.supports_uris ())
-                non_uri_apps.append (app);
-        }
-
-        foreach (var app in non_uri_apps) {
-            apps.remove (app);
-        }
+            if (app.supports_uris ()) {
+                uri_apps.append (app);
+            }
+        }
+        return uri_apps;
     }
 
     private static List<AppInfo> intersect_application_lists (List<AppInfo> a, List<AppInfo> b) {

=== modified file 'libwidgets/View/BreadcrumbsEntry.vala'
--- libwidgets/View/BreadcrumbsEntry.vala	2015-11-15 17:46:29 +0000
+++ libwidgets/View/BreadcrumbsEntry.vala	2015-11-22 18:20:03 +0000
@@ -20,8 +20,11 @@
 
 namespace Marlin.View.Chrome {
     public class BreadcrumbsEntry : BasicBreadcrumbsEntry {
+        /** Breadcrumb context menu support **/
+        ulong files_menu_dir_handler_id = 0;
+        Gtk.Menu menu;
+
         /** Completion support **/
-        ulong files_menu_handler_id = 0;
         GOF.Directory.Async? current_completion_dir = null;
         string completion_text = "";
         bool autocompleted = false;
@@ -360,8 +363,9 @@
     /****************************/
         private void load_right_click_menu (Gdk.EventButton event, BreadcrumbElement clicked_element) {
             string path = get_path_from_element (clicked_element);
-            GLib.File loc = File.new_for_commandline_arg (path);
-            GLib.File? root = loc.get_parent ();
+            GLib.File loc = PF.FileUtils.get_file_for_path (path);
+            string parent_path = PF.FileUtils.get_parent_path_from_path (path);
+            GLib.File? root = PF.FileUtils.get_file_for_path (parent_path);
 
             var style_context = get_style_context ();
             var padding = style_context.get_padding (style_context.get_state ());
@@ -371,29 +375,34 @@
                 menu_x_root = event.x_root - event.x + clicked_element.x - BREAD_SPACING;
 
             menu_y_root = event.y_root - event.y + get_allocated_height () - padding.bottom - padding.top;
-            var menu = new Gtk.Menu ();
-            menu.cancel.connect (() => { reset_elements_states (); });
-            menu.deactivate.connect (() => { reset_elements_states (); });
+
+            menu = new Gtk.Menu ();
+            menu.cancel.connect (() => {reset_elements_states ();});
+            menu.deactivate.connect (() => {reset_elements_states ();});
+
             build_base_menu (menu, loc);
-
             if (root != null) {
-                var files_menu = GOF.Directory.Async.from_gfile (root);
-                files_menu_handler_id = files_menu.done_loading.connect (() => {
-                    append_subdirectories (menu, files_menu);
+                var files_menu_dir = GOF.Directory.Async.from_gfile (root);
+                files_menu_dir_handler_id = files_menu_dir.done_loading.connect (() => {
+                    append_subdirectories (menu, files_menu_dir);
+                    files_menu_dir.disconnect (files_menu_dir_handler_id);
                 });
-                files_menu.load ();
+                files_menu_dir.load ();
+            } else {
+                warning ("Root directory null for %s", path);
             }
 
+            menu.show_all ();
             menu.popup (null,
                         null,
                         right_click_menu_position_func,
                         0,
-                        Gtk.get_current_event_time ());
+                        event.time);
         }
 
         private void build_base_menu (Gtk.Menu menu, GLib.File loc) {
             /* First the "Open in new tab" menuitem is added to the menu. */
-            var path = loc.get_path ();
+            var path = loc.get_uri ();
             var menuitem_newtab = new Gtk.MenuItem.with_label (_("Open in New Tab"));
             menu.append (menuitem_newtab);
             menuitem_newtab.activate.connect (() => {                
@@ -409,17 +418,12 @@
 
             menu.append (new Gtk.SeparatorMenuItem ());
 
-            /* Then the "Open with" menuitem is added to the menu. */
-            var menu_open_with = new Gtk.MenuItem.with_label (_("Open with"));
-            menu.append (menu_open_with);
             var submenu_open_with = new Gtk.Menu ();
-            menu_open_with.set_submenu (submenu_open_with);
-
             var root = GOF.File.get (loc);
             var app_info_list = Marlin.MimeActions.get_applications_for_folder (root);
             bool at_least_one = false;
             foreach (AppInfo app_info in app_info_list) {
-                if (app_info.get_executable () != Environment.get_application_name ()) {
+                if (app_info != null && app_info.get_executable () != Environment.get_application_name ()) {
                     at_least_one = true;
                     var menu_item = new Gtk.ImageMenuItem.with_label (app_info.get_name ());
                     menu_item.set_data ("appinfo", app_info);
@@ -436,8 +440,11 @@
                     submenu_open_with.append (menu_item);
                 }
             }
-
             if (at_least_one) {
+                /* Then the "Open with" menuitem is added to the menu. */
+                var menu_open_with = new Gtk.MenuItem.with_label (_("Open with"));
+                menu.append (menu_open_with);
+                menu_open_with.set_submenu (submenu_open_with);
                 submenu_open_with.append (new Gtk.SeparatorMenuItem ());
             }
 
@@ -451,22 +458,22 @@
         }
 
         private void append_subdirectories (Gtk.Menu menu, GOF.Directory.Async dir) {
-            menu.append (new Gtk.SeparatorMenuItem ());
-
             /* Append list of directories at the same level */
             unowned List<GOF.File>? sorted_dirs = dir.get_sorted_dirs ();
-            foreach (var gof in sorted_dirs) {
-                var menuitem = new Gtk.MenuItem.with_label(gof.get_display_name ());
-                menuitem.set_data ("location", gof.uri);
-                menu.append (menuitem);
-                menuitem.activate.connect (() => {
-                    text = menu.get_active ().get_data ("location");
-                    activate ();
-                });
+            if (sorted_dirs.length () > 0) {
+                menu.append (new Gtk.SeparatorMenuItem ());
+                foreach (var gof in sorted_dirs) {
+                    var menuitem = new Gtk.MenuItem.with_label(gof.get_display_name ());
+                    menuitem.set_data ("location", gof.uri);
+                    menu.append (menuitem);
+                    menuitem.activate.connect (() => {
+                        text = menu.get_active ().get_data ("location");
+                        activate ();
+                    });
+                }
             }
             menu.show_all ();
             /* Release the Async directory as soon as possible */
-            dir.disconnect (files_menu_handler_id);
             dir.cancel ();
             dir = null;
         }