The embedding widget doesn't handle reparenting well. If you try to reparent it from one notebook to another it will create a new toplevel window. We need the hidden window again so we can unrealize and re-realize safely.

Can you guys review/super-review this?

Great, the patch is a huge improvement! There are still a few things I'm noticing: 1. I'm frequently, but not always, seeing segfaults when destroying an embed that's been reparented. 2. When I remove an embed from one notebook and add it to another, it seems to be placed a bit farther down on the notebook page than it should be. See http://prtr-13.ucsc.edu/~erat/temp/mozembed.png for a screenshot of this. This happened in 0.8 as well, I believe.

Chris, This works much better with SkipStone now, when reparenting an embed out of hte notebook into a toplevel no more top level window for itself. However, the crash that used to happen when reparenting the only embed in the notebook is now happening when you reparent an embed that was reparented once before into the notebook I think sorta like Daniel said.. Here is the backtrace, its gtk+ related so it must be coming from gtksuperwin internals is my guess. Steps to reproduce in SkipStone: 1) open a couple of notebook tabs 2) detab one (alt-d) 3) retab the same one (ctrl-r) Program received signal SIGSEGV, Segmentation fault. 0x4017f466 in gdk_window_get_size (window=0x39, width=0xbffff57c, height=0xbffff580) at gdkwindow.c:1395 1395 gdkwindow.c: No such file or directory. (gdb) bt #0 0x4017f466 in gdk_window_get_size (window=0x39, width=0xbffff57c, height=0xbffff580) at gdkwindow.c:1395 #1 0x401227f8 in gtk_widget_clip_rect (widget=0x8164028, window=0x8518a80, rect=0x82ed780, x_offset=0x0, y_offset=0x0) at gtkwidget.c:2019 #2 0x40122b02 in gtk_widget_idle_draw (cb_data=0x0) at gtkwidget.c:2112 #3 0x401a0803 in g_idle_dispatch (source_data=0x40122988, dispatch_time=0xbffff6e4, user_data=0x0) at gmain.c:1367 #4 0x4019f796 in g_main_dispatch (dispatch_time=0xbffff6e4) at gmain.c:656 #5 0x4019fda3 in g_main_iterate (block=1, dispatch=1) at gmain.c:877 #6 0x4019ff4c in g_main_run (loop=0x8303338) at gmain.c:935 #7 0x400bcb59 in gtk_main () at gtkmain.c:524 #8 0x8051c4f in main (argc=1, argv=0xbffff7e4) at main.c:102 (gdb)

OK, this patch fixes the problems reported with retabbing. There's still a problem with skipstone if you de-tab the only window that you have open but I suspect that's a problem with the somewhat frighting way that skipstone reparents windows or a problem with notebooks. Please prove me wrong and tell me it's my code. Anyway, I was able to drag tabs like nuts in galeon and detab to my heart's content in skipstone as long as it wasn't the last one. :)

The latest patch changes the order that we remove items in the Destroy() method. The old way would trigger state changes and location changes and you would end up emitting signals on the already destroyed widget.

EmbedPrivate.cpp:220: non-lvalue in unary `&' egcs 1.1.2 (yes, some people still use that, namely all on rh 6.2 :)

This is much better Chris, the stuff I had to do for detabbing that horrified you is becuae of moz 0.8 wierdness with detabbing, however even when I do remove that uneeded stuff anymore, I do get this crash but it sure looks like its Gtk+ related at the core, I can make a workaround I suppose by not allowing detabbing until you have 2 or more tabs, here is a backtrace: 1065 gtkcontainer.c: No such file or directory. (gdb) bt #0 gtk_container_resize_children (container=0x813b0c0) at gtkcontainer.c:1065 #1 0x4012d839 in gtk_window_move_resize (window=0x813b0c0) at gtkwindow.c:1803 #2 0x4012d191 in gtk_window_check_resize (container=0x813b0c0) at gtkwindow.c:1524 #3 0x400be63f in gtk_marshal_NONE__NONE (object=0x813b0c0, func=0x4012d0d4 <gtk_window_check_resize>, func_data=0x0, args=0xbffff33c) at gtkmarshal.c:312 #4 0x400ee87d in gtk_signal_real_emit (object=0x813b0c0, signal_id=57, params=0xbffff33c) at gtksignal.c:1492 #5 0x400ec837 in gtk_signal_emit (object=0x813b0c0, signal_id=57) at gtksignal.c:552 #6 0x40079e38 in gtk_container_check_resize (container=0x813b0c0) at gtkcontainer.c:928 #7 0x40079ad2 in gtk_container_idle_sizer (data=0x0) at gtkcontainer.c:847 #8 0x401a0803 in g_idle_dispatch (source_data=0x40079a6c, dispatch_time=0xbffff704, user_data=0x0) at gmain.c:1367 #9 0x4019f796 in g_main_dispatch (dispatch_time=0xbffff704) at gmain.c:656 #10 0x4019fda3 in g_main_iterate (block=1, dispatch=1) at gmain.c:877 #11 0x4019ff4c in g_main_run (loop=0x824ddc0) at gmain.c:935 #12 0x400bcb59 in gtk_main () at gtkmain.c:524 #13 0x8051bff in main (argc=1, argv=0xbffff804) at main.c:108 (gdb)

Yeah, that's the crash I was seeing. It's a crash on the new window that pops up which I thought was odd. It doesn't seem to be related to the widget at all. I would suggest trying reparenting with another non-mozilla kind of widget and see if it's still there.

I need more testing on this if possible. tor/bryner/pavlov can you guys review this patch for me? Thanks!

The latest patch appears to work perfectly here under Galeon. Thanks again! What are the chances of seeing it in 0.9.1? :)

Isn't danm the guy to look at this?

danm can come up to bat if he feels like it, sure. Come on down, Dan!

Out to 0.9.2 since I can't get a review to save my life.

r=bryner

sr=tor

a= asa@mozilla.org for checkin to the trunk. (on behalf of drivers)

Checked in.