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 754098 - Cannot resize window with destroyed EventBox
Cannot resize window with destroyed EventBox
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Class: GtkGesture
3.16.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2015-08-26 00:39 UTC by Will Greenberg
Modified: 2015-09-18 11:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
minimal_example.js (992 bytes, text/plain)
2015-08-26 00:39 UTC, Will Greenberg
  Details
gtkwidget: refactor code into separate function (2.54 KB, patch)
2015-09-10 12:30 UTC, Carlos Garnacho
committed Details | Review
gtkwidget: Ensure unrealization during event dispatching cancels gestures (2.76 KB, patch)
2015-09-10 12:30 UTC, Carlos Garnacho
committed Details | Review
widget: Fix propagation of gesture cancellation on widget unrealize/destroy (3.05 KB, patch)
2015-09-16 10:17 UTC, Carlos Garnacho
committed Details | Review
widget: Cancel also denied sequences (1.03 KB, patch)
2015-09-16 10:17 UTC, Carlos Garnacho
committed Details | Review
window: Reset on unhandled gestures right away (2.10 KB, patch)
2015-09-18 10:42 UTC, Carlos Garnacho
committed Details | Review

Description Will Greenberg 2015-08-26 00:39:04 UTC
There seems to be a problem handling user input when combining two EventBoxes in an Overlay, and then destroying one. Pretty obscure use case, sure, but it's reproducible every time.

Here's a minimal example: http://paste.fedoraproject.org/259303/14405462

Note that the titlebar is needed to force client-side decorations. Steps to reproduce:
1. Run the app
2. Click once in the window to cause the topmost EventBox to be destroyed
3. Attempt to resize the Window. No dice.
Comment 1 Will Greenberg 2015-08-26 00:39:47 UTC
Created attachment 309984 [details]
minimal_example.js
Comment 2 Jasper St. Pierre (not reading bugmail) 2015-08-26 03:02:11 UTC
I traced this down to the multipress gesture in GtkWindow somehow "getting stuck". I'm not overly familiar with the gesture system in GTK+, so let's ask Carlos to take a look at it.
Comment 3 Matthias Clasen 2015-08-28 20:10:55 UTC
Carlos, any insight here ?
Comment 4 Carlos Garnacho 2015-09-10 12:30:39 UTC
Created attachment 311063 [details] [review]
gtkwidget: refactor code into separate function

This "cancel sequence across widget hierarchy" code will be useful
in other places, so take it out to a separate function.
Comment 5 Carlos Garnacho 2015-09-10 12:30:44 UTC
Created attachment 311064 [details] [review]
gtkwidget: Ensure unrealization during event dispatching cancels gestures

We use to rely on grab broken events for most of the event sequence
lifetime, this breaks though on GDK_BUTTON_RELEASE/GDK_TOUCH_END, as there's
no longer a grab at that time.

For these cases (and all others where there's destroy/unrealize calls
involved during event dispatching), catch this on the late
WIDGET_REALIZED_FOR_EVENT calls on widget event handling functions.
Comment 6 Matthias Clasen 2015-09-13 05:43:52 UTC
Review of attachment 311063 [details] [review]:

ok
Comment 7 Matthias Clasen 2015-09-13 05:45:34 UTC
Review of attachment 311064 [details] [review]:

ok
Comment 8 Carlos Garnacho 2015-09-14 15:39:17 UTC
Attachment 311063 [details] pushed as 0dae974 - gtkwidget: refactor code into separate function
Attachment 311064 [details] pushed as 13873d2 - gtkwidget: Ensure unrealization during event dispatching cancels gestures
Comment 9 Cosimo Cecchi 2015-09-15 16:24:28 UTC
This still does not seem to work properly; after the patches I see this behavior:
- run the app - the window can be resized
- click once in the window. It can't be resized anymore
- click again in the window. It can be resized again
Comment 10 Carlos Garnacho 2015-09-15 16:29:02 UTC
Can the app be disclosed? Does the target window/widget have both button press/release mask bits? There's unfortunately cases where we just don't control the event mask set on windows, so certain events can be no-op from the GDK core, one argument in favor of removing evmasks I guess...
Comment 11 Cosimo Cecchi 2015-09-15 16:30:50 UTC
Yes, the app I'm referring to is attached in the first comment here.
Comment 12 Carlos Garnacho 2015-09-16 10:13:46 UTC
Doh, on the attempts to gather info I managed to modify the test app in a way it fooled me into thinking this alone worked. I'm attaching two more patches to fix this intent up.
Comment 13 Carlos Garnacho 2015-09-16 10:17:31 UTC
Created attachment 311444 [details] [review]
widget: Fix propagation of gesture cancellation on widget unrealize/destroy

At the time event_check_cancel_sequence_on_hierarchy() is called, the widget
has been already unparented. Given the widget itself is being destroyed,
cancellation on it is impending in one way or another, we still must
propagate cancellation across all parents, so retrieve it early before
possible widget destruction.
Comment 14 Carlos Garnacho 2015-09-16 10:17:37 UTC
Created attachment 311445 [details] [review]
widget: Cancel also denied sequences

It makes no sense to skip denied sequences here, the gestures are
still carrying out the accounting for these, which must be also put
to an end if we're possibly not receiving any further events from
this sequence.
The en masse _gtk_widget_cancel_sequence() function was needlessly
checking we
Comment 15 Matthias Clasen 2015-09-16 14:57:11 UTC
Review of attachment 311444 [details] [review]:

ok, looks easy enough
Comment 16 Matthias Clasen 2015-09-16 14:58:21 UTC
Review of attachment 311445 [details] [review]:

sure
Comment 17 Jasper St. Pierre (not reading bugmail) 2015-09-16 15:03:52 UTC
Review of attachment 311445 [details] [review]:

I think you have an unfinished commit message here.
Comment 18 Carlos Garnacho 2015-09-16 17:18:56 UTC
Fixed the commit log and pushed.

Attachment 311444 [details] pushed as 3aaf730 - widget: Fix propagation of gesture cancellation on widget unrealize/destroy
Attachment 311445 [details] pushed as 63e255e - widget: Cancel also denied sequences
Comment 19 Cosimo Cecchi 2015-09-17 22:16:16 UTC
There's still a case where this does not work; after talking to Carlos on IRC, he wrote another patch that fixes the other case.
Reopening this so Carlos can attach the patch here.
Comment 20 Carlos Garnacho 2015-09-18 10:42:00 UTC
Created attachment 311623 [details] [review]
window: Reset on unhandled gestures right away

Traditionally a sequence is set to GTK_EVENT_SEQUENCE_DENIED state when
it is to be ignored, which means it is dormant, but still managed by the
gesture (accounting, "denied" sequences still make "slots" in multitouch
gesture busy, etc...).

This gesture will run for all button presses and releases in the window
though when presses happen on the "window content" region, and we can't
account for every children to be as educated as setting the proper mask
on every window, or ensuring events will be propagated as they should.

In order to cater for this, just reset the gestures, we can live without
such accounting in these specific GtkGestureSingle gestures.
Comment 21 Carlos Garnacho 2015-09-18 11:06:32 UTC
I've finally pushed this last patch, and reverted all last 3 ones, the one with the minor refactor is still fine.