Merge lp:~aauzi/midori/fix-1177553-2 into lp:midori

[André Auzi]

Take current selection into account for bookmark folders when adding/editing bookmark

in midori_bookmarks_add_clicked_cb of panels/midori-bookmarks.c:
 uses the current selection to find the folder a bookmark or folder is added into and passes it as a parameter of midori_browser_edit_bookmark_dialog_new

in midori_browser_edit_bookmark_dialog_new of midori/midori-browser.c:
  when a new bookmark or folder is created initializes its "parentid" meta data with the id of the given bookmark_or_parent parameter.
  call midori_bookmark_folder_button_new without the new_bookmark parameter.
  The parameter 'parentid' corresponds to the newly created or edited bookmarks "parentid" meta data.

in midori_bookmark_folder_button_new:
  remove the case 'new_bookmark, ie. select the given parentid in any case

in _action_bookmarks_import_activate:
  call midori_bookmark_folder_button_new without the new_bookmark parameter.
  The parameter 'parentid' corresponds to the "Bookmarks" (root) folder

[Cris Dywan (kalikiana)]

Looks sensible to me.

review: Approve

[Cris Dywan (kalikiana)]

I do notice something when creating a new folder:
GLib-CRITICAL **: g_str_has_prefix: assertion `str != NULL' failed

I'm uncertain if this is due to this change or not. I'd appreciate if somebody else can double-check.

[André Auzi (aauzi)]

Le Sat, 29 Jun 2013 18:36:26 -0000,
Christian Dywan <email address hidden> a écrit :

> I do notice something when creating a new folder:
> GLib-CRITICAL **: g_str_has_prefix: assertion `str != NULL' failed
>
> I'm uncertain if this is due to this change or not. I'd appreciate if
> somebody else can double-check.

Hi kalikiana,

I've checked lp:midori (revno: 6238), it does something very similar:

(midori4:8087): GLib-CRITICAL **: g_str_has_prefix: assertion `str !=
NULL' failed

[André Auzi (aauzi)]

> Le Sat, 29 Jun 2013 18:36:26 -0000,
> Christian Dywan <email address hidden> a écrit :
>
> > I do notice something when creating a new folder:
> > GLib-CRITICAL **: g_str_has_prefix: assertion `str != NULL' failed
> >
> > I'm uncertain if this is due to this change or not. I'd appreciate if
> > somebody else can double-check.
>
> Hi kalikiana,
>
> I've checked lp:midori (revno: 6238), it does something very similar:
>
> (midori4:8087): GLib-CRITICAL **: g_str_has_prefix: assertion `str !=
> NULL' failed

The problem actually comes from the midori_browser_update_history that's done in midori_browser_edit_bookmark_dialog_new.

It calls the function zeitgeist_manifestation_for_uri with the created folder's uri, which is a NULL string.

I wonder if midori_browser_update_history must be done in case of folder creation.

I wonder if it must be done in any case.

Here is the backtrace.

-------------------------------------------------------------------------------
env G_DEBUG=fatal-criticals _build/default/midori/midori -g

** (midori:8329): WARNING **: Couldn't connect to accessibility bus: Failed to connect to socket /tmp/dbus-06tODYGRbI: Connexion refusée
Launching command: '/usr/bin/gdb' --batch -ex 'set print thread-events off' -ex run -ex 'set logging on /run/user/1000/midori/gdb.bt' -ex 'bt' --return-child-result --args _build/default/midori/midori
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/libthread_db.so.1".
warning: cannot close "/usr/lib/gio/modules/libgvfsdbus.so": Opération invalide
warning: cannot close "/lib/libgvfscommon.so.0": Opération invalide
warning: cannot close "/lib/libbluray.so.1": Opération invalide

** (midori:8332): WARNING **: Couldn't connect to accessibility bus: Failed to connect to socket /tmp/dbus-06tODYGRbI: Connexion refusée
warning: cannot close "/usr/lib/gio/modules/libgiognomeproxy.so": Opération invalide
warning: cannot close "/usr/lib/gio/modules/libgiolibproxy.so": Opération invalide
warning: cannot close "/lib/libproxy.so.1": Opération invalide
warning: cannot close "/lib/libmodman.so.1": Opération invalide
warning: cannot close "/usr/lib/gio/modules/libgsettingsgconfbackend.so": Opération invalide
warning: cannot close "/lib/libgconf-2.so.4": Opération invalide
Detaching after fork from child process 8339.
warning: cannot close "/usr/lib/gio/modules/libgiognutls.so": Opération invalide
warning: cannot close "/lib/libgnutls.so.26": Opération invalide
warning: cannot close "/lib/libtasn1.so.3": Opération invalide
warning: cannot close "/lib/libfam.so.0": Opération invalide
Detaching after fork from child process 8345.
java version "1.7.0_03"
OpenJDK Runtime Environment (fedora-2.3.9.8.fc18-i386)
OpenJDK Client VM (build 23.7-b01, mixed mode, sharing)

(midori4:8332): Gtk-WARNING **: Error loading theme icon 'text-x-javascript' for stock: L'icône « text-x-javascript » n'est pas présente dans le thème

(midori4:8332): Gtk-WARNING **: Can't set a parent on a toplevel widget

(midori4:8332): GLib-CRITICAL **: g_str_has_prefix: assertion `str != NULL' failed

