Bug 14227 – Scroll does not work with IBM Thinkpad

• Component: New Bugs
• Status: RESOLVED FIXED

[Cameo Wood]

I have a laptop with a "IBM ThinkPad UltraNav Pointing Device.
The driver I am using is "Synaptics" from 7/16/2004, driver version 75.17.12 on Windows XP SP2.  

The scroll does work in Word, FF, and IE.  All other mouse actions work fine.

[Dave Hyatt]

Would be happy to fix this... just need to know what messages are sent.

[Cameo Wood]

(In reply to comment #1)
> Would be happy to fix this... just need to know what messages are sent.
> 

Tell me how I can help, and I will! 

[Cameo Wood]

Created attachment 15125 [details]
Mouse messages while scrolling FF page with a T42 Thinkpad scroll device

[Cameo Wood]

Created attachment 15126 [details]
Mouse messages while scrolling Safari page with a T42 Thinkpad scroll device

[Mark Rowe (bdash)]

Thanks Cameo!

[Cameo Wood]

In these files, I performed the following actions in each window:

1. moved my mouse into the browser window
2 clicked the blue "scroll mouse" button on the thinkpad
3. moved my "IBM ThinkPad UltraNav Pointing Device" [TM] up, then down twice.
4. moved my mouse away from the window.

I hope this helps!  I was looking for some WM_SCROLL but I wasn't seeing that at all. 

I also found a blog post by someone solving this issue for another piece of software:

http://blogs.msdn.com/markrideout/archive/2006/01/12/512359.aspx

hope that helps!  Thanks, fellows!

[Mark Rowe (bdash)]

<rdar://problem/5279370>

[Dave Hyatt]

Firefox has no native scrollbar.  We should probably figure out how they managed to work with this.

[Cameo Wood]

Let me know if I can help further.

[Anton Mostovoy]

FYI, I have the same problem, so it's not an isolated occurrence.

*** Bug 14098 has been marked as a duplicate of this bug. ***

[David Kilzer (:ddkilzer)]

See also bug 14990.

[Soren]

(In reply to comment #1)
> Would be happy to fix this... just need to know what messages are sent.
> 

In the new 3.0.4 build for Windows, the funny thing is that I can use the trackpoint for scrolling in the Bookmarks (CTRL-ALT-B) list, but still not in on webpages. Maybe it helps you a little

[Cameo Wood]

I upgraded to the latest synaptics driver from 8/10/2007, version 7.5.17.25, and the scroll still doesn't work.

[Anton Mostovoy]

I found this info here (http://levelsofdetail.kendeeter.com/2007/06/safari_browser_on_windows.html) and it worked for me.
In my case tp4table.dat is in C:\Program Files\Synaptics\SynTP

begin quote
Edit C:\windows\system32\tp4table.dat. Towards the bottom of this file, right before the [AutoScrollTable] section, I added the following lines:

;iTunes/Safari Scroll Fix
*,*,itunes.exe,*,*,*,WheelStd,0,9
*,*,Safari.exe,*,*,*,WheelStd,0,9
end quote

[Anton Mostovoy]

(In reply to comment #15)
Correct link is http://levelsofdetail.kendeeter.com/2007/06/make_the_thinkpads_middle_butt.html

[Cameo Wood]

followed anton's instructions, and still no dice.  Not sure what's not working.  I edited the file and then rebooted, and still it doesn't work.

[Cameo Wood]

At sfalken's recommendation, I tried editing SynTPEnh.ini with:

[Safari]
FC = "WebViewWindowClass"
SF = 0x10000000
SF |= 0x00004000

Still doesn't work.

[Cameo Wood]

Also, to clarify.  This is to scroll using the middle scroll button and the pointstick, not the touchpad.

[Maxime Britto]

It would be great to test this again on the IBM ThinkPad UltraNav Pointing Device with the changes from r35083
It seems to be the same bug as #16424 which is now fixed (except if the blue "scroll mouse" button is not assimilated as the middle mouse button) 

[Brent Fulgham]

*** Bug 22534 has been marked as a duplicate of this bug. ***

[Brent Fulgham]

This appears to be a problem somewhat specific to certain kinds of laptops. 
See "http://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/webinputevent_win.cc?view=markup"
for specific information on the Chromium logic to handle this.

Basically, "ThinkPad touchpads (or trackpoints) send WM_HSCROLL messages",
which probably need to be handled PlatformMouseEventWin.cpp.

[Kyle Huey (Mozilla Developer)]

I just went through fixing this over at https://bugzilla.mozilla.org/show_bug.cgi?id=507222 .  The trackpoint driver used a nasty hack to work on Firefox and other Mozilla products that broke as a result of some internal changes.  The patches are there for viewing and pretty straightforward, and I'm more than willing to provide advice or answer questions on what I did to get this working.

[Brian Weinstein]

Created attachment 47976 [details]
Patch

[WebKit Review Bot]

Attachment 47976 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/win/WebView.cpp:1599:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
WebKit/win/WebView.cpp:1630:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Total errors found: 2


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brian Weinstein]

(In reply to comment #25)
> Attachment 47976 [details] did not pass style-queue:
> 
> Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
> WebKit/win/WebView.cpp:1599:  A case label should not be indented, but line up
> with its switch statement.  [whitespace/indent] [4]
> WebKit/win/WebView.cpp:1630:  A case label should not be indented, but line up
> with its switch statement.  [whitespace/indent] [4]
> Total errors found: 2
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

Every other switch in this file had the case labels indented, in this case, I feel going against the style bot is right, but could be convinced either way.

[Brian Weinstein]

Created attachment 47983 [details]
Patch

[Steve Falkenburg]

Comment on attachment 47983 [details]
Patch


> +void WebView::initializeTrackpointHackIfNecessary(HWND parentWindow)
> +{
> +    const TCHAR trackpointKeys[][50] = {L"Software\\Lenovo\\TrackPoint",
> +        L"Software\\Lenovo\\UltraNav",
> +        L"Software\\Alps\\Apoint\\TrackPoint",
> +        L"Software\\Synaptics\\SynTPEnh\\UltraNavUSB",
> +        L"Software\\Synaptics\\SynTPEnh\\UltraNavPS2"};

Mixing TCHAR and L" isn't really correct (but won't cause any issues in practice for us...) since TCHAR's size depends on whether UNICODE is defined, while L" is always UTF-16.
You could switch from L" to TEXT( or switch from TCHAR to WCHAR. Since you're passing these into Win32 APIs not suffixed with A or W, TCHAR seems more appropriate.

> +    for (int i = 0; i < 5; i++) {

We use ++i in most places.

> +        int readkeyResult = ::RegOpenKeyEx(HKEY_CURRENT_USER, (LPCWSTR)&trackpointKeys[i], 0, KEY_READ, &trackpointKey);

We don't use C style casts in our code. Please change this to a C++ style cast. And cast to LPCTSTR instead.

Strange casing of your variables here. How about:

trackPointKey
readKeyResult

> +        if (readkeyResult == ERROR_SUCCESS) {
> +            // If we detected a registry key belonging to a TrackPoint driver, then create fake trackpoint
> +            // scrollbars, so the WebView will receive WM_VSCROLL and WM_HSCROLL messages. We create one
> +            // vertical scrollbar and one horizontal to allow for receiving both types of messages.
> +            ::CreateWindow(L"SCROLLBAR", L"FAKETRACKPOINTHSCROLLBAR", WS_CHILD | WS_VISIBLE | SBS_HORZ, 0, 0, 0, 0, parentWindow, 0, gInstance, 0);
> +            ::CreateWindow(L"SCROLLBAR", L"FAKETRACKPOINTVSCROLLBAR", WS_CHILD | WS_VISIBLE | SBS_VERT, 0, 0, 0, 0, parentWindow, 0, gInstance, 0);

More L"" use in non-suffixed Windows calls. Nitpicky, since it won't really create issues unless somebody tried to build this without UNICODE.

>  HRESULT STDMETHODCALLTYPE WebView::initWithFrame( 

I don't believe the STDMETHODCALLTYPE is really required here.

>      /* [in] */ RECT frame,
> @@ -2404,6 +2495,8 @@ HRESULT STDMETHODCALLTYPE WebView::initW
>          frame.left, frame.top, frame.right - frame.left, frame.bottom - frame.top, m_hostWindow ? m_hostWindow : HWND_MESSAGE, 0, gInstance, 0);
>      ASSERT(::IsWindow(m_viewWindow));
>  
> +    initializeTrackpointHackIfNecessary(m_viewWindow);
> +

We should cache this code so we don't access the registry unnecessarily every time we create a WebView! It is unlikely this driver will get installed while we're running.

[Brian Weinstein]

Created attachment 48054 [details]
Patch

[Steve Falkenburg]

Comment on attachment 48054 [details]
Patch

> +    Frame* frame = m_page->focusController()->focusedOrMainFrame();

You can move this line down to near the end of the method (in both horizontal and vertical scrolling methods).

> +bool WebView::shouldInitializeTrackPointHack()
> +{
> +    static bool shouldCreateScrollbars = false;
> +    static bool hasRunTrackPointCheck = false;

No need to init these - they will be inited to false for you.

> +    shouldCreateScrollbars = false;

This is already false.

[Brian Weinstein]

Landed in r54293.