Required egg-editable-toolbar changes for extensions

[Adam Hooper <adamh densi com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

There's an item in our plan called "Extensions: Adding Toolbar Buttons".
Somebody really mean put 'Adam Hooper' as responsible ;). Obviously, I'm
too inexperienced to do this on my own. I've been staring at the
egg-editable-toolbar and asking Christian a few questions on IRC, and
I'd like to write down here what I've thought up, in the hopes of
getting some feedback to clear things up.

So if you have something to add or correct (marco! chpe! anybody!)
please do :).

- -----

Desired UI:

Right now, the toolbar has a fixed set of actions and toolbar buttons
associated with them. They are all loaded when Epiphany starts. We want
to add/remove custom buttons at any time extensions are loaded/unloaded
(at any time during normal operation). However, the user doesn't want to
*lose* those custom buttons forever when the extension providing the
button is unavailable. Imagine the following scenario:

1. User loads Epiphany and customizes his toolbar
2. User loads "Foo" extension
3. User adds "Bar" button (provided by "Foo" extenison) to his toolbar
4. User unloads "Foo" extension
5. User loads "Foo" extension

The button should show up in the exact same place on step 5 as it did on
step 3. This is imperative, since steps 4 and 5 will occur every time
Epiphany is restarted, and the toolbar must look the same across browser
restarts.

Conclusion: the XML file epiphany-toolbar.xml must store toolbar buttons
which are in non-loaded extensions.

- -----

Code

EggEditableToolbar already does pretty much all of what we need.
EggToolbarsModel lets us add items with nothing more than a name (the
GtkAction is only part of the actual GUI portion of the code).

First, loading the XML. In egg-toolbars-model.c, in parse_item_list()
(which will get called on startup, conceivably before extensions are
loaded [1]), there is a call to egg_toolbars_model_get_item_id(). It
will simply return a copy of the passed 'name' whether a valid action or
not. Would it be "good form" to load an id and name into the toolbars
model which may not even have an associated action for the duration of
the model's existence? If so, then the EggToolbarsModel will load fine
with no changes, as far as I can see.

Next, the toolbar will be constructed. This is in
egg-editable-toolbar.c, egg_editable_toolbar_construct(). Halfway down,
there is an "item = create_item (t, model, i, l, &action);".
create_item() in turn calls create_item_from_action() (with the action
name from the EggToolbarsModel), which will call find_action() with that
name. This will return NULL (since the action hasn't been created yet).
So the action will be removed from the EggToolbarsModel.

My proposed workaround is to simply *not* remove the action from the
EggToolbarsModel. Any time an extension is loaded/unloaded, call
egg_editable_toolbar_construct() again. Also, the EggToolbarEditor would
have to reload its icons when extensions are loaded/unloaded.
egg_editable_toolbar_construct() would need another temp variable to
keep track of position and a few lines would need to be deleted. Problem
solved.

Would this workaround break anything? Would it be considered "bad"?

After that, we'd need to change egg-toolbar-editor.c's
parse_item_list(). egg_toolbar_editor_load_actions() could take a "const
GtkUIManager *" instead of a "const char *xml_file". Instead of looking
for "<available>" it could instead look for a '<toolbar
name="AvailableToolbarButtons">' or something in epiphany-ui.xml. In the
interests of abstraction, this may mean extending EggToolbarEditor for
use in Epiphany.

Would these changes be all we need? I can't think of what I'm
forgetting; it seems to me like this would work!

- -----

Extensions code

Here's some pseudo-code for adding a toolbar button:

GtkActionEntry action_entries [] =
{
	{ "Bar", NULL, "Do Something", NULL, "Do something fun", G_CALLBACK
(my_callback) }
};
EphyWindow *window = get_ephy_window_somehow ();
EphyToolbarsModel *model = ephy_shell_get_toolbars_model ();

GtkUIManager *manager;
GtkActionGroup *group;
GtkAction *action;
guint merge_id;

manager = GTK_UI_MANAGER (window->ui_merge);
group = gtk_action_group_new ("FooExtensionActions");
gtk_action_group_add_actions (group, action_entries, 1, NULL);
gtk_ui_manager_insert_action_group (manager, group, 0);
g_object_unref (action_group);

merge_id = gtk_ui_manager_new_merge_id (manager);
gtk_ui_manager_add_ui (manager, merge_id,
"/AvailableToolbarButtons/Bar", "Bar", "Bar", GTK_UI_MANAGER_TOOLITEM,
FALSE);

... If this code is in the extension's impl_attach_window() and the
toolbar is recreated *after* impl_attach_window() is called, the toolbar
will be recreated to include "/AvailableToolbarButtons/Bar" if "Bar" is
in the EphyToolbarsModel; otherwise, it will show up in the
EggToolbarEditor.

On second thought, recreating toolbars for every extension load/unload
may be too much. But it would be straightforward to optimize this.

- -----

Obviously, I'm an uber-newbie. We all know this. Go ahead and laugh and
make fun, but please accompany the giggles with some advice. What am I
missing, what could I do better, do I have a clue at all? Please help :).

[1] Even if we loaded extensions before calling this parse_item_list(),
a user could still disable an extension, restart the browser then enable it.

- --
Adam Hooper
adamh densi com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFAsrb7D7ySSW2J9egRAhZdAJ4//3TehQ+nMR16AefNzCS32We9NACePbRL
m1cgCCCAQPHGEVeUeEM7jTU=
=oHYv
-----END PGP SIGNATURE-----