Instead of making new browser windows transient as soon as they are created, do ir later when handling the "run-as-modal" signal. This makes it easier to use the MiniBrowser to do tests involving multiple windows, and will be useful as well when multi-web-process mode is enabled for the GTK port.
Created attachment 220965 [details] Patch
Attachment 220965 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/MiniBrowser/gtk/BrowserWindow.c', '--commit-queue']" exit_code: 1 ERROR: Tools/MiniBrowser/gtk/BrowserWindow.c:273: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Tools/MiniBrowser/gtk/BrowserWindow.c:274: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Tools/MiniBrowser/gtk/BrowserWindow.c:706: Declaration has space between * and variable name in GtkWidget* window [whitespace/declaration] [3] ERROR: Tools/MiniBrowser/gtk/BrowserWindow.c:707: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 4 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 220969 [details] Patch
Attachment 220969 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/MiniBrowser/gtk/BrowserWindow.c', '--commit-queue']" exit_code: 1 ERROR: Tools/MiniBrowser/gtk/BrowserWindow.c:706: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 220970 [details] Patch
Comment on attachment 220970 [details] Patch I think we can simply make the browser windows independent to each other. If you open two windows, then close the parent, and run-as-modal is emitted on the child window, it will crash because the parent window has already been destroyed.
(In reply to comment #6) > (From update of attachment 220970 [details]) > I think we can simply make the browser windows independent to each other. If you open two windows, then close the parent, and run-as-modal is emitted on the child window, it will crash because the parent window has already been destroyed. Good point, though I think it is more correct to connect to the "destroy" signal of the parent window, and clear the pointer to it in the child window. This way closing parent windows before run-as-modal is emitted will not cause a crash, and the transient-for property is still set when parent windows do exist.
Created attachment 220973 [details] Patch
Comment on attachment 220973 [details] Patch I still think we could just make windows independent to each other, but if we really want to keep the parent window, I think it would be simpler to keep the pointer to the parent window in the BroserWindow struct and use g_object_add_weak_pointer to make sure the parent pointer is NULL when the parent is destroyed.
(In reply to comment #9) > (From update of attachment 220973 [details]) > I still think we could just make windows independent to each other, but if we really want to keep the parent window, I think it would be simpler to keep the pointer to the parent window in the BroserWindow struct and use g_object_add_weak_pointer to make sure the parent pointer is NULL when the parent is destroyed. IMHO it is more correct to try to make the windows set the transient-for when they are meant to be modal. Not that it will matter much for the MiniBrowser, but provided that the fix is easy, I am making the change you suggested to use g_object_add_weak_pointer() and re-upload.
Created attachment 220979 [details] Patch
Comment on attachment 220979 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220979&action=review > Tools/MiniBrowser/gtk/BrowserWindow.c:704 > +static void parentWindowDestroy(GtkWidget *parent_window, gpointer window) > +{ > + if (window) > + g_object_set_data(G_OBJECT(window), "transient-for", NULL); > +} You forgot to remove this.
Created attachment 220980 [details] Patch
(In reply to comment #12) > (From update of attachment 220979 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220979&action=review > > > Tools/MiniBrowser/gtk/BrowserWindow.c:704 > > +static void parentWindowDestroy(GtkWidget *parent_window, gpointer window) > > +{ > > + if (window) > > + g_object_set_data(G_OBJECT(window), "transient-for", NULL); > > +} > > You forgot to remove this. Certainly, good catch. I have already re-uploaded with this cleaned.
Comment on attachment 220980 [details] Patch Clearing flags on attachment: 220980 Committed r161864: <http://trac.webkit.org/changeset/161864>
All reviewed patches have been landed. Closing bug.