677279 - Scrolling in <option> box closes box quickly

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Mehdi Mulani [:mmm] (I don't check this)]

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 [:bzbarsky]]

Can you narrow down the regression range?

Comment 2

[Mounir Lamouri (:mounir)]

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 bugmail, email directly)]

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)]

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)]

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)]

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 [:bzbarsky]]

Indeed....  Ehsan, Josh, any idea what's up here?

Comment 8

[(no longer active)]

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)]

(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)]

(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]]

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]]

Attachment: 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 14

[Reuben Morais [:reuben]]

Weird, it doesn't happen on

Comment 15

[Reuben Morais [:reuben]]

(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]]

Attachment: 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)]

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]]

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)]

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 20

[Markus Stange [:mstange]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/7646eade9842

Comment 21

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/7646eade9842

Comment 23

[Ryan Lewis]

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 [:bzbarsky]]

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 26

[Dan Warne]

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 [:bzbarsky]]

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)]

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]]

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

[LegNeato]

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]]

Pushed to releases/mozilla-beta:
http://hg.mozilla.org/releases/mozilla-beta/rev/4020779302f2

Comment 32

[u279076]

qa+ for verification with testcase in Firefox 8.