Last Comment Bug 677279 - Scrolling in <option> box closes box quickly
: Scrolling in <option> box closes box quickly
Status: RESOLVED FIXED
[inbound][qa+]
: regression
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: Trunk
: All Mac OS X
: -- normal (vote)
: mozilla9
Assigned To: Markus Stange [:mstange]
:
Mentors:
http://main.library.utoronto.ca/eir/a...
: 678851 680673 687030 698572 (view as bug list)
Depends on:
Blocks: 562138
  Show dependency treegraph
 
Reported: 2011-08-08 09:50 PDT by Mehdi Mulani [:mmm] (I don't check this)
Modified: 2011-11-01 15:32 PDT (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
ignore locations of momentum scroll events and send them to the position of the last non-momentum scroll event (7.65 KB, patch)
2011-08-12 06:16 PDT, Markus Stange [:mstange]
no flags Details | Diff | Review
ignore locations of momentum scroll events and send them to the position of the last non-momentum scroll event (7.74 KB, patch)
2011-08-17 14:38 PDT, Markus Stange [:mstange]
smichaud: review+
christian: approval‑mozilla‑beta+
Details | Diff | Review

Description Mehdi Mulani [:mmm] (I don't check this) 2011-08-08 09:50:36 PDT
When I click the "Select your topic area here and go" box and scroll with the mouse inside the options dropdown, the dropdown closes after scrolling about 10% of the way.

I think this is a regression from Fx4. (possibly later, not sure)
Comment 1 Boris Zbarsky [:bz] 2011-08-08 09:56:03 PDT
Can you narrow down the regression range?
Comment 2 Mounir Lamouri (:mounir) 2011-08-08 15:43:49 PDT
On my laptop (GNU/Linux with GTK), it seems to work with a trunk (probably not fully up-to-date) and with Firefox 5.
Comment 3 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-08 16:13:44 PDT
Probably just an OS X thing. I notice this bug on Lion. Works on 4 and breaks on 5 (and still on trunk), so somewhere in there.
Comment 4 Mehdi Mulani [:mmm] (I don't check this) 2011-08-08 20:30:17 PDT
Sorry, forgot to mention that I'm getting this on a Mac running Snow Leopard. I'm a bit busy at the moment but could try finding a regression tomorrow, perhaps..
Comment 5 Mounir Lamouri (:mounir) 2011-08-09 02:53:58 PDT
Tested on my Windows 7 VM and it's working fine too. Definitely MacOS specific.
I will see if I can do a regression range check on Mac.
Comment 6 Mehdi Mulani [:mmm] (I don't check this) 2011-08-09 06:02:41 PDT
Narrowed it down to the nightlies from April 9 and 10th. Specifically:
2011 04 09
7d90da136b2c - good

2011 04 10
fe3f7889918b - bad

and pushes in that range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7d90da136b2c&tochange=fe3f7889918b

Bug 562138 looks related.
Comment 7 Boris Zbarsky [:bz] 2011-08-09 06:19:13 PDT
Indeed....  Ehsan, Josh, any idea what's up here?
Comment 8 :Ehsan Akhgari (busy, don't ask for review please) 2011-08-09 14:18:57 PDT
I can't reproduce this on 10.6.  Is it Lion only?

Josh, Markus, is it possible for you to take a look please?  I don't know a lot about mac stuff...
Comment 9 Mounir Lamouri (:mounir) 2011-08-09 14:20:32 PDT
(In reply to Ehsan Akhgari [:ehsan] from comment #8)
> I can't reproduce this on 10.6.  Is it Lion only?

I was able to reproduce it on 10.6 but I had to do multiple forward and backward move before the select dropdown disappeared. Not 10% of the way like comment 0 seems to say.
Comment 10 Mehdi Mulani [:mmm] (I don't check this) 2011-08-09 14:27:21 PDT
(In reply to Mounir Lamouri (:volkmar) from comment #9)
> (In reply to Ehsan Akhgari [:ehsan] from comment #8)
> > I can't reproduce this on 10.6.  Is it Lion only?
> 
> I was able to reproduce it on 10.6 but I had to do multiple forward and
> backward move before the select dropdown disappeared. Not 10% of the way
> like comment 0 seems to say.

I can also reproduce it on 10.6, it seems like the point it disappears at is based on how much time the control has been scrolling for. That is, if I do a quick flick it can close at 80-90% whereas a slow flick has it close earlier.

I've also been reproducing this by doing a two finger scroll gesture and with momentum scrolling turned ON. I just tested it with momentum scrolling turned off and was not able to reproduce the problem.
Comment 11 Markus Stange [:mstange] 2011-08-10 09:53:49 PDT
Looks like this is an Apple bug. The value in [scrollEvent mouseLocation] is bogus, and that only seems to happen for momentum scroll events.

(gdb) p (NSPoint)[theEvent locationInWindow]
$18 = {
  x = 574, 
  y = 44
}
(gdb) p (NSPoint)[NSEvent mouseLocation]
$19 = {
  x = 659, 
  y = 471
}
(gdb) p (NSRect)[(NSWindow*)[theEvent window] frame]
$20 = {
  origin = {
    x = 462, 
    y = 319
  }, 
  size = {
    width = 404, 
    height = 306
  }
}
(gdb) p (NSRect)[(NSWindow*)[NSApp mainWindow] frame]
$21 = {
  origin = {
    x = 85, 
    y = 143
  }, 
  size = {
    width = 1042, 
    height = 590
  }
}

$19 is the real mouse location on the screen, relative to the bottom left screen corner. $20 is the rect of the popup and $21 the rect of the browser window behind it. $18 is the bogus value from mouseLocationInWindow.

Usually, this should hold true:
$18.x == $19.x - $20.origin.x
$18.y == $19.y - $20.origin.y

But in the case of the bug, this is happening instead:
$18.x == $19.x - $21.origin.x
$18.y == $19.y - ($21.origin.y + $21.size.height) - $20.origin.y

So the event doesn't really know what coordinate system it's relative to.
Comment 12 Markus Stange [:mstange] 2011-08-12 06:16:23 PDT
Created attachment 552640 [details] [diff] [review]
ignore locations of momentum scroll events and send them to the position of the last non-momentum scroll event

This also fixes bug 666719 and maybe also bug 630272.
Comment 13 Markus Stange [:mstange] 2011-08-15 05:23:20 PDT
*** Bug 678851 has been marked as a duplicate of this bug. ***
Comment 14 Reuben Morais [:reuben] 2011-08-15 20:53:08 PDT
Weird, it doesn't happen on
Comment 15 Reuben Morais [:reuben] 2011-08-17 14:38:25 PDT
(In reply to Reuben Morais [:reuben] from comment #14)
> Weird, it doesn't happen on

Ugh, sorry about that - I keep forgetting Bugzilla sends the comment if you use the "Save Changes" button in the top to CC yourself.

I was going to say it doesn't happen on all option boxes for me. For example, it doesn't happen for me on the State option box in this page [1], but it does happen on the Country and Timezone boxes.

[1] http://www.gotfrag.com/tf2/xact/user/register/
Comment 16 Markus Stange [:mstange] 2011-08-17 14:38:41 PDT
Created attachment 553915 [details] [diff] [review]
ignore locations of momentum scroll events and send them to the position of the last non-momentum scroll event

updated to trunk
Comment 17 Steven Michaud [:smichaud] (Retired) 2011-08-17 15:02:18 PDT
Your patch seems reasonable, Markus, but I have a question:

The Apple doc on [NSEvent locationInWindow] says that this returns screen coordinates if the NSEvent has a nil 'window'.  It mentions NSMouseMoved events as an example, but also says "and possibly other events".

Could that be what's going on here?
Comment 18 Markus Stange [:mstange] 2011-08-17 15:10:01 PDT
Maybe it happens sometimes, though I don't recall ever having seen an event with a nil window. In the case of this bug it's definitely not the case, or I'd have found a simpler formula at the end of comment 11. And I'd have noticed window being nil when I did (gdb) p (NSRect)[(NSWindow*)[theEvent window] frame].
Comment 19 Steven Michaud [:smichaud] (Retired) 2011-08-17 15:19:43 PDT
Comment on attachment 553915 [details] [diff] [review]
ignore locations of momentum scroll events and send them to the position of the last non-momentum scroll event

> And I'd have noticed window being nil when I did (gdb) p
> (NSRect)[(NSWindow*)[theEvent window] frame].

True enough.

I haven't tested this.  But like I said, it seems reasonable, and
*you* seem to have done quite a bit of testing.
Comment 21 Marco Bonardo [::mak] 2011-08-18 03:57:11 PDT
http://hg.mozilla.org/mozilla-central/rev/7646eade9842
Comment 22 Boris Zbarsky [:bz] 2011-08-20 20:16:57 PDT
*** Bug 680673 has been marked as a duplicate of this bug. ***
Comment 23 Ryan Lewis 2011-09-14 10:09:36 PDT
Just a quick question: what does having a target milestone of "mozilla9" actually mean? Is it related to a Gecko version, a Firefox version, or something different?

I couldn't find any documents regarding this naming convention.
Comment 24 Boris Zbarsky [:bz] 2011-09-14 10:18:55 PDT
Ryan, the target milestone in Core is the Gecko version.  So in this case Gecko 9.

Now it happens that Gecko and Firefox version numbers are in sync now, so the fix will appear in Firefox 9.
Comment 25 Markus Stange [:mstange] 2011-09-16 01:38:35 PDT
*** Bug 687030 has been marked as a duplicate of this bug. ***
Comment 26 Dan Warne 2011-09-24 20:01:40 PDT
Seriously? You are going to leave two finger scrolling in drop-down lists in Lion broken until Firefox 9, even though we're only on Firefox 6 now? This is an excruciatingly annoying problem for Lion users...
Comment 27 Boris Zbarsky [:bz] 2011-09-25 11:57:50 PDT
Dan, if Markus thinks this is safe to backport to Firefox 8 he should request aurora approval.

Firefox 7 is done, and was done by the time this patch was checked in...
Comment 28 Mounir Lamouri (:mounir) 2011-09-25 13:06:50 PDT
And Firefox 8 will be "done" in 2 days so if this has to be backported, it should be done very soon.
Comment 29 Markus Stange [:mstange] 2011-09-25 16:52:54 PDT
Comment on attachment 553915 [details] [diff] [review]
ignore locations of momentum scroll events and send them to the position of the last non-momentum scroll event

I think we should take this on Aurora. Risk is low, the patch has baked for over a month in Nightly and no regressions have been found.
The patch applies as-is on aurora.
Comment 30 christian 2011-09-27 14:22:13 PDT
Comment on attachment 553915 [details] [diff] [review]
ignore locations of momentum scroll events and send them to the position of the last non-momentum scroll event

Approved for releases/mozilla-beta. Please land asap.
Comment 31 Markus Stange [:mstange] 2011-09-28 04:19:44 PDT
Pushed to releases/mozilla-beta:
http://hg.mozilla.org/releases/mozilla-beta/rev/4020779302f2
Comment 32 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-10-13 10:48:08 PDT
qa+ for verification with testcase in Firefox 8.
Comment 33 Steve Mokris 2011-11-01 15:32:05 PDT
*** Bug 698572 has been marked as a duplicate of this bug. ***

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