Code review comment for lp:~aauzi/midori/fix-1179624

> 8 + if (KATZE_ITEM_IS_FOLDER (item))
> 199 + {
> 200 + gint child_bookmarks_count = midori_array_count_recursive
> (browser->bookmarks,
> 201 + "uri <> ''", NULL, item, FALSE);
> 202 +
> 203 + if (!child_bookmarks_count)
> 204 + midori_browser_bookmark_popup_item (menu,
> 205 + STOCK_TAB_NEW, _("Open all in _Tabs"), item, NULL,
> browser);
> 206 + else
> 207 + midori_browser_bookmark_popup_item (menu,
> 208 + STOCK_TAB_NEW, _("Open all in _Tabs"),
> 209 + item,
> midori_browser_bookmark_open_in_tab_activate_cb, browser);
> 210 + }
>
> This part looks a bit strange.

hmm, I guess it's the coding style that looks unfamiliar.

Maybe I could rewrite it this way:

   if (KATZE_ITEM_IS_FOLDER (item))
   {
       gint child_bookmarks_count = midori_array_count_recursive (browser->bookmarks,
           "uri <> ''", NULL, item, FALSE);

       midori_browser_bookmark_popup_item (menu,
           STOCK_TAB_NEW, _("Open all in _Tabs"), item,
           (!child_bookmarks_count ? NULL : midori_browser_bookmark_open_in_tab_activate_cb),
           browser);
    }

The intent is to make the menu item insensitive when the folder does not contain bookmarks as suggested in : http://elementaryos.org/docs/widget-concepts.

It's related to the following change in midori_browser_bookmark_popup_item :
182 - g_signal_connect (menuitem, "activate", G_CALLBACK (callback), userdata);
183 + if (callback)
184 + g_signal_connect (menuitem, "activate", G_CALLBACK (callback), userdata);
185 + else
186 + gtk_widget_set_sensitive (menuitem, FALSE);