Last Comment Bug 296002 - drop-down menus fail when gtkmozembed is moved to different toplevel
: drop-down menus fail when gtkmozembed is moved to different toplevel
Status: NEW
:
Product: Core
Classification: Components
Component: Widget: Gtk (show other bugs)
: Trunk
: x86 Linux
: -- major with 3 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
https://bugzilla.mozilla.org/query.cgi
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-05-30 15:22 PDT by Christian Persch (GNOME) (away; not receiving bug mail)
Modified: 2011-12-01 12:56 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (6.63 KB, patch)
2005-05-30 15:24 PDT, Christian Persch (GNOME) (away; not receiving bug mail)
no flags Details | Diff | Review
updated patch (9.06 KB, patch)
2005-05-31 11:18 PDT, Christian Persch (GNOME) (away; not receiving bug mail)
mpgritti: review-
Details | Diff | Review

Description Christian Persch (GNOME) (away; not receiving bug mail) 2005-05-30 15:22:07 PDT
When moving a GtkMozEmbed to a different toplevel window, drop-down elements are
closed immediately after being popped up, making selection impossible.

Steps to reproduce:
(0) Start Epiphany
(1) Load https://bugzilla.mozilla.org/query.cgi
(2) Open another tab
(3) Pop up the "Summary:" drop-down
(4) Right-click the mozilla.org tab's tab label, and choose "Detach tab"
(5) Repeat step (3)

Expected results:
In steps (3) and (5), the drop-down opens and I can choose an option.

Actual results:
In step (3) and (5), the drop-down opens. But I can only choose an option in
step (3), since the drop-down immediately closes itself in step (5).

Analysis:
In widget/src/gtk2/nsWindow.cpp: nsWindow::NativeCreate with type
eWindowType_popup the toplevel the widget is _currently_ in is stored in
mTransientParent, and used later for adding a keyboard grab when opening the
drop-down. But the mTransientParent variable is not updated when the GtkMozEmbed
widget containing the web page owning the popup widget is moved to a different
toplevel, resulting in the keyboard grab being against the wrong window.

(The same thing probably applies in the eWindowType_dialog case, but I cannot
test it since I have no idea under what circumstances a widget of this type will
be created in an embedding scenario.)

Patch follows.
Comment 1 Christian Persch (GNOME) (away; not receiving bug mail) 2005-05-30 15:24:07 PDT
Created attachment 184879 [details] [diff] [review]
fix

We need to keep the mTransientParent variable updated when moving the parent
container to another toplevel.
Comment 2 Christian Persch (GNOME) (away; not receiving bug mail) 2005-05-31 02:33:17 PDT
Comment on attachment 184879 [details] [diff] [review]
fix

This crashes when the parent container is reparented after the popup is
destroyed. New patch coming up later.
Comment 3 Christian Persch (GNOME) (away; not receiving bug mail) 2005-05-31 11:18:21 PDT
Created attachment 184942 [details] [diff] [review]
updated patch

This patch fixes the crash, by disconnecting the signal from the parent
container when the popup is destroyed.
Comment 4 Marco Pesenti Gritti 2005-07-01 02:42:34 PDT
Comment on attachment 184942 [details] [diff] [review]
updated patch

Some comments:

- The approach looks correct to me. The toplevel window is not specified by the
caller but nsWindow is determining it, so I think it's nsWindow job to keep it
updated. Though, I'd like someone with more knowledge of the code to take a
look.

- While we are at it I think we should fix also the dialog case. We should
probably connect the signal just after get_toplevel and have SetToplevelWindow
deal with both cases.

+	     mParentContainer = parentMozContainer;

Would probably be cleaner to just store this as class member.

+  GtkWidget *toplevelCandidate = gtk_widget_get_toplevel (container);
+  GtkWidget *toplevel = GTK_WIDGET_TOPLEVEL (toplevelCandidate) ?
toplevelCandidate : nsnull;
+
+  window->SetTransientParent (GTK_WINDOW (toplevel));

Using something like if (GTK_WIDGET_TOPLEVEL (toplevel)) ... else... would be
probably more readable.
Comment 5 Sebastian Dröge 2005-10-03 05:07:21 PDT
Any progress on this bug?
Comment 6 Chris Lord [:cwiiis] 2006-02-21 19:25:09 PST
Again, any progress on this?
Comment 7 Sebastien Bacher 2006-04-01 14:36:04 PST
Ubuntu bug about that: https://launchpad.net/distros/ubuntu/+source/firefox/+bug/21753
Comment 8 Mike Hommey [:glandium] 2006-07-27 01:36:40 PDT
Again, any progress on this?
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-07-28 08:03:34 PDT
Marco minused the patch. Someone needs to submit a new patch.
Comment 10 Christian Persch (GNOME) (away; not receiving bug mail) 2008-02-23 06:24:18 PST
Comment on attachment 184942 [details] [diff] [review]
updated patch

This patch is also buggy;  see http://bugs.debian.org/432672  (they ship this patch in their xulrunner package).

Note You need to log in before you can comment on or make changes to this bug.