[seahorse/wip/nielsdg/fix-unlock-empty-keyring: 2/2] Place: Move away from GtkActionGroup

[Niels De Graef <nielsdg src gnome org>]

commit 8cee07aa2d73209176ef2625888cf7d4714c1035
Author: Niels De Graef <nielsdegraef gmail com>
Date:   Fri Feb 22 08:32:04 2019 +0100

    Place: Move away from GtkActionGroup
    
    GtkActionGroup was deprecated in favor of GAction, so use that instead.
    To provide the necessary information for the UI (such as translatable
    strings), we also provide a property of type `GMenuModel`, so that we
    don't have to care which acions are available on the SeahorsePlace or
    not.
    
    This also solves a regression where we were no longer showing a context
    menu in the sidebar if actions were available.

 common/place.vala               | 39 +++++++++++++++-----
 gkr/gkr-keyring-properties.vala |  4 +--
 gkr/gkr-keyring.vala            | 80 +++++++++++++++++++++++++----------------
 pgp/seahorse-gpgme-keyring.c    | 44 +++++++++++++++++------
 pgp/seahorse-server-source.c    | 52 ++++++++++++++++++---------
 pgp/seahorse-unknown-source.c   | 33 ++++++++++++++---
 pkcs11/pkcs11-token.vala        | 14 ++++++--
 src/sidebar.vala                | 18 ++++------
 ssh/source.vala                 | 10 +++++-
 9 files changed, 204 insertions(+), 90 deletions(-)
