Should be easy to fix :)

Okay, will fix it.

Created attachment 60601 [details] [review]
Proposed fix

This patch adds a confirmation dialog (discussed with chpe in #epiphany) that is transient for the ephy window.

Note that this patch includes new strings, so it should not be committed before string freeze is over.

chpe, can I commit this patch? I vaguely remember complaints about using gtk_dialog_run(), but personally I don't think it's a problem :)

It shouldn't be too hard to go async, the add-sidebar dialogue already does that. I'd like to stamp out gtk_dialog_run whereever possible! :)

I'm sorry, but my C coding skills are not sufficient to get this done in a clean way, since it involves adding structs (for the g_signal_connect user data that only takes 2 arguments, but we need 2: *extension and *removeNode) and other stuff I don't want to get my hands dirty on.

Rumour has it that chpe is volunteering to fix this bug! :)

Hi, I made a patch using the async method asked by Christian.
I tested and it works great for me.

Created attachment 92047 [details] [review]
Proposed fix using async methods

Thanks for the patch!

+struct RemoveCallbackData {
+	EphyNode *removeNode;
+	EphySidebarExtension *extension;
+};

I'd rather store the page_id string (g_strdup'd of course) and get the corresponding EphyNode only when actually removing it. That way we make sure it's not deleted in some other way before, which would cause a crash. 

Otherwise the patch looks good.

Created attachment 92073 [details] [review]
proposed fix using async methods - modified using chpe's suggestions

Ok, I also think it is better this way. The attached patch is modified following your suggestions!

struct RemoveCallbackData {
	const char *page_id;

|char *page_id|, no |const|.

And in free_remove_data you need to g_free (data->page_id).

Created attachment 92113 [details] [review]
proposed fix

oops...you're right!

+struct RemoveCallbackData {
+	const char *page_id;

You still need to remove that const. With that fixed, ok to commit; thanks!

Created attachment 92115 [details] [review]
fixed const declaration

Right, I must have been still half sleepy while doing that and i missed the const :( 
Now it should be finally ok. Thanks for the directions and the help!

2007-07-21  Wouter Bolsterlee  <wbolster@svn.gnome.org>

    * extensions/sidebar/ephy-sidebar-extension.c:
    (remove_dialog_response_cb), (free_remove_data),
    (sidebar_page_remove_requested_cb), (extension_weak_notify_cb),
    (dialog_weak_notify_cb), (ephy_sidebar_extension_add_sidebar_cb):

    Show delete confirmation dialog in sidebar extension.
    Original patch by Wouter Bolsterlee, updated and
    improved by Cosimo Cecchi. Fixes bug #333301.