314792 - [BeOS] Horizontal wheel scrolling support

Status: RESOLVED FIXED

(Core Graveyard :: Widget: BeOS, defect)

Description

[Sergei Dolgov]

Currently we don't support horizontal wheels.
It seems there is time to add such support.
Also we can rewrite wheel event processing according to ideas from
https://bugzilla.mozilla.org/show_bug.cgi?id=314687
and reduce code size in CallMethod() by moving part of wheel code to OnWheel() method.

Only thing i'm wondering about, why such methods as OnPaint() are declared as virtual, while there is no prototypes in parent classes for that, and methods are for local use only. So I'm in doubts if coming OnWheel() should be also virtual.

Comment 1

[Sergei Dolgov]

additional issue of current wheel-scrolling implementations is unused system default for step. It hardcodes 3 as step-value. If i remember correctly old discussions, it was set that way for same reason we use various tricks for mouse move. It means - in order to avoid too tight sequence of native messages.
At current time i don't see real need for that.
Only thing i don't know, if Firefox allows to set wheel-parameters in Preferences, as SeaMokey does.
FF users may be accustomized to use current big step and may be confused, if there is no visible method to set the step (Not all do like digging in about:config). If this is the case, maybe there will be need to change defaults in all.js for FF-BeOS

Comment 2

[Doug Shelton]

Firefox has no mouse-related settings in the Preferences panel, so fyysik's suggestion in comment #1 should be considered.

Comment 3

[Sergei Dolgov]

Attachment: patch diff -up6

review request.

Adding X-wheel support. Using same technique to reduce native event number as used for drawing events - local delta storage in nsViewBeOS + f*Dispatched flag.

Due some bugs in BeOS TwoWheel code (X-delta is sometimes generated when only Y-movement is present), making two events exclusive with Y-wheel priority. I dislike non-balanced "if else", but hope it is temporary.

How to test: get input filter from here:
http://bebits.com/app/3805
install it manually, if you have single-wheel mouse (install-script fails in this case), also install settigs file manually into home/config/settings/*, edit it if needed, restart input server and use ScrollLock key to switch between Y and X scrolling.

Comment 4

[tqh]

Comment on attachment 202551 [details] [diff] [review]
patch diff -up6

r=[e-mail]

Don't have the BeBook here, but as long as GetWheel retvalue = wheel copies the BPoint I think it's ok.

Comment 5

[Sergei Dolgov]

Attachment: patch

exactly same as previous, but *.h changes included

Comment 6

[Sergei Dolgov]

landed in trunk
Checking in mozilla/widget/src/beos/nsWindow.cpp;
/cvsroot/mozilla/widget/src/beos/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.111; previous revision: 1.110
done
Checking in mozilla/widget/src/beos/nsWindow.h;
/cvsroot/mozilla/widget/src/beos/nsWindow.h,v  <--  nsWindow.h
new revision: 1.46; previous revision: 1.45
done 

Closing

Comment 7

[Niels Reedijk]

Next on patch queue.

Comment 8

[Niels Reedijk]

Attachment: Patch for 1_8 branch

Requesting approval for landing in the MOZILLA_1_8_BRANCH.

This is a BeOS-only change and will not affect any other platforms.

Comment 9

[Mike Schroepfer]

Comment on attachment 232095 [details] [diff] [review]
Patch for 1_8 branch

a=schrep for drivers for BeOS only change.

Comment 10

[Sergei Dolgov]

Checking in mozilla/widget/src/beos/nsWindow.cpp;
/cvsroot/mozilla/widget/src/beos/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.91.4.20; previous revision: 1.91.4.19
done
Checking in mozilla/widget/src/beos/nsWindow.h;
/cvsroot/mozilla/widget/src/beos/nsWindow.h,v  <--  nsWindow.h
new revision: 1.35.4.12; previous revision: 1.35.4.11
done 

Comment 11

[Sergei Dolgov]

Ooops! This checkin to MOZILLA_1_8_BRANCH somewhat broke wheel scrolling.
More exactly - wheel scrolling in IFRAME.
Reason - scrollEvent.point should be used in 1.8
while trunk moved to scrollEvent.refPoint.

That mistake was hardly detectable at time - all compiled and kind of worked.

Wondering if I should open special bug for that.

Comment 12

[Doug Shelton]

Bug 359081 opened for regression.

Comment 13

[Niels Reedijk]

This would only require a quick fix, right?

Comment 14

[Doug Shelton]

(In reply to comment #13)
> This would only require a quick fix, right?
> 
Yes, it's a two-line change in BeOS-only code.  See new bug for patch.