Program received signal SIGTRAP, Trace/breakpoint trap.
g_logv (log_domain=log_domain@entry=0x41ea5aae "...

[Cris Dywan (kalikiana)]

I'll propose a separate fix for the g_str_has_prefix issue. This is good to go then. Thanks!

review: Approve

=== modified file 'midori/midori-browser.c'
--- midori/midori-browser.c	2013-06-20 20:07:11 +0000
+++ midori/midori-browser.c	2013-06-20 21:18:27 +0000
@@ -818,7 +818,6 @@
 
 static GtkWidget*
 midori_bookmark_folder_button_new (KatzeArray* array,
-                                   gboolean    new_bookmark,
                                    gint64      selected,
                                    gint64      parentid)
 {
@@ -856,7 +855,7 @@
             gtk_list_store_insert_with_values (model, NULL, G_MAXINT,
                 0, name, 1, PANGO_ELLIPSIZE_END, 2, id, -1);
 
-            if (!new_bookmark && id == parentid)
+            if (id == parentid)
                 gtk_combo_box_set_active (GTK_COMBO_BOX (combo), n);
             n++;
         }
@@ -910,11 +909,12 @@
 /* Private function, used by MidoriBookmarks and MidoriHistory */
 /* static */ gboolean
 midori_browser_edit_bookmark_dialog_new (MidoriBrowser* browser,
-                                         KatzeItem*     bookmark,
+                                         KatzeItem*     bookmark_or_parent,
                                          gboolean       new_bookmark,
                                          gboolean       is_folder,
                                          GtkWidget*     proxy)
 {
+    KatzeItem*   bookmark = bookmark_or_parent;
     const gchar* title;
     GtkWidget* dialog;
     GtkWidget* content_area;
@@ -968,7 +968,7 @@
     gtk_window_set_icon_name (GTK_WINDOW (dialog),
         new_bookmark ? GTK_STOCK_ADD : GTK_STOCK_REMOVE);
 
-    if (!bookmark)
+    if (new_bookmark)
     {
         view = midori_browser_get_current_tab (browser);
         if (is_folder)
@@ -981,6 +981,11 @@
             bookmark = g_object_new (KATZE_TYPE_ITEM,
                 "uri", midori_view_get_display_uri (MIDORI_VIEW (view)),
                 "name", midori_view_get_display_title (MIDORI_VIEW (view)), NULL);
+        katze_item_set_meta_integer (
+            bookmark, "parentid",
+            (!bookmark_or_parent
+                ? 0
+                : katze_item_get_meta_integer (bookmark_or_parent, "id")));
     }
 
     entry_title = gtk_entry_new ();
@@ -1008,7 +1013,7 @@
     }
 
     combo_folder = midori_bookmark_folder_button_new (browser->bookmarks,
-        new_bookmark, katze_item_get_meta_integer (bookmark, "id"),
+        katze_item_get_meta_integer (bookmark, "id"),
         katze_item_get_meta_integer (bookmark, "parentid"));
     gtk_box_pack_start (GTK_BOX (vbox), combo_folder, FALSE, FALSE, 0);
 
@@ -4426,7 +4431,7 @@
     gtk_widget_show_all (hbox);
 
     combobox_folder = midori_bookmark_folder_button_new (browser->bookmarks,
-                                                         FALSE, 0, 0);
+                                                         0, 0);
     gtk_container_add (GTK_CONTAINER (content_area), combobox_folder);
 
     gtk_dialog_set_default_response (GTK_DIALOG (dialog), GTK_RESPONSE_ACCEPT);

=== modified file 'panels/midori-bookmarks.c'
--- panels/midori-bookmarks.c	2013-06-13 16:53:19 +0000
+++ panels/midori-bookmarks.c	2013-06-20 21:18:27 +0000
@@ -27,7 +27,7 @@
 
 gboolean
 midori_browser_edit_bookmark_dialog_new (MidoriBrowser* browser,
-                                         KatzeItem*     bookmark,
+                                         KatzeItem*     bookmark_or_parent,
                                          gboolean       new_bookmark,
                                          gboolean       is_folder,
                                          GtkWidget*     proxy);
@@ -390,14 +390,55 @@
 }
 
 static void
-midori_bookmarks_add_clicked_cb (GtkWidget* toolitem)
+midori_bookmarks_add_clicked_cb (GtkWidget* toolitem,
+				                 MidoriBookmarks* bookmarks)
 {
     MidoriBrowser* browser = midori_browser_get_for_widget (toolitem);
-    /* FIXME: Take selected folder into account */
+    GtkTreeView* treeview = GTK_TREE_VIEW (bookmarks->treeview);
+    GtkTreeModel* model;
+    GtkTreeIter iter;
+    KatzeItem* parent = NULL;
+
+    if (katze_tree_view_get_selected_iter (treeview,
+            &model, &iter))
+    {
+        gboolean done = FALSE;
+        while (!done)
+        {
+            gtk_tree_model_get (model, &iter, 0, &parent, -1);
+
+            if (KATZE_ITEM_IS_FOLDER (parent))
+            {
+                GtkTreePath* path = gtk_tree_model_get_path(model, &iter);
+
+                if (!gtk_tree_view_row_expanded (treeview, path))
+                    gtk_tree_view_expand_row (treeview, path, FALSE);
+
+                gtk_tree_path_free (path);
+                done = TRUE;
+            }
+            else
+            {
+                GtkTreeIter child = iter;
+
+                if (parent) g_object_unref (parent);
+                parent = NULL;
+
+                if (!gtk_tree_model_iter_parent (model, &iter, &child))
+                {
+                    done = TRUE;
+                }
+            }
+        }
+    }
+
     if (g_str_equal (gtk_widget_get_name (toolitem), "BookmarkFolderAdd"))
-        midori_browser_edit_bookmark_dialog_new (browser, NULL, TRUE, TRUE, toolitem);
+        midori_browser_edit_bookmark_dialog_new (browser, parent, TRUE, TRUE, toolitem);
     else
-        midori_browser_edit_bookmark_dialog_new (browser, NULL, TRUE, FALSE, toolitem);
+        midori_browser_edit_bookmark_dialog_new (browser, parent, TRUE, FALSE, toolitem);
+
+    if (parent)
+        g_object_unref (parent);
 }
 
 static void