Closed Bug 347875 Opened 14 years ago Closed 14 years ago

Support WM_MOUSEHWHEEL for tilt wheel mouse

Categories

(Core :: Widget: Win32, defect)

x86
Windows Vista
defect
Not set

Tracking

()

VERIFIED FIXED

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: verified1.8.1.1)

Attachments

(2 files, 4 obsolete files)

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;".
Attached patch Patch rv2.0 (obsolete) — Splinter Review
I'm creating the environment of Vista for testing this...
Status: NEW → ASSIGNED
Attached patch Patch rv3.0 (obsolete) — Splinter Review
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.
Attachment #232717 - Attachment is obsolete: true
Attachment #233833 - Flags: review?(emaijala)
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?
(In reply to comment #3)
> One question though: is 1 a good default for ulScrollChars?

The default value of Vista beta2 is 1.
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.
Attachment #233833 - Flags: review?(emaijala)
Attachment #233833 - Flags: review?(VYV03354)
Attachment #233833 - Flags: review+
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.
Attachment #233833 - Flags: review?(VYV03354) → review+
Attached patch Patch rv3.1 (obsolete) — Splinter Review
Thank you, Ere and Kimura-san for your review.
Attachment #233833 - Attachment is obsolete: true
Attachment #236609 - Flags: superreview?(roc)
Attachment #236609 - Flags: review+
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|.
Attached patch Patch rv3.2Splinter Review
Wow, thank you, Kimura-san.

This patch returns same value as the sample code of the MSDN (comment 0).
Attachment #236609 - Attachment is obsolete: true
Attachment #236962 - Flags: superreview?(roc)
Attachment #236962 - Flags: review+
Attachment #236609 - Flags: superreview?(roc)
Attachment #236962 - Flags: superreview?(roc) → superreview+
checked-in to trunk.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
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.
Attachment #236962 - Flags: approval1.8.1?
This fix was confirmed on Win Vista RC1 by Kimura-san.(Thank you for your work!)

-> v.
Status: RESOLVED → VERIFIED
Can anyone comment on the risk to regressions for this bug on the 1.8 branch?
Attached patch Patch rv3.2 for 1.8 branch (obsolete) — Splinter Review
Sorry, the previous patch cannot be applied to 1.8 branch.
Attachment #237959 - Flags: approval1.8.1?
Attachment #236962 - Flags: approval1.8.1?
Oops, sorry for the spam.
Attachment #237959 - Attachment is obsolete: true
Attachment #237971 - Flags: approval1.8.1?
Attachment #237959 - Flags: approval1.8.1?
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.
Attachment #237971 - Flags: approval1.8.1? → approval1.8.1-
Attachment #237971 - Flags: approval1.8.1.1?
Mike Schroepfer:

Please recheck this bug. Win Vista will be released 11/30 for companies.
Flags: blocking1.8.1.1?
Thanks for the reminder - have any regressions or other issues come up for this since it has been on the trunk?
(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.
Flags: blocking1.8.1.1? → blocking1.8.1.1-
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.
Attachment #237971 - Flags: approval1.8.1.1? → approval1.8.1.1+
checked in to 1.8 branch too.
Keywords: fixed1.8.1.1
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
Depends on: 364518
You need to log in before you can comment on or make changes to this bug.