Same happens with "gtk3-demo --run editable_cells". Moving to gtk+

I'm getting the same error with a ComboBox using a ListStore model, not editable - BUT not if I set wrap-width to >1 (as I do for most of my ComboBoxes in this app).

Perhaps related?

Created attachment 335315 [details]
backtrace after same critical error when popping up ComboBox with model and wrap-width = 1

fwiw, here is my bt from the above scenario.

In fact, for the simplest ever replication step, run any app with GTK_DEBUG=interactive and drop down the top-left ComboBox. So this seems to be a general thing.

Sounds like a duplicate of bug 771117

*** This bug has been marked as a duplicate of bug 771117 ***

Well, I don't know about the OP, but I'm not running Wayland, to which the supposed dupe seems to be specific.

Created attachment 335390 [details]
screencast of same critical warning occurring on X 1st time any CB is opened

Here it is happening on my X machine with the single-column ComboBoxes in the Inspector and Widget Factory.

It seems only to happen the 1st time the menu is popped up. And again, with most of my other CBs, which have wrap-width > 1, it does not occur.

Hope this helps.

OP, are you using X or Wayland when you get this error?

I'm ashamed to say I don't really know.

Is this enough to tell?

~$ ps aux | grep -i [w]ayland
Debian-+ 16857  0.0  0.0 189000  1424 tty1     Ssl+ set12   0:00 /usr/lib/gdm3/gdm-wayland-session gnome-session --autostart /usr/share/gdm/greeter/autostart
Debian-+ 16877  0.0  0.0 259712  5556 tty1     Sl+  set12   0:00 /usr/bin/Xwayland :1024 -rootless -noreset -listen 4 -listen 5 -displayfd 6

~$ ps aux | grep -i [x]org
andreas  16956  1.4  0.8 369232 71944 tty3     S+   set12  11:44 /usr/lib/xorg/Xorg vt3 -displayfd 3 -auth /run/user/1000/gdm/Xauthority -background none -noreset -keeptty -verbose 3

I just upgraded from GTK+ 3.21.5 to .6 and this critical warning is still present in the same scenarios.

Is this somehow not happening/replicable for anyone else? Either way, if I can provide any more info to help diagnose it, please let me know.


(In reply to andreastacchiotti from comment #9)
> I'm ashamed to say I don't really know.
> 
> Is this enough to tell?
> > [snip]

I get similar output, showing matches for both, but am running on X.org - so I'm not sure that tells either of us very much at all. (other than maybe Wayland is always used by gdm, which would be interesting)

the way to tell is either of
 * while logging in with gdm, open the cog menu to check whether it's going to run GNOME or GNOME (Wayland); the former means X.
 * once already logged in, you can try this: http://unix.stackexchange.com/questions/202891/how-to-know-whether-wayland-or-x11-is-being-used

I'll be interested to hear what you're running either way.

I confirm I run Xorg.

It's not surprising 3.21.6 doesn't fix the issue, as the seemingly related 771117 has been fixed 10 commits after the tag.

If you know how, you could try with the current master (or at least up to commit c529d0a96e688099c01f0c3fc1b9361f7875f9c8 ).

Thanks. Weird, I must've gotten mixed up with another report which I thought had landed prior to the 3.21.6 tag. I'll give master a try indeed!

...though looking at the commit you linked, I'm reminded of how unrelated it seems. given that it only touches a wayland-specific file, so I can't imagine it affecting our systems in any way.

3.21.6 hit Debian, and I confirm the issue persists.

Sorry for linking an unrelated issue, I thought that bug would be related.

At this point I'm wondering if my system is somehow botched, as the issue seems rather big to be noticed only by 2 people.

Devs are welcome to somehow blame this on our systems, as that would mean they could determine what the problem is! I'll provide whatever help I can either way. But as of yet, I don't conclude that my system is at fault, as everything else seems to work just fine (including the ComboBoxes themselves, after the 1st error).

Are the CellRendererCombo cells fine on your system?
That one is the biggest problem, IMHO, they don't work at all.

(In reply to andreastacchiotti from comment #16)
> Are the CellRendererCombo cells fine on your system?
> That one is the biggest problem, IMHO, they don't work at all.


No, since I didn't yet state it explicitly: I get (almost) the same problem as you originally reported, _plus_ the more general issues with other ComboBoxes that I added.

That means that for the CellRendererCombo -
 * I get the same 2 error messages when trying to open the menu
 * To get the menu to STAY open, I have to click and hold, or click extremely briefly... as it seems that other patterns cause it to immediately close again,
 * The messages repeat every time the menu is popped up.

- whereas for ComboBox -
 * Only the 1st error message appears,
 * and only on the 1st time the menu is popped up. Subsequent tries are OK.

I hope this is clear enough now.

bt from the CellRendererCombo case, to match my earlier one for ComboBox:

$ G_DEBUG=fatal_warnings gdb /usr/bin/gtk3-demo
GNU gdb (Debian 7.11.1-2) 7.11.1
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /usr/bin/gtk3-demo...(no debugging symbols found)...done.
+(gdb) run
Starting program: /usr/bin/gtk3-demo 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7fffee70b700 (LWP 17865)]
[New Thread 0x7fffedf0a700 (LWP 17866)]
[New Thread 0x7fffe7b61700 (LWP 17867)]
[New Thread 0x7fffe7360700 (LWP 17868)]
[Thread 0x7fffe7b61700 (LWP 17867) exited]

(gtk3-demo:17861): Gdk-CRITICAL **: gdk_window_get_window_type: assertion 'GDK_IS_WINDOW (window)' failed

Thread 1 "gtk3-demo" received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff325c241 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
+(gdb) bt

+ Trace 236676

and here's the next part, for the 2nd warning that only occurs (that we know of) for the CellRendererCombo

+(gdb) run
+The program being debugged has been started already.
Start it from the beginning? (y or n) n
Program not restarted.
+(gdb) cont
Continuing.
[Thread 0x7fffe7360700 (LWP 17868) exited]

(gtk3-demo:17861): Gtk-WARNING **: no trigger event for menu popup

Thread 1 "gtk3-demo" received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff325c241 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
+(gdb) bt

+ Trace 236677

Created attachment 336477 [details] [review]
[PATCH] gtkcombobox: pass trigger event when popping up menu

That looks like it'll fix the original case for CellRendererCombo.

For the one that happens there, and also for combos with a popup window and no wrap-width - I'm still new at all this, but might it be related to the following?

gtk_combo_box_popup_for_device():
>   if (!popup_grab_on_window (gtk_widget_get_window (priv->popup_window), pointer))

then gtk_widget_get_window():
>  * Returns the widget’s window if it is realized, %NULL otherwise

This might explain why the critical assertion only appears on the 1st time the window is popped up - if at that point it's not realised yet - because it gets a NULL pointer that time, but on any subsequent try, the window has been realised, so it works OK.

The fix would then be to ensure the popup_window is realized well in advance of the user being able to open it.

(In reply to djb from comment #21)
> That looks like it'll fix the original case for CellRendererCombo.
> 
> For the one that happens there, and also for combos with a popup window and
> no wrap-width - I'm still new at all this, but might it be related to the
> following?

Hello, what's the other non-CellRendererCombo bug here? Does the patch fix that or does it still exist? I tried to use the combo box in the inspector, but couldn't reproduce what you described even without the above patch:

(In reply to djb from comment #4)
> In fact, for the simplest ever replication step, run any app with
> GTK_DEBUG=interactive and drop down the top-left ComboBox. So this seems to
> be a general thing.

Hi William, and thanks for the reply and patch so far.

I have now been able to re-clone master and apply your patch, and can confirm that your patch fixes the issue with CellRendererCombo on my system: the warning is gone, and the popup seems to be more stable and not prone to closing itself instantly on short mouse clicks, etc.

--

The other bug that I have seen involves the GDK_IS_WiNDOW (window) assertion failure that occurs when I open 
 * a CellRendererCombo any time,
 * or a ComboBox that does not have wrap-width set.

Here are some descriptions:

https://bugzilla.gnome.org/show_bug.cgi?id=771242#c7
https://bugzilla.gnome.org/show_bug.cgi?id=771242#c17

And here are 2 backtraces:

https://bugzilla.gnome.org/show_bug.cgi?id=771242#c3
https://bugzilla.gnome.org/show_bug.cgi?id=771242#c18

I am currently able to replicate this on GTK+ 3.22.0, but compiled master last night and was not able to. This is strange, because I can't see anything that looks relevant in the intervening commits. At this point I have to go and check the Debian patches as I'm baffled by how no one else can seem to repro this...

Hmmm, could either of these Debian patches be at fault? Given that my backtraces include calls to gtk_grab_add()...

https://sources.debian.net/src/gtk%2B3.0/3.22.0-1/debian/patches/016_no_offscreen_widgets_grabbing.patch/
https://sources.debian.net/src/gtk%2B3.0/3.22.0-1/debian/patches/017_no_offscreen_device_grabbing.patch/

Next I'll test applying these over master and see whether they introduce the problem there.

Yes, simply applying this one over master:

https://sources.debian.net/src/gtk%2B3.0/3.22.0-1/debian/patches/016_no_offscreen_widgets_grabbing.patch/

Causes me to get the GDK_IS_WINDOW failure again for the mentioned ComboBoxes.

My guess is those patches are not good ideas anyway

I have complained about the patch downstream at Ubuntu, where Debian links as background for their version of it:
https://bugs.launchpad.net/ubuntu/+source/gtk+2.0/+bug/512427

I am also attempting to report this as a new bug to Debian, if my email ever reaches their... quaint bugtracker.

Gotta laugh at the fact that this patch was originally for GTK+ 2 on Ubuntu, yet Debian are still here in 2016 applying it to GTK+ 3 and introducing criticals.

If any of you have any Debian contacts to whom you could fast-track this, that would be much appreciated. It might take me a while to get anywhere.

I'm sorry about all the spam.

This one is to say that I filed a Debian bug:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=839122

Having said all that... maybe the solution is somewhere in the middle: if I'm interpreting this correctly, and the Debian-specific problem indicates the popup is not realised and so doesn't have a GdkWindow on 1st try - then could that indicate that there is a need to do something earlier in GTK+ in order to realise the window?

Anyway, even if the patch was well-intentioned,  it doesn't work with how GTK+ currently does things. I don't know enough to say whose problem that really is.

Ubuntu seems to need that downstream patch for the sound slider in Unity:

https://bugs.launchpad.net/ubuntu/+source/gtk+3.0/+bug/804009

I'm not sure if Debian should drop it or not, but I guess not many Debian users are using Unity.

Anyways, does the other upstream patch look ok to you? Would you be able to review it so we can merge it in master?

(In reply to William Hua from comment #28)
> Ubuntu seems to need that downstream patch for the sound slider in Unity:
> 
> https://bugs.launchpad.net/ubuntu/+source/gtk+3.0/+bug/804009

That Ubuntu ticket is listed as a reference on Debian's patch 017, which is for offscreen device grabs, not widget grabs. Applying patch 017 (devices) did not introduce the critical assert failure when I tried it, but 016 (widgets) did.

Anyway, the ticket makes it look like they ended up working around that in their ido package.
https://code.launchpad.net/~robertcarr/ido/offscreen-scale/+merge/73872
If so, that'd make the GTK+ patch a moot point from their POV.


> I'm not sure if Debian should drop it or not, but I guess not many Debian
> users are using Unity.

Yeah, because Unity is Ubuntu's thing, I would've expected the patch - if necessary - to be on their side, not back in Debian. But I don't know all the ins and outs and politics. ;)

Michael Biebl's preference on IRC was for GTK+ to decide whether or not to adopt the patches upstream - rather than keeping them in Debian only - and either way, figure out why patch 016 reveals a problem with GtkComboBox.


> Anyways, does the other upstream patch look ok to you? Would you be able to
> review it so we can merge it in master?

I'm not a maintainer, just have ticket editing powers for some reason! So I could review it, but I wouldn't want to change the status to much more than "reviewed". However, I'll do this, so it's clearly visible on the attachment. Thanks again for writing the patch!

Review of attachment 336477 [details] [review]:

Applying this to master resolved the "no trigger event for menu popup" warning when clicking on a CellRendererCombo. The warning is gone, and the popup seems more stable and not prone to closing itself on short clicks, etc (which I guess was because of the missing event). Thanks!

Here is a debug-enabled backtrace from the failing window assertion:

Reading symbols from /opt/gtk+/bin/gtk3-widget-factory...done.
+(gdb) run
Starting program: /opt/gtk+/bin/gtk3-widget-factory 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7fffee79f700 (LWP 9038)]
[New Thread 0x7fffedf9e700 (LWP 9039)]
Gtk-Message: Failed to load module "canberra-gtk-module"
Gtk-Message: Failed to load module "canberra-gtk-module"
[New Thread 0x7fffecef1700 (LWP 9041)]
[New Thread 0x7fffdffff700 (LWP 9042)]
[New Thread 0x7fffdf7fe700 (LWP 9043)]
[New Thread 0x7fffdeffd700 (LWP 9044)]
[New Thread 0x7fffde7fc700 (LWP 9045)]
[New Thread 0x7fffddffb700 (LWP 9046)]

(gtk3-widget-factory:9034): Gtk-WARNING **: Could not load a pixbuf from icon theme.
This may indicate that pixbuf loaders or the mime database could not be found.

Thread 1 "gtk3-widget-fac" received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff3bb2241 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
+(gdb) cont
Continuing.
[Thread 0x7fffde7fc700 (LWP 9045) exited]
[Thread 0x7fffdeffd700 (LWP 9044) exited]
[Thread 0x7fffdf7fe700 (LWP 9043) exited]
[Thread 0x7fffdffff700 (LWP 9042) exited]
[New Thread 0x7fffde7fc700 (LWP 9047)]
[Thread 0x7fffde7fc700 (LWP 9047) exited]

(gtk3-widget-factory:9034): Gdk-CRITICAL **: gdk_window_get_window_type: assertion 'GDK_IS_WINDOW (window)' failed

Thread 1 "gtk3-widget-fac" received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff3bb2241 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
+(gdb) bt

+ Trace 236712

So, we know the assert fail happens in the code added by Debian patch 016, in gtk_grab_add()

> gtkmenushell.c:632    gtk_grab_add (GTK_WIDGET (menu_shell));

We're led there from this call in gtkcombobox.c

> gtkcombobox.c:2185    gtk_menu_shell_select_item (GTK_MENU_SHELL (priv->popup_widget), active);

...which is the else case of this:

> gtkcombobox.c2161    if (priv->wrap_width > 0 || priv->cell_view == NULL)

...explaining why I don't get this with comboboxes having non-zero wrap_width, at least!

So on the 1st popup, a grab is added for popup_widget (a GtkTreeView), but at that point, popup_widget does not have a toplevel with a GtkWindow, as probed by the Debian patch.

Created attachment 336534 [details] [review]
[PATCH] gtkcombobox: pass trigger event when popping up menu

One small change. It's unlikely to happen, but also release the old trigger_event if one was already set.

Correction:

(In reply to djb from comment #32)
> So on the 1st popup, a grab is added for popup_widget (a GtkTreeView), but
> at that point, popup_widget does not have a toplevel with a GtkWindow, as
> probed by the Debian patch.

This is all true, /except/ that the popup_widget might instead be a GtkMenu - and in this case, very clearly is (duh, me).

If wrap_width != 0, we definitely get "list mode", where priv->popup_widget is a GtkTreeView. If wrap_width = 0, we get "menu mode", where priv->popup_widget is a Menu... unless the style appears-as-list is present, which AFAICT usually isn't the case.

So, an attempt is made to add a grab for the GtkMenu, and if gtk_grab_add() contains the Debian patch to check the GdkWindow type of said menu's toplevel - shenanigans are revealed, because gtk_widget_get_window(toplevel of popup_widget) returns NULL to pass to gdk_window_get_window_type() @ level #3 in the backtrace.

Hopefully this indicates what needs to be fixed?

Review of attachment 336534 [details] [review]:

This looks good to me.

Created attachment 338216 [details] [review]
HACK? silence assertion failure by calling gtk_widget_realize in menu creation

Adding gtk_widget_realize (popup) after the popup GtkMenu is created seems sufficient to kill the remaining assertion failure, but I have no idea (A) whether gtkcombobox.c is the right place for that or, more so, (B) whether the fact it seems to be needed indicates the real problem is something else entirely.

Created attachment 338229 [details] [review]
combobox: fix tabs to spaces/rm whitespace at EOLs

just some spring-cleaning before the real event...

Created attachment 338230 [details] [review]
combobox: do not select item before menu realised

> If there is an active item, this was being selected in the MenuShell -
> leading to it taking a grab - before the menu had been realised. As
> a Debian patch would reveal, this caused a critical assertion because,
> during the grab, the MenuShell's toplevel did not have a GdkWindow.
> By caching the selection and applying it after calling menu_popup, we do
> things in the correct order and thereby avoid triggering said assertion.

This seems like a better way to get around this than the previous patch.

Am I on the right track here? This certainly fixes the assertion for me, but I want to be sure it's fixing it for the right reasons, rather than just hiding something else.

Created attachment 338231 [details] [review]
combobox: do not select item before menu realised

> If there is an active item, this was being selected in the MenuShell -
> leading to it taking a grab - before the menu had been realised. As
> a Debian patch would reveal, this caused a critical assertion because,
> during the grab, the MenuShell's toplevel did not have a GdkWindow.
> By caching the selection and applying it after calling menu_popup, we do
> things in the correct order and thereby avoid triggering said assertion.

Check for a NULL selection, duh.

Created attachment 338234 [details] [review]
combobox: do not select item before menu realised

> If in menu mode with wrap_width == 0 and an active item, the item is
> selected in the MenuShell, where it takes a grab. This was done before
> the menu was popped up. A patch proposed on Bugzilla - and applied in
> Debian - revealed that, if this is the 1st time that popup is called,
> then in grab_add, the MenuShell's toplevel did not have a GdkWindow.
> This causes a critical assertion fail. By caching the selected item and
> applying it after calling menu_popup, we ensure that the MenuShell is
> realised before the item gets a grab, so we stop failing that assertion.

improved the commit msg... I think. sorry for the spam. hope this is good.

Created attachment 338239 [details] [review]
combobox: do not select item before menu realised

fixed up the explanation in the commit msg

Glad I found this, spend some unsuccessful time debugging my application for this not finding any thing in my code -- just ending up in libs and at main loop launch form my code. What is the status? Developing on Debian-Testing. Thanks.

(In reply to Percy from comment #42)
> What is the status?

I requested a review of my patch on IRC but haven't heard anything yet.

I don't know whether Debian's patches (which originated on this BZ) will get merged into 3.22, but they seem to reveal a wrong (if seemingly benign) order of operations, which currently only Debian users can notice. For GTK+ 4, the whole concept of offscreen windows is being dropped, so this error would be obscured again, for everyone.

I don't think we should let a critical assertion failure be hidden away for other distros or in GTK+ 4, so it'd be nice to see it sorted, whether that's with my patch or some other fix. I think my logic is sound, though. :P

Review of attachment 338239 [details] [review]:

ok

Fixed in master and gtk-3-22. \o/ I also cherry-picked William's trigger event patch to gtk-3-22.

*** Bug 775674 has been marked as a duplicate of this bug. ***

 When using latest gtk's FileChooserButton with SELECT_FOLDER action, I get a seg fault at gtk+/gtl/gtkmenushell.c:1249:
 g_return_if_fail (GTK_IS_MENU_ITEM (menu_item));

This can be avoided by setting a uri for the FileChooserButton before using it, but it's a little inconvenient.

I don't see how that's relevant to this bug. Please file a new bug for that issue.

I see now that you did open another bug, where this is identified as a side-effect of the fix for this one: https://bugzilla.gnome.org/show_bug.cgi?id=776949 It would have made sense to tell us that when posting here.

That's an interesting side-effect; previously, all that would've happened was a selection of a model item that was about to be removed, but now that item is cached and then re-selected (which is invalid) after the model changes. I'm not sure what the best fix would be, but will give it some thought.

*** Bug 773127 has been marked as a duplicate of this bug. ***