347875 - Support WM_MOUSEHWHEEL for tilt wheel mouse

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

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

We should support WM_MOUSEHWHEEL that is a new message for tilt wheel on WindowsVista and future drivers of Microsoft. See http://msdn.microsoft.com/library/en-us/dnwui/html/BestPracticesforSupportingMicrosoftMouseandKeyboardDevices.asp

I think that we should fix this bug on major release before Windows Vista.

Note that current driver of Microsoft on Win2k and WinXP has a quirk for the window of Mozilla. It sends key down and key up messages for compatibility. But by the way, if an editor has focus, the caret is moved by the tilt wheel. And the user cannot scroll the elements that has "overflow: auto|scroll;".

Comment 1

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

Attachment: Patch rv2.0

I'm creating the environment of Vista for testing this...

Comment 2

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

Attachment: Patch rv3.0

Kimura-san found the other problem for current wheel code. The current code doesn't use the surplus delta value of previous event. Therefore, on default settings on WinVista, we cannot use vertical wheel scroll. This patch contains the code for the problem too.

And I tested this patch on Win Vista. This patch works fine for me.

Comment 3

[Ere Maijala (slow)]

Comment on attachment 233833 [details] [diff] [review]
Patch rv3.0

I cannot test this, but will give review if you can't find anyone who could test. One question though: is 1 a good default for ulScrollChars?

Comment 4

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

(In reply to comment #3)
> One question though: is 1 a good default for ulScrollChars?

The default value of Vista beta2 is 1.

Comment 5

[Ere Maijala (slow)]

Comment on attachment 233833 [details] [diff] [review]
Patch rv3.0

Need someone with Vista to verify this works. Asking Kimura-san to test it. Otherwise ok.

Comment 6

[Masatoshi Kimura [:emk]]

Comment on attachment 233833 [details] [diff] [review]
Patch rv3.0

> +        *aRetValue = isVertical ? !result : result;
I prefer to set aRetValue only if result is true because aRetValue will be ignored if result is false.
Otherwise looks good. I confirmed that it works on WinVista Beta 2.

Comment 7

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

Attachment: Patch rv3.1

Thank you, Ere and Kimura-san for your review.

Comment 8

[Masatoshi Kimura [:emk]]

Comment on attachment 236609 [details] [diff] [review]
Patch rv3.1

> +          *aRetValue = isVertical ? PR_FALSE : PR_TRUE;
Why are you using PR_FALSE and PR_TRUE here?
|*aRetValue| is not a PRBool but a LRESULT. Other |nsWindow| handlers seem to use Windows native typedefs and macros for |*aRetValue|.

Comment 9

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

Attachment: Patch rv3.2

Wow, thank you, Kimura-san.

This patch returns same value as the sample code of the MSDN (comment 0).

Comment 10

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

checked-in to trunk.

Comment 11

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

Comment on attachment 236962 [details] [diff] [review]
Patch rv3.2

This may be too late for Fx2. But we need to fix this on 2.0.1 or later release for WinVista.

The risk is low. This improves the wheel scroll processing, and this adds to support for new mouse wheel message that is added on WinVista.

Comment 12

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

This fix was confirmed on Win Vista RC1 by Kimura-san.(Thank you for your work!)

-> v.

Comment 13

[Mike Schroepfer]

Can anyone comment on the risk to regressions for this bug on the 1.8 branch?

Comment 14

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

Attachment: Patch rv3.2 for 1.8 branch

Sorry, the previous patch cannot be applied to 1.8 branch.

Comment 15

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

Attachment: Patch rv3.2 for 1.8 branch

Oops, sorry for the spam.

Comment 16

[Mike Schroepfer]

Comment on attachment 237971 [details] [diff] [review]
Patch rv3.2 for 1.8 branch

It is, alas, a little late for 1.8.1 at this point.  After some good baking on the trunk we'd consider for a 1.8.1.1 release.

Comment 17

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

Mike Schroepfer:

Please recheck this bug. Win Vista will be released 11/30 for companies.

Comment 18

[Mike Schroepfer]

Thanks for the reminder - have any regressions or other issues come up for this since it has been on the trunk?

Comment 19

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

(In reply to comment #18)
> Thanks for the reminder - have any regressions or other issues come up for this
> since it has been on the trunk?
> 

I don't have the regression reports.

Comment 20

[Jay Patel [:jay]]

Comment on attachment 237971 [details] [diff] [review]
Patch rv3.2 for 1.8 branch

This seemed to have just missed 2.0, and has been baking for a while now.  Approved for 1.8.1 branch, a=jay for drivers.  Please land asap.

Comment 21

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

checked in to 1.8 branch too.

Comment 22

[Masatoshi Kimura [:emk]]

Verified fixed on
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.1pre) Gecko/20061122 BonEcho/2.0.0.1pre
Windows Vista Build 5744