Closed Bug 516654 Opened 15 years ago Closed 15 years ago

Cannot scroll to horizontal continuously with Logitech (Logicool) mouse

Categories

(Core :: Widget: Win32, defect, P2)

x86
Windows Vista
defect

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: regression)

Attachments

(1 file, 4 obsolete files)

Attached patch Patch v1.0 (obsolete) — Splinter Review
I tested with MX-1100 and 4.82.11 on trunk. When I tilted the mouse wheel and kept the tilting, Fx scrolled to horizontal only once, not continuously.

The mouse driver sends WM_MOUSEHWHEEL first. Then, if we return 0 (that means we process the message), the mouse driver posts the same message continuously (mode#1). Otherwise, the mouse driver posts WM_KEYDOWN message with the left arrow key or the right arrow key (mode#2).

At bug 507222, the mouse wheel handling code was changed. The patch changes to use ::GetMessagePos API instead of the lParam.

The lParam is always 0 if the message was sent from the Logitech mouse driver. So, the old code always failed the WM_MOUSEHWEEL handling and returned 1.  I.e., the old Firefox was scrolled to horizontal by the WM_KEYDOWN messages (mode#2).

However, by the patch, we return 0 only at first time (when the message was sent).  So, we are in mode#1 now.  But in the handling of the posted WM_MOUSEHWHEEL, ::GetMessagePos API always returns 0. Therefore, we failed to scroll only at the posted messages.

The attached patch use ::GetCursorPos API if the lParam is 0.  This is wrong logic, however, lParam should be the mouse cursor position and (0,0) is very rare case.  So, I think that this patch doesn't break the behavior in most cases.
Flags: blocking1.9.2?
Attachment #400687 - Flags: review?(roc)
Version: 1.9.2 Branch → Trunk
> But in the handling of the posted WM_MOUSEHWHEEL, ::GetMessagePos API always
> returns 0.

That sounds like someone else's bug. Is it a driver bug or a Windows bug?
(In reply to comment #1)
> > But in the handling of the posted WM_MOUSEHWHEEL, ::GetMessagePos API always
> > returns 0.
> 
> That sounds like someone else's bug. Is it a driver bug or a Windows bug?

Probably, the driver's bug because lParam also 0. It must have same value the result of ::GetMessagePos API.
Is there some way to detect this driver directly? This whole area of mouse drivers seems to be fragile. I think we should have very narrowly targeted hacks with specific detection of bad drivers to enable each hack.

0,0 could easily happen if the user moves their mouse to the top-left corner...
(In reply to comment #3)
> 0,0 could easily happen if the user moves their mouse to the top-left corner...

Yes, but the users stay to 0,0 until we process the message, ::GetCursorPosition also returns 0,0. There is a difference "when" we get the cursor pos. When we use ::GetMessagePos, we get the cursor pos at last GetMessage called. So, we may be able to cache the position in nsAppSheel, maybe.
Ah, sorry, it's wrong. The result of ::GetMessagePos is not the position at last ::GetMessage. The position is when it was posted.
Attached patch Patch v2.0 (obsolete) — Splinter Review
How about this?

This checks whether the mouse driver is logitech's or not.

If the result of ::GetMessagePos is zero, this patch gets the window of 0,0. If it's our window, this patch trust the values. Otherwise, we assume that we are in the Logitech mouse driver problem. Then, this uses ::GetCursorPos instead.

But if we get non-zero lParam for WM_MOUSEHWEEL, the user may change the pointing device to another one (e.g., touchpad of the notebook). Then this patch reset the flag.
Attachment #400687 - Attachment is obsolete: true
Attachment #400949 - Flags: review?(roc)
Attachment #400687 - Flags: review?(roc)
Attached patch Patch v2.0.1 (obsolete) — Splinter Review
oops, fix a nit.
Attachment #400949 - Attachment is obsolete: true
Attachment #400950 - Flags: review?(roc)
Attachment #400949 - Flags: review?(roc)
Comment on attachment 400950 [details] [diff] [review]
Patch v2.0.1

Ah, this cannot be usable when the user maximize our window on the primary screen...
Attachment #400950 - Flags: review?(roc) → review-
Attached patch Patch v3.0 (obsolete) — Splinter Review
This is simple and low risk.

The lParam and the result of ::GetMessagePos should be always same value. Therefore, if these values are different and the lParam is zero and in send message processing, we can assume that the mouse driver is the Logitech's.

And also both lParam and the result of ::GetMessagePos is zero, we should replace the position by ::GetCursorPos.
Attachment #400950 - Attachment is obsolete: true
Attachment #400978 - Flags: review?(roc)
Attached patch Patch v3.0Splinter Review
oops, I posted an old patch, sorry for the spam.
Attachment #400978 - Attachment is obsolete: true
Attachment #400980 - Flags: review?(roc)
Attachment #400978 - Flags: review?(roc)
Roc:

Would you review this too? This bug makes the logitech mouse hard to use.
http://hg.mozilla.org/mozilla-central/rev/83af6b9b03b1
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Comment on attachment 400980 [details] [diff] [review]
Patch v3.0

Let's take this patch for the Logitech mouse users.

This bug is a regression of bug 507222 which is a regression of the widget removal.

This patch checks the strange lParam value which comes from Logitech mouse driver. If this patch finds the strange lParam, this is using the current cursor position instead of the wrong result of the GetMessagePos API.

In most cases, the result of GetMessagePos and the current cursor position are same or similar. So, the risk is low even if the detecting condition is wrong.
Attachment #400980 - Flags: approval1.9.2?
Flags: blocking1.9.2? → blocking1.9.2+
Priority: -- → P2
Attachment #400980 - Flags: approval1.9.2?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: