Last Comment Bug 459532 - Faulty event compression in gtk2 widget implementation
: Faulty event compression in gtk2 widget implementation
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Gtk (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla1.9.1b2
Assigned To: Chris Toshok
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-11 19:11 PDT by Chris Toshok
Modified: 2012-05-01 16:21 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch which fixes the problem (990 bytes, patch)
2008-10-11 19:11 PDT, Chris Toshok
no flags Details | Diff | Review
doh, remove that printf.. [Checkin: Comment 8] (937 bytes, patch)
2008-10-11 19:13 PDT, Chris Toshok
roc: review+
roc: superreview+
Details | Diff | Review

Description Chris Toshok 2008-10-11 19:11:17 PDT
Created attachment 342750 [details] [diff] [review]
patch which fixes the problem

Right now if there are multiple MotionNotify events in the X event queue, they are all compressed to 1 (the newest), regardless whether or not they're contiguous in the queue.  So, you can end up in situations where you have a queue that looks like this:

MotionNotify (100, 200) => ButtonPress (100, 200) => ButtonRelease (100, 200) => MotionNotify (200, 200)

and that gets compressed to:

MotionNotify (200, 200) => ButtonPress (100, 200) => ButtonRelease (100, 200)

Clearly that's broken.  This is due to XCheckWindowEvent being used, instead of Peek+Next event.  This usually won't be a problem, but it's definitely breaking our automated Moonlight testing using XTest, as that extension has the tendency to load up the client event queue.

I've attached a patch which fixes our issue and has no negative impact on normal usage.
Comment 1 Chris Toshok 2008-10-11 19:13:02 PDT
Created attachment 342751 [details] [diff] [review]
doh, remove that printf..
[Checkin: Comment 8]
Comment 2 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-10-12 14:58:39 PDT
Comment on attachment 342751 [details] [diff] [review]
doh, remove that printf..
[Checkin: Comment 8]

Thanks man!
Comment 3 Serge Gautherie (:sgautherie) 2008-10-12 17:21:58 PDT
"toshok@hungry.com": Please, supply your real name(s), not your email address only.
Comment 4 Chris Toshok 2008-10-12 17:52:07 PDT
huh, I thought bugzilla had my real name already...  anyway, fixed.
Comment 5 Reed Loden [:reed] (use needinfo?) 2008-10-12 18:19:48 PDT
(In reply to comment #3)
> "toshok@hungry.com": Please, supply your real name(s), not your email address
> only.

Even so, please don't remove the checkin-needed keyword for reasons like this. Bugs get lost when you do that. :(
Comment 6 Chris Toshok 2008-10-12 18:54:39 PDT
ack - wasn't aware i'd done that - sorry :)
Comment 7 Reed Loden [:reed] (use needinfo?) 2008-10-12 19:05:26 PDT
(In reply to comment #6)
> ack - wasn't aware i'd done that - sorry :)

You didn't. I was talking to Serge. :)
Comment 8 Serge Gautherie (:sgautherie) 2008-10-12 19:35:02 PDT
Comment on attachment 342751 [details] [diff] [review]
doh, remove that printf..
[Checkin: Comment 8]

http://hg.mozilla.org/mozilla-central/rev/1ce7fddcb2ce
Comment 9 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2008-10-15 06:40:19 PDT
It looks like Stuart backed this out on the FENNEC_A1_BRANCH because it broke something: http://hg.mozilla.org/mozilla-central/rev/04cb6a409e9e
Comment 10 Vladimir Vukicevic [:vlad] [:vladv] 2008-10-29 11:44:50 PDT
This should be backed out on trunk -- it's causing the fennec problems; need to figure out why (and why they're not being seen on the desktop).
Comment 11 Vladimir Vukicevic [:vlad] [:vladv] 2008-10-29 11:45:35 PDT
("fennec problems" = mouse events are very laggy, you touch the screen and move the mouse, and the app responds only after a second or so delay, with the motion that you did)
Comment 12 Larry Ewing 2008-10-30 08:36:13 PDT
I think the problems on fennec are probably due to the maemo gtk2 patches that unconditionally enable xinput events thus breaking this hack to do event compression since every motion event is followed by a device motion event.  See bug 459780 for some more details.  Perhaps the right way to fix this is using motion hints properly instead of emulating them poorly.
Comment 13 Oleg Romashin (:romaxa) 2008-11-09 03:31:39 PST
https://bugzilla.mozilla.org/show_bug.cgi?id=459780#c6
Comment 14 Karl Tomlinson (ni?:karlt) 2012-05-01 16:21:20 PDT
This, probably appropriately, wasn't backed out on m-c trunk.

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