[Webkit-unassigned] [Bug 14227] Scroll does not work with IBM Thinkpad

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=14227


Steve Falkenburg <sfalken at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #47983|review?                     |review+
               Flag|                            |




--- Comment #28 from Steve Falkenburg <sfalken at apple.com>  2010-02-03 00:13:54 PST ---
(From update of attachment 47983)

> +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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.