---
diff --git a/common/place.vala b/common/place.vala
index ac925fea..dc66e7dd 100644
--- a/common/place.vala
+++ b/common/place.vala
@@ -18,16 +18,37 @@
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
-namespace Seahorse {
+/**
+ * A SeahorsePlace is a collection of objects (passwords/keys/certificates/...).
+ * An example of this is a keyring.
+ */
+public interface Seahorse.Place : Gcr.Collection {
+    public abstract string label { owned get; set; }
+    public abstract string description { owned get; }
+    public abstract string uri { owned get; }
+    public abstract Icon icon { owned get; }
 
-public interface Place : Gcr.Collection {
-       public abstract string label { owned get; set; }
-       public abstract string description { owned get; }
-       public abstract string uri { owned get; }
-       public abstract GLib.Icon icon { owned get; }
-       public abstract Gtk.ActionGroup? actions { owned get; }
+    /**
+     * Returns the {@link GLib.Action}s that are defined for this Place,
+     * or null if none.
+     */
+    public abstract GLib.ActionGroup? actions { owned get; }
 
-       public abstract async bool load(GLib.Cancellable? cancellable) throws GLib.Error;
-}
+    /**
+     * Returns the prefix that is used for the actions in this.menu_model.
+     */
+    public abstract unowned string? action_prefix { get; }
+
+    /**
+     * Returns the menu of basic actions that apply specifically to
+     * this Place. Can be used for example to show a context menu.
+     */
+    public abstract MenuModel? menu_model { owned get; }
 
+    /**
+     * Loads the items in this Place so they are available.
+     *
+     * Returns false if an error occurre
+     */
+    public abstract async bool load(Cancellable? cancellable) throws GLib.Error;
 }
diff --git a/gkr/gkr-keyring-properties.vala b/gkr/gkr-keyring-properties.vala
index 20e75d0f..d384d711 100644
--- a/gkr/gkr-keyring-properties.vala
+++ b/gkr/gkr-keyring-properties.vala
@@ -68,12 +68,12 @@ public class Seahorse.Gkr.KeyringProperties : Gtk.Dialog {
 
     [GtkCallback]
     private void on_set_default_button_clicked(Gtk.Button button) {
-        this.keyring.on_keyring_default(null);
+        this.keyring.set_as_default();
     }
 
     [GtkCallback]
     private void on_change_pw_button_clicked(Gtk.Button button) {
-        this.keyring.on_keyring_password(null);
+        this.keyring.change_password();
     }
 
     private void update_lock_unlock_button() {
diff --git a/gkr/gkr-keyring.vala b/gkr/gkr-keyring.vala
index 49f94f95..e829edba 100644
--- a/gkr/gkr-keyring.vala
+++ b/gkr/gkr-keyring.vala
@@ -22,6 +22,12 @@ namespace Seahorse {
 namespace Gkr {
 
 public class Keyring : Secret.Collection, Gcr.Collection, Place, Deletable, Lockable, Viewable {
+
+    private const ActionEntry[] KEYRING_ACTIONS = {
+        { "set-default",     on_action_set_default },
+        { "change-password", on_action_change_password },
+    };
+
        public string description {
                owned get {
                        if (Backend.instance().has_alias ("login", this))
@@ -40,13 +46,20 @@ public class Keyring : Secret.Collection, Gcr.Collection, Place, Deletable, Lock
        public GLib.Icon icon {
                owned get { return new GLib.ThemedIcon("folder"); }
        }
-       public Gtk.ActionGroup? actions {
-               owned get {
-                       if (this._actions == null)
-                               this._actions = create_actions();
-                       return this._actions;
-               }
-       }
+
+    private GLib.ActionGroup? _actions = null;
+    public GLib.ActionGroup? actions {
+        owned get { return this._actions; }
+    }
+
+    public unowned string? action_prefix {
+        get { return "gkr-keyring"; }
+    }
+
+    private MenuModel? _menu_model = null;
+    public MenuModel? menu_model {
+        owned get { return this._menu_model; }
+    }
 
        public bool is_default {
                get { return Backend.instance().has_alias ("default", this); }
@@ -65,7 +78,6 @@ public class Keyring : Secret.Collection, Gcr.Collection, Place, Deletable, Lock
        }
 
        private GLib.HashTable<string, Item> _items;
-       private Gtk.ActionGroup? _actions;
 
        construct {
                this._items = new GLib.HashTable<string, Item>(GLib.str_hash, GLib.str_equal);
@@ -77,6 +89,9 @@ public class Keyring : Secret.Collection, Gcr.Collection, Place, Deletable, Lock
                        notify_property ("is-default");
                        notify_property ("description");
                });
+
+        this._actions = create_actions();
+        this._menu_model = create_menu_model();
        }
 
        public uint get_length() {
@@ -161,8 +176,11 @@ public class Keyring : Secret.Collection, Gcr.Collection, Place, Deletable, Lock
                }
        }
 
-       [CCode (instance_pos = -1)]
-       public void on_keyring_default(Gtk.Action? action) {
+    public void on_action_set_default(SimpleAction action, Variant? param) {
+        set_as_default();
+    }
+
+    public void set_as_default() {
         var parent = null;
                var service = this.service;
 
@@ -174,10 +192,13 @@ public class Keyring : Secret.Collection, Gcr.Collection, Place, Deletable, Lock
                                Util.show_error(parent, _("Couldn’t set default keyring"), err.message);
                        }
                });
-       }
+    }
+
+    public void on_action_change_password(SimpleAction action, Variant? param) {
+        change_password();
+    }
 
-       [CCode (instance_pos = -1)]
-       public void on_keyring_password (Gtk.Action? action) {
+    public void change_password() {
         var parent = null;
                var service = this.service;
                service.get_connection().call.begin(service.get_name(),
@@ -203,29 +224,26 @@ public class Keyring : Secret.Collection, Gcr.Collection, Place, Deletable, Lock
                                Util.show_error(parent, _("Couldn’t change keyring password"), err.message);
                        }
                });
-       }
-
-       private const Gtk.ActionEntry[] KEYRING_ACTIONS = {
-               { "keyring-default", null, N_("_Set as default"), null,
-                 N_("Applications usually store new passwords in the default keyring."), on_keyring_default 
},
-               { "keyring-password", null, N_("Change _Password"), null,
-                 N_("Change the unlock password of the password storage keyring"), on_keyring_password },
-       };
+    }
 
-       private Gtk.ActionGroup create_actions() {
-               Gtk.ActionGroup actions = new Gtk.ActionGroup("KeyringActions");
+    private SimpleActionGroup create_actions() {
+        var group = new SimpleActionGroup();
+        group.add_action_entries (KEYRING_ACTIONS, this);
 
-               /* Add these actions, but none of them are visible until cloned */
-               actions.set_translation_domain(Config.GETTEXT_PACKAGE);
-               actions.add_actions(KEYRING_ACTIONS, this);
+        var action = group.lookup_action("set-default");
+        bind_property("is-default", action, "enabled", BindingFlags.INVERT_BOOLEAN);
 
-               var action = actions.get_action("keyring-default");
-               this.bind_property("is-default", action, "sensitive",
-                                  GLib.BindingFlags.INVERT_BOOLEAN | GLib.BindingFlags.SYNC_CREATE);
+        return group;
+    }
 
-               return actions;
-       }
+    private GLib.Menu create_menu_model() {
+        var menu = new GLib.Menu();
+        unowned string prefix = this.action_prefix;
 
+        menu.insert(0, _("_Set as default"),  prefix + ".set-default");
+        menu.insert(1, _("Change _Password"), prefix + ".change-password");
+        return menu;
+    }
 }
 
 class KeyringDeleter : Deleter {
diff --git a/pgp/seahorse-gpgme-keyring.c b/pgp/seahorse-gpgme-keyring.c
index af48a8aa..e942aa37 100644
--- a/pgp/seahorse-gpgme-keyring.c
+++ b/pgp/seahorse-gpgme-keyring.c
@@ -52,7 +52,9 @@ enum {
        PROP_DESCRIPTION,
        PROP_ICON,
        PROP_URI,
-       PROP_ACTIONS
+    PROP_ACTIONS,
+    PROP_ACTION_PREFIX,
+    PROP_MENU_MODEL
 };
 
 /* Amount of keys to load in a batch */
@@ -127,7 +129,7 @@ struct _SeahorseGpgmeKeyring {
        guint scheduled_refresh;                /* Source for refresh timeout */
        GFileMonitor *monitor_handle;           /* For monitoring the .gnupg directory */
        GList *orphan_secret;                   /* Orphan secret keys */
-       GtkActionGroup *actions;
+    GActionGroup *actions;
 };
 
 static void     seahorse_gpgme_keyring_place_iface        (SeahorsePlaceIface *iface);
@@ -770,13 +772,25 @@ seahorse_gpgme_keyring_get_icon (SeahorsePlace *place)
        return g_themed_icon_new (GCR_ICON_GNUPG);
 }
 
-static GtkActionGroup *
+static GActionGroup *
 seahorse_gpgme_keyring_get_actions (SeahorsePlace *place)
 {
-       SeahorseGpgmeKeyring *self = SEAHORSE_GPGME_KEYRING (place);
-       if (self->actions)
-               return g_object_ref (self->actions);
-       return NULL;
+    SeahorseGpgmeKeyring *self = SEAHORSE_GPGME_KEYRING (place);
+    if (self->actions)
+        return g_object_ref (self->actions);
+    return NULL;
+}
+
+static const gchar *
+seahorse_gpgme_keyring_get_action_prefix (SeahorsePlace* self)
+{
+    return NULL;
+}
+
+static GMenuModel *
+seahorse_gpgme_keyring_get_menu_model (SeahorsePlace *place)
+{
+    return NULL;
 }
 
 static gchar *
@@ -809,6 +823,12 @@ seahorse_gpgme_keyring_get_property (GObject *obj,
        case PROP_ACTIONS:
                g_value_take_object (value, seahorse_gpgme_keyring_get_actions (place));
                break;
+       case PROP_ACTION_PREFIX:
+               g_value_set_string (value, seahorse_gpgme_keyring_get_action_prefix (place));
+               break;
+       case PROP_MENU_MODEL:
+               g_value_take_object (value, seahorse_gpgme_keyring_get_menu_model (place));
+               break;
        default:
                G_OBJECT_WARN_INVALID_PROPERTY_ID (obj, prop_id, pspec);
                break;
@@ -839,8 +859,6 @@ seahorse_gpgme_keyring_dispose (GObject *object)
        SeahorseGpgmeKeyring *self = SEAHORSE_GPGME_KEYRING (object);
        GList *l;
 
-       if (self->actions)
-               gtk_action_group_set_sensitive (self->actions, TRUE);
        g_hash_table_remove_all (self->keys);
 
        cancel_scheduled_refresh (self);
@@ -890,7 +908,9 @@ seahorse_gpgme_keyring_class_init (SeahorseGpgmeKeyringClass *klass)
        g_object_class_override_property (gobject_class, PROP_DESCRIPTION, "description");
        g_object_class_override_property (gobject_class, PROP_URI, "uri");
        g_object_class_override_property (gobject_class, PROP_ICON, "icon");
-       g_object_class_override_property (gobject_class, PROP_ACTIONS, "actions");
+    g_object_class_override_property (gobject_class, PROP_ACTIONS, "actions");
+    g_object_class_override_property (gobject_class, PROP_ACTION_PREFIX, "action-prefix");
+    g_object_class_override_property (gobject_class, PROP_MENU_MODEL, "menu-model");
 }
 
 static void
@@ -898,7 +918,9 @@ seahorse_gpgme_keyring_place_iface (SeahorsePlaceIface *iface)
 {
        iface->load = seahorse_gpgme_keyring_load_async;
        iface->load_finish = seahorse_gpgme_keyring_load_finish;
-       iface->get_actions = seahorse_gpgme_keyring_get_actions;
+    iface->get_actions = seahorse_gpgme_keyring_get_actions;
+    iface->get_action_prefix = seahorse_gpgme_keyring_get_action_prefix;
+    iface->get_menu_model = seahorse_gpgme_keyring_get_menu_model;
        iface->get_description = seahorse_gpgme_keyring_get_description;
        iface->get_icon = seahorse_gpgme_keyring_get_icon;
        iface->get_label = seahorse_gpgme_keyring_get_label;
diff --git a/pgp/seahorse-server-source.c b/pgp/seahorse-server-source.c
index 38813da3..0c2b2282 100644
--- a/pgp/seahorse-server-source.c
+++ b/pgp/seahorse-server-source.c
@@ -49,7 +49,9 @@ enum {
     PROP_ICON,
     PROP_KEY_SERVER,
     PROP_URI,
-    PROP_ACTIONS
+    PROP_ACTIONS,
+    PROP_ACTION_PREFIX,
+    PROP_MENU_MODEL
 };
 
 /* -----------------------------------------------------------------------------
@@ -87,16 +89,12 @@ seahorse_server_source_class_init (SeahorseServerSourceClass *klass)
     gobject_class->set_property = seahorse_server_set_property;
     gobject_class->get_property = seahorse_server_get_property;
 
-    /* These properties are used to conform to SeahorseSource, but are not actually used */
-    g_object_class_install_property (gobject_class, PROP_LABEL,
-            g_param_spec_string ("label", "Label", "Label", "", G_PARAM_READWRITE));
-    g_object_class_install_property (gobject_class, PROP_DESCRIPTION,
-            g_param_spec_string ("description", "Description", "Description", "", G_PARAM_READABLE));
-    g_object_class_install_property (gobject_class, PROP_ICON,
-            g_param_spec_object ("icon", "icon", "Icon", G_TYPE_ICON, G_PARAM_READABLE));
-    g_object_class_install_property (gobject_class, PROP_ACTIONS,
-            g_param_spec_object ("actions", "actions", "Actions",
-                                 GTK_TYPE_ACTION_GROUP, G_PARAM_READABLE));
+       g_object_class_override_property (gobject_class, PROP_LABEL, "label");
+       g_object_class_override_property (gobject_class, PROP_DESCRIPTION, "description");
+       g_object_class_override_property (gobject_class, PROP_ICON, "icon");
+       g_object_class_override_property (gobject_class, PROP_ACTIONS, "actions");
+       g_object_class_override_property (gobject_class, PROP_ACTION_PREFIX, "action-prefix");
+       g_object_class_override_property (gobject_class, PROP_MENU_MODEL, "menu-model");
 
     g_object_class_install_property (gobject_class, PROP_KEY_SERVER,
             g_param_spec_string ("key-server", "Key Server",
@@ -185,10 +183,22 @@ seahorse_server_source_get_icon (SeahorsePlace* self)
        return g_themed_icon_new (NULL);
 }
 
-static GtkActionGroup *
+static GActionGroup *
 seahorse_server_source_get_actions (SeahorsePlace* self)
 {
-       return NULL;
+    return NULL;
+}
+
+static const gchar *
+seahorse_server_source_get_action_prefix (SeahorsePlace* self)
+{
+    return NULL;
+}
+
+static GMenuModel *
+seahorse_server_source_get_menu_model (SeahorsePlace* self)
+{
+    return NULL;
 }
 
 static void
@@ -196,7 +206,9 @@ seahorse_server_source_place_iface (SeahorsePlaceIface *iface)
 {
        iface->load = seahorse_server_source_load;
        iface->load_finish = seahorse_server_source_load_finish;
-       iface->get_actions = seahorse_server_source_get_actions;
+    iface->get_actions = seahorse_server_source_get_actions;
+    iface->get_action_prefix = seahorse_server_source_get_action_prefix;
+    iface->get_menu_model = seahorse_server_source_get_menu_model;
        iface->get_description = seahorse_server_source_get_description;
        iface->get_icon = seahorse_server_source_get_icon;
        iface->get_label = seahorse_server_source_get_label;
@@ -279,9 +291,15 @@ seahorse_server_get_property (GObject *obj,
        case PROP_ICON:
                g_value_take_object (value, seahorse_server_source_get_icon (place));
                break;
-       case PROP_ACTIONS:
-               g_value_set_object (value, seahorse_server_source_get_actions (place));
-               break;
+    case PROP_ACTIONS:
+        g_value_set_object (value, seahorse_server_source_get_actions (place));
+        break;
+    case PROP_ACTION_PREFIX:
+        g_value_set_string (value, seahorse_server_source_get_action_prefix (place));
+        break;
+    case PROP_MENU_MODEL:
+        g_value_set_object (value, seahorse_server_source_get_menu_model (place));
+        break;
        default:
                G_OBJECT_WARN_INVALID_PROPERTY_ID (obj, prop_id, pspec);
                break;
diff --git a/pgp/seahorse-unknown-source.c b/pgp/seahorse-unknown-source.c
index 1d36e3bc..fd0503e9 100644
--- a/pgp/seahorse-unknown-source.c
+++ b/pgp/seahorse-unknown-source.c
@@ -35,7 +35,9 @@ enum {
        PROP_DESCRIPTION,
        PROP_ICON,
        PROP_URI,
-       PROP_ACTIONS
+       PROP_ACTIONS,
+       PROP_ACTION_PREFIX,
+       PROP_MENU_MODEL
 };
 
 struct _SeahorseUnknownSource {
@@ -110,12 +112,24 @@ seahorse_unknown_source_get_icon (SeahorsePlace* self)
        return NULL;
 }
 
-static GtkActionGroup *
+static GActionGroup *
 seahorse_unknown_source_get_actions (SeahorsePlace* self)
 {
        return NULL;
 }
 
+static const gchar *
+seahorse_unknown_source_get_action_prefix (SeahorsePlace* self)
+{
+    return NULL;
+}
+
+static GMenuModel *
+seahorse_unknown_source_get_menu_model (SeahorsePlace* self)
+{
+       return NULL;
+}
+
 static void
 seahorse_unknown_source_get_property (GObject *obj,
                                       guint prop_id,
@@ -137,9 +151,15 @@ seahorse_unknown_source_get_property (GObject *obj,
        case PROP_ICON:
                g_value_take_object (value, seahorse_unknown_source_get_icon (place));
                break;
-       case PROP_ACTIONS:
-               g_value_take_object (value, seahorse_unknown_source_get_actions (place));
-               break;
+    case PROP_ACTIONS:
+        g_value_take_object (value, seahorse_unknown_source_get_actions (place));
+        break;
+    case PROP_ACTION_PREFIX:
+        g_value_set_string (value, seahorse_unknown_source_get_action_prefix (place));
+        break;
+    case PROP_MENU_MODEL:
+        g_value_take_object (value, seahorse_unknown_source_get_menu_model (place));
+        break;
        default:
                G_OBJECT_WARN_INVALID_PROPERTY_ID (obj, prop_id, pspec);
                break;
@@ -187,6 +207,8 @@ seahorse_unknown_source_class_init (SeahorseUnknownSourceClass *klass)
        g_object_class_override_property (gobject_class, PROP_DESCRIPTION, "description");
        g_object_class_override_property (gobject_class, PROP_ICON, "icon");
        g_object_class_override_property (gobject_class, PROP_ACTIONS, "actions");
+       g_object_class_override_property (gobject_class, PROP_ACTION_PREFIX, "action-prefix");
+       g_object_class_override_property (gobject_class, PROP_MENU_MODEL, "menu-model");
        g_object_class_override_property (gobject_class, PROP_URI, "uri");
 }
 
@@ -227,6 +249,7 @@ seahorse_unknown_source_place_iface (SeahorsePlaceIface *iface)
        iface->load = seahorse_unknown_source_load;
        iface->load_finish = seahorse_unknown_source_load_finish;
        iface->get_actions = seahorse_unknown_source_get_actions;
+       iface->get_menu_model = seahorse_unknown_source_get_menu_model;
        iface->get_description = seahorse_unknown_source_get_description;
        iface->get_icon = seahorse_unknown_source_get_icon;
        iface->get_label = seahorse_unknown_source_get_label;
diff --git a/pkcs11/pkcs11-token.vala b/pkcs11/pkcs11-token.vala
index c4a83c4e..a9494f05 100644
--- a/pkcs11/pkcs11-token.vala
+++ b/pkcs11/pkcs11-token.vala
@@ -100,9 +100,17 @@ public class Token : GLib.Object, Gcr.Collection, Place, Lockable {
                }
        }
 
-       public Gtk.ActionGroup? actions {
-               owned get { return null; }
-       }
+    public GLib.ActionGroup? actions {
+        owned get { return null; }
+    }
+
+    public unowned string? action_prefix {
+        get { return null; }
+    }
+
+    public MenuModel? menu_model {
+        owned get { return null; }
+    }
 
        public Flags object_flags {
                get { return 0; }
diff --git a/src/sidebar.vala b/src/sidebar.vala
index 0bf016bd..b9255e1d 100644
--- a/src/sidebar.vala
+++ b/src/sidebar.vala
@@ -42,7 +42,6 @@ public class Seahorse.Sidebar : Gtk.TreeView {
     private Gtk.TreePath? action_highlight_path;
     private Gtk.CellRendererPixbuf action_cell_renderer;
     private int action_button_size;
-    private Gtk.AccelGroup accel_group = new Gtk.AccelGroup();
 
     private uint update_places_sig;
 
@@ -626,16 +625,13 @@ public class Seahorse.Sidebar : Gtk.TreeView {
     }
 
     private void popup_menu_for_place(Place place) {
-        Gtk.Menu menu = new Gtk.Menu();
-
-        // First add all the actions from the collection
-        Gtk.ActionGroup actions = place.actions;
-        if (actions != null) {
-            foreach (weak Gtk.Action action in actions.list_actions()) {
-                action.set_accel_group(this.accel_group);
-                menu.append((Gtk.MenuItem) action.create_menu_item());
-            }
-        }
+        // Start from the menu model provided by the place (if any)
+        var menu = (place.menu_model != null)? new Gtk.Menu.from_model(place.menu_model)
+                                             : new Gtk.Menu();
+
+        // Make sure the actions from the collection
+        if (place.actions != null)
+            menu.insert_action_group(place.action_prefix, place.actions);
 
         // Lock and unlock items
         if (place is Lockable) {
diff --git a/ssh/source.vala b/ssh/source.vala
index f29df296..eb8ba554 100644
--- a/ssh/source.vala
+++ b/ssh/source.vala
@@ -55,7 +55,15 @@ public class Seahorse.Ssh.Source : GLib.Object, Gcr.Collection, Seahorse.Place {
         owned get { return new ThemedIcon(Gcr.ICON_HOME_DIRECTORY); }
     }
 
-    public Gtk.ActionGroup? actions {
+    public GLib.ActionGroup? actions {
+        owned get { return null; }
+    }
+
+    public unowned string? action_prefix {
+        get { return null; }
+    }
+
+    public MenuModel? menu_model {
         owned get { return null; }
     }