330999 - Mac doesn't call nsIRollupListener::ShouldRollupOnMouseWheelEvent

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Mac, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Mac doesn't call nsIRoolupListener::ShouldRollupOnMouseWheelEvent.
Therefore, bug 187772 is not fixed on Mac.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.1

Probably, this fixes this bug. But I don't have Mac, we need to test this patch.

Comment 2

[Hiro]

(In reply to comment #1)
> Created an attachment (id=215555) [edit]
> Patch rv1.1
> 
> Probably, this fixes this bug. But I don't have Mac, we need to test this
> patch.
> 

This patch was tested.
This patch is working fine.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 215555 [details] [diff] [review]
Patch rv1.1

Thanks, let's go this patch.

Comment 4

[Jo Hermans]

Too late for alpha 1 I suppose ...

Bug 187772 was blocking-firefox2+, but target mozilla1.9alpha1, so I don't think I can set that target to mozilla1.8.1alpha1, as asked for alpha1 triaging. So, I'ver set the blocking-firefox2? flag, and I'll hope it will be in alpha2.

Comment 5

[Josh Aas]

I'm  going to review this today, sorry for the long delay.

Comment 6

[Josh Aas]

Comment on attachment 215555 [details] [diff] [review]
Patch rv1.1

Looks good, my tree is taking too long to build so I didn't test it myself. Looks like Hiro tested it though and it works.

Comment 7

[Josh Aas]

Note to self to make sure this gets fixed in Cocoa widgets too.

Comment 8

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 215555 [details] [diff] [review]
Patch rv1.1

rs=pink

Comment 9

[Josh Aas]

Masayuki - do you have cvs commit privs or do you want me to check this in? It can go on the FF 2 branch and the trunk. I'll take responsibility for fixing this in Cocoa, so don't close this bug before I do that.

Thanks Masayuki!

Comment 10

[Josh Aas]

landed on 1.8 branch and trunk

Comment 11

[Hiro]

verified.
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20060406 Firefox/2.0a1

Comment 12

[Josh Aas]

Attachment: cocoa fix v1.0

This also fixes rollup for normal scrolling.

Comment 13

[Mark Mentovai]

Comment on attachment 218184 [details] [diff] [review]
cocoa fix v1.0

+  // close popups if we're supposed to...
+  if (gRollupListener && gRollupWidget &&
+      [self window] != gRollupWidget->GetNativeData(NS_NATIVE_WINDOW)) {
+    PRBool rollup = PR_FALSE;
+    gRollupListener->ShouldRollupOnMouseWheelEvent(&rollup);
+    if (rollup)
+      gRollupListener->Rollup();
+  }

I think you should do this in scrollWheel: and not scrollWheel:forAxis:.

Comment 14

[Josh Aas]

landed Cocoa fix on trunk with mento's suggestion