Merge lp:~jeremywootten/pantheon-files/fix-1631982-run-as-admin-warning into lp:~elementary-apps/pantheon-files/trunk

[Jeremy Wootten]

When running as admin, show info screen and change headerbar color

Show warning info bar when running as administrator.
Remove "as Administrator" from tab label.

The exact text of the warning to be decided.

[Zisu Andrei (matzipan)]

Linked 3 more relevant issues. Low hanging fruit, I think.

[Jeremy Wootten (jeremywootten)]

Initial attempt at addressing bugs 1006000 and 1353974. Needs input from UX team

[Zisu Andrei (matzipan)]

The yellow sidebar is different enough. I was thinking we could just do with a darker gray.

The infoscreen however, looks a bit out of place. Perhaps just a simple confirmation dialog with a Granite.AlertView.

I don't know what it's called, but I can describe the behavior. You know when you have a popover/dialog which opens up above your main window and it turns your main window insensitive until you interact with the dialog? Is that possible with Gtk?

review: Needs Fixing

[Zisu Andrei (matzipan)]

Looking good. Just tiny comment about the "WARNING" label inline.

review: Needs Fixing

[Jeremy Wootten (jeremywootten)]

Fixed tab label string.

[Danielle Foré (danrabbit)]

lgtm :)

review: Approve (ux)

[lp:~jeremywootten/pantheon-files/fix-1631982-run-as-admin-warning]

2363. By Jeremy Wootten on 2017-01-16

Merge trunk to r2445

2364. By Jeremy Wootten on 2017-01-16

Fix regression in sidebar sensitivity on tab change

2365. By Jeremy Wootten on 2017-02-04

Merge changes from parent

[Dieter Debast (ddieter)]

I left some diff comments. Please correct me if I'm wrong.

review: Needs Fixing

[lp:~jeremywootten/pantheon-files/fix-1631982-run-as-admin-warning]

2366. By Jeremy Wootten on 2017-06-09

Merge trunk to r2575 and resolve conflicts

2367. By Jeremy Wootten on 2017-06-09

Use empty array rather than null for absent commandline parameters

2368. By Jeremy Wootten on 2017-06-09

Fix indentation

[Jeremy Wootten (jeremywootten)]

Dieter: Thanks for the review. The reason a local variable is assigned to the Files[] parameter in a couple of places is so that it can be used in the following closure. However, I have changed the code a little so that an empty array is used when there are no commandline parameters as this feels better than using a null and fixes conflicting types.

I have also fixed the indentation and updated the branch so that it does not conflict with current trunk.

There seems to be a problem using commandline parameters with pantheon-files-pkexec anyway - they are not being passed to the application or are ignored at some point. I need to fix this as it does not occur in trunk. Putting back to "In progress" for now.

Unmerged revisions

2368. By Jeremy Wootten on 2017-06-09

Fix indentation

2367. By Jeremy Wootten on 2017-06-09

Use empty array rather than null for absent commandline parameters

2366. By Jeremy Wootten on 2017-06-09

Merge trunk to r2575 and resolve conflicts

2365. By Jeremy Wootten on 2017-02-04

Merge changes from parent

2364. By Jeremy Wootten on 2017-01-16

Fix regression in sidebar sensitivity on tab change

2363. By Jeremy Wootten on 2017-01-16

Merge trunk to r2445

2362. By Jeremy Wootten on 2016-12-03

Hide sidebar and tabs when showing admin warning; do not nullify current_tab when removing admin warning; do not restore user tabs when root

2361. By Jeremy Wootten on 2016-12-03

Merge trunk to r2390

2360. By Jeremy Wootten on 2016-11-17

Remove unnecessary capitals and exclamation mark

2359. By Jeremy Wootten on 2016-11-12

Use AlertView, make sidebar insensitive, work with commandline directory parameters

=== modified file 'data/pantheon-files-pkexec.cmake'
--- data/pantheon-files-pkexec.cmake	2017-02-15 14:00:30 +0000
+++ data/pantheon-files-pkexec.cmake	2017-06-09 11:40:39 +0000
@@ -1,2 +1,2 @@
 #!/bin/sh
-pkexec "@CMAKE_INSTALL_PREFIX@/bin/pantheon-files"
\ No newline at end of file
+pkexec "@CMAKE_INSTALL_PREFIX@/bin/pantheon-files"

=== modified file 'src/Application.vala'
--- src/Application.vala	2017-06-04 11:12:44 +0000
+++ src/Application.vala	2017-06-09 11:40:39 +0000
@@ -196,12 +196,12 @@
             }
         }
 
-        File[] files = null;
+        File[] files = {};
 
         /* Convert remaining arguments to GFiles */
         foreach (string filepath in remaining) {
             string path = PF.FileUtils.sanitize_path (filepath, null);
-            GLib.File? file = null;
+            File? file = null;
 
             if (path.length > 0) {
                 file = File.new_for_uri (PF.FileUtils.escape_uri (path));
@@ -281,13 +281,28 @@
                                    GOF.Preferences.get_default (), "clock-format", GLib.SettingsBindFlags.GET);
     }
 
-    private void open_windows (File[]? files) {
-        if (files == null)
-            open_tabs (null); /* open_tabs () will restore saved tabs or default tab depending on preference */ 
+    private void open_windows (File[] files) {
+        if (files.length == 0)
+            open_tabs (files); /* open_tabs () will restore saved tabs or default tab depending on preference */ 
         else {
-            /* Open windows with tab at each requested location. */
-            foreach (var file in files) {
-                create_window (file);
+            if (Posix.getuid () == 0) {
+                var warning_window = create_window (null);
+                Granite.Widgets.AlertView warning = warning_window.add_admin_warning_tab ();
+
+                File[] locations = files; /* Needed for closure */
+                warning.action_activated.connect (() => {
+                    /* Open windows with tab at each requested location. */
+                    foreach (var file in locations) {
+                        create_window (file);
+                    }
+
+                    warning_window.close ();
+                });
+            } else {
+                /* Open windows with tab at each requested location. */
+                foreach (var file in files) {
+                    create_window (file);
+                }
             }
         }
     }
@@ -347,9 +362,8 @@
         return win;
     }
 
-    private void open_tabs (File[]? files, Gdk.Screen screen = Gdk.Screen.get_default ()) {
+    private void open_tabs (File[] files, Gdk.Screen screen = Gdk.Screen.get_default ()) {
         Marlin.View.Window window = null;
-
         /* Get the first window, if any, else create a new window */
         if (windows_exist ()) {
             window = (this.get_windows ()).data as Marlin.View.Window;
@@ -360,7 +374,27 @@
                 return;
             }
         }
-        if (files == null) {
+
+        if (Posix.getuid () == 0) {
+            Granite.Widgets.AlertView warning = window.add_admin_warning_tab ();
+            File[] locations = files; /* Needed for closure */
+
+            warning.action_activated.connect (() => {
+                /* When as root only create tabs if location given on commandline.
+                 * Otherwise default (home) tab is shown */
+                if (locations.length > 0) {
+                    restore_or_create_tabs (locations, window);
+                }
+
+                window.remove_warning_tab (warning);
+            });
+        } else {
+            restore_or_create_tabs (files, window);
+        }
+    }
+
+    private void restore_or_create_tabs (File[] files, Marlin.View.Window window) {
+        if (files.length == 0) {
             /* Restore session if not root and settings allow */
             if (Posix.getuid () == 0 ||
                 !Preferences.settings.get_boolean ("restore-tabs") ||
@@ -372,8 +406,9 @@
             }
         } else {
             /* Open tabs at each requested location */
-            foreach (var file in files)
+            foreach (var file in files) {
                 window.add_tab (file, Marlin.ViewMode.PREFERRED);
+            }
         }
     }
 

=== modified file 'src/View/Widgets/TopMenu.vala'
--- src/View/Widgets/TopMenu.vala	2017-03-08 19:39:11 +0000
+++ src/View/Widgets/TopMenu.vala	2017-06-09 11:40:39 +0000
@@ -33,7 +33,11 @@
 
         public bool working {
             set {
-                location_bar.sensitive = !value;
+                var sensitive = !value;
+                location_bar.sensitive = sensitive;
+                view_switcher.sensitive = sensitive;
+                button_forward.sensitive = sensitive;
+                button_back.sensitive = sensitive;
             }
         }
 

=== modified file 'src/View/Window.vala'
--- src/View/Window.vala	2017-06-04 11:12:44 +0000
+++ src/View/Window.vala	2017-06-09 11:40:39 +0000
@@ -56,7 +56,7 @@
         private unowned UndoManager undo_manager;
         public Chrome.TopMenu top_menu;
         public Chrome.ViewSwitcher view_switcher;
-        public Gtk.InfoBar info_bar;
+        private Gtk.InfoBar info_bar;
         public Granite.Widgets.DynamicNotebook tabs;
         private Gtk.Paned lside_pane;
         public Marlin.Places.Sidebar sidebar;
@@ -101,8 +101,6 @@
             undo_manager = Marlin.UndoManager.instance ();
             construct_top_menu ();
             set_titlebar (top_menu);
-            construct_info_bar ();
-            show_infobar (!is_marlin_mydefault_fm ());
             construct_notebook ();
             construct_sidebar ();
             build_window ();
@@ -124,7 +122,15 @@
         private void build_window () {
             Gtk.Box window_box = new Gtk.Box(Gtk.Orientation.VERTICAL, 0);
             window_box.show();
+
+            if (Posix.getuid () == 0) {
+                apply_admin_theming ();
+            }
+
+            construct_info_bar ();
             window_box.pack_start(info_bar, false, false, 0);
+            show_infobar (!is_marlin_mydefault_fm ());
+
             window_box.pack_start(tabs, true, true, 0);
 
             lside_pane = new Gtk.Paned (Gtk.Orientation.HORIZONTAL);
@@ -230,6 +236,13 @@
             ((Gtk.Box)info_bar.get_content_area ()).add (bbox);
         }
 
+        private void apply_admin_theming () {
+            var warning_bar = new Gtk.InfoBar ();
+            warning_bar.message_type = Gtk.MessageType.WARNING;
+            /* Set headerbar to the appropriate colour for a warning, as determined by the current theme */
+            Granite.Widgets.Utils.set_color_primary (top_menu, warning_bar.get_style_context ().get_color (Gtk.StateFlags.NORMAL));
+        }
+
         private void connect_signals () {
             /*/
             /* Connect and abstract signals to local ones
@@ -303,16 +316,19 @@
             });
 
             tabs.close_tab_requested.connect ((tab) => {
-                var view_container = (tab.page as ViewContainer);
-                tab.restore_data = view_container.location.get_uri ();
-
-                /* If closing tab is current, set current_tab to null to ensure
-                 * closed ViewContainer is destroyed. It will be reassigned in tab_changed
-                 */
-                if (view_container == current_tab)
-                    current_tab = null;
-
-               view_container.close ();
+                if (tab.page is ViewContainer) {
+                    var view_container = (tab.page as ViewContainer);
+                    tab.restore_data = view_container.location.get_uri ();
+
+                    /* If closing tab is current, set current_tab to null to ensure
+                     * closed ViewContainer is destroyed. It will be reassigned in tab_changed
+                     */
+                    if (view_container == current_tab) {
+                        current_tab = null;
+                    }
+
+                   view_container.close ();
+                }
 
                 if (tabs.n_tabs == 1)
                     add_tab ();
@@ -343,11 +359,17 @@
             });
 
             sidebar.request_focus.connect (() => {
-                return !current_tab.locked_focus && !top_menu.locked_focus;
+                if (current_tab is ViewContainer) {
+                    return !current_tab.locked_focus && !top_menu.locked_focus;
+                } else {
+                    return !top_menu.locked_focus;
+                }
             });
 
             sidebar.sync_needed.connect (() => {
-                loading_uri (current_tab.uri);
+                if (current_tab is ViewContainer) {
+                    loading_uri (current_tab.uri);
+                }
             });
         }
 
@@ -414,10 +436,11 @@
         }
 
         private void show_infobar (bool val) {
-            if (val)
+            if (val) {
                 info_bar.show_all ();
-            else
+            } else {
                 info_bar.hide ();
+            }
         }
 
         public GOF.AbstractSlot? get_active_slot() {
@@ -437,6 +460,7 @@
             }
 
             ViewContainer? old_tab = current_tab;
+
             current_tab = (tabs.get_tab_by_index (offset)).page as ViewContainer;
 
             if (current_tab == null || old_tab == current_tab)
@@ -455,6 +479,39 @@
             loading_uri (current_tab.uri);
             current_tab.set_active_state (true, false); /* changing tab should not cause animated scrolling */
             top_menu.working = current_tab.is_frozen;
+            sidebar.sensitive = !current_tab.is_frozen;
+        }
+
+        public Granite.Widgets.AlertView add_admin_warning_tab () {
+            var content = new Granite.Widgets.AlertView (_("Files is Running as Administrator"),
+                                                         _("Altering or deleting system files could damage your system or render it unusable"),
+                                                         "dialog-warning");
+
+            content.show_action (_("Continue"));
+
+            var tab = new Granite.Widgets.Tab ("", null, content);
+            tab.label = _("Warning");
+            change_tab ((int)tabs.insert_tab (tab, -1));
+            tabs.current = tab;
+ 
+            set_warning_visible (true);
+
+            return content;
+        }
+
+        public void remove_warning_tab (Gtk.Widget content) {
+            set_warning_visible (false);
+            remove_tab (content);
+        }
+
+        private void set_warning_visible (bool warning_is_visible) {
+            /* Set widget properties according to whether admin warning view is showing */
+            top_menu.working = warning_is_visible;
+            sidebar.sensitive = !warning_is_visible;
+            sidebar.visible = !warning_is_visible;
+            sidebar.no_show_all = warning_is_visible;
+            tabs.show_tabs = !warning_is_visible;
+            tabs.no_show_all = warning_is_visible;
         }
 
         public void add_tab_by_uri (string uri, Marlin.ViewMode mode = Marlin.ViewMode.PREFERRED) {
@@ -553,8 +610,8 @@
             return !sidebar.has_bookmark (uri);
         }
 
-        public void remove_tab (ViewContainer view_container) {
-            actual_remove_tab (tabs.get_tab_by_widget (view_container as Gtk.Widget));
+        public void remove_tab (Gtk.Widget content) {
+            actual_remove_tab (tabs.get_tab_by_widget (content));
         }
 
         private void actual_remove_tab (Granite.Widgets.Tab tab) {
@@ -913,7 +970,9 @@
 
             foreach (var tab in tabs.tabs) {
                 current_tab = null;
-                (tab.page as Marlin.View.ViewContainer).close ();
+                if (tab.page is Marlin.View.ViewContainer) {
+                    (tab.page as Marlin.View.ViewContainer).close ();
+                }
             }
 
             this.destroy ();
@@ -1087,6 +1146,8 @@
             top_menu.can_go_forward = (current_tab.can_show_folder && current_tab.can_go_forward);
             top_menu.working = current_tab.is_loading;
 
+            sidebar.sensitive = !current_tab.is_loading;
+
             /* Update viewmode switch, action state and settings */
             var mode = current_tab.view_mode;
             view_switcher.mode = mode;