After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 767468 - Popover over a treeview cellrenderer is hidden immediately after being shown
Popover over a treeview cellrenderer is hidden immediately after being shown
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkTreeView
3.21.x
Other All
: Normal normal
: ---
Assigned To: gtktreeview-bugs
gtktreeview-bugs
Depends on:
Blocks:
 
 
Reported: 2016-06-09 17:23 UTC by LRN
Modified: 2018-01-06 20:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Testcase (2.36 KB, text/plain)
2016-06-09 17:23 UTC, LRN
  Details
treeview: Ensure the treeview has the implicit grab before grabbing focus (1.29 KB, patch)
2016-06-13 22:00 UTC, Carlos Garnacho
committed Details | Review

Description LRN 2016-06-09 17:23:06 UTC
Created attachment 329494 [details]
Testcase

How to reproduce:
* Compile and run the Testcase
* Click on the checkboxes

Expected result:
* Popover is shown

Actual result:
* Popover is shown and immediately hidden

This is caused by commit 72ea348ad624c184aed437704e5a31e5eefb16cc.
Setting "can-focus"=False on the treeview works around this, but it's obviously not a valid fix if one needs treeview to be keyboard-navigatable.
Comment 1 Carlos Garnacho 2016-06-13 21:59:45 UTC
Ok, so what seems to happen here:

1) user presses the cellrenderertoggle
2) ::activated signal triggers showing a modal GtkPopover, a GTK+ grab takes input there.
3) GtkTreeView still tries to fully focus the clicked row, which implies grabbing the keyboard focus too.
4) popover sees the focus going outside the popover, and closes itself.

So I'd say the behavior with that commit is more correct than the previous behavior, where the treeview would still manage to grab focus while the popover is still shown. This can be seen with git revert 72ea348a, popping up the popover, and using the up/down keys, you'll still be able to change rows even if the popover is shown.

So I'd say it's GtkTreeView which shouldn't attempt grabbing the focus after this happens. I'm attaching a patch to have it check whether a grab wasn't added in the mean time, and bail out on grabbing focus otherwise.
Comment 2 Carlos Garnacho 2016-06-13 22:00:07 UTC
Created attachment 329733 [details] [review]
treeview: Ensure the treeview has the implicit grab before grabbing focus

The cellrenderer signals might be taking the grab somewhere else, at which
point it's dubious we should attempt to take the keyboard focus into the
treeview.

This concretely breaks popovers triggered from cellrenderer signals on
button press, because the treeview will attempt to grab focus
inconditionally then.
Comment 3 LRN 2016-06-17 04:02:32 UTC
attachment 329733 [details] [review] fixes this for me (both in the testcase and in actual application).
Comment 4 Matthias Clasen 2016-06-19 21:48:32 UTC
Review of attachment 329733 [details] [review]:

ok
Comment 5 Matthias Clasen 2016-06-20 01:23:26 UTC
Attachment 329733 [details] pushed as e33e23a - treeview: Ensure the treeview has the implicit grab before grabbing focus
Comment 6 Roman Lebedev 2016-08-28 12:36:57 UTC
Hello.
This "fix" has caused a regression - https://bugzilla.gnome.org/show_bug.cgi?id=770508
Comment 7 Kjell Ahlstedt 2016-09-06 16:45:48 UTC
I suspect that this fix is also responsible for the erroneous behaviour of
GtkFileChooserDialog described in gtk+ bug 770130 and gtkmm bug 770325.
Comment 8 Egmont Koblinger 2018-01-06 20:24:42 UTC
Hi guys,

What's the state of this bug?

It's marked as FIXED, then apparently the change got reverted and fixed in another way, there are three additional bugs also linked all marked as FIXED, all in Sep 2016.

I'm currently on Ubuntu Artful with GTK+ 3.22.25, and the attached test case doesn't work. It prints "activated" but the popover isn't presented, just like in the original report.

Did a subsequent commit break it again??