Bug 49830 – WebKit2: Add Trackpoint driver hack to support IBM trackpads

• Component: WebKit2
• Status: RESOLVED FIXED

[Sam Weinig]

We need to add the Trackpoint driver hack to support IBM trackpads that we had in WebKit1.

[Sam Weinig]

<rdar://problem/8705951>

[Brian Weinstein]

Created attachment 86809 [details]
[PATCH] Fix

[Adam Roben (:aroben)]

Comment on attachment 86809 [details]
[PATCH] Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=86809&action=review

> Source/WebKit2/UIProcess/win/WebView.cpp:294
> +        // trackpoint scrollbars, so the WebView will receive WM_VSCROLl and WM_HSCROLL messages.

They aren't "trackpoint scrollbars", they're just normal Windows scrollbars.

WM_VSCROLl should be WM_VSCROLL.

> Source/WebKit2/UIProcess/win/WebView.cpp:742
> +    const TCHAR trackPointKeys[][50] = { TEXT("Software\\Lenovo\\TrackPoint"),
> +        TEXT("Software\\Lenovo\\UltraNav"),
> +        TEXT("Software\\Alps\\Apoint\\TrackPoint"),
> +        TEXT("Software\\Synaptics\\SynTPEnh\\UltraNavUSB"),
> +        TEXT("Software\\Synaptics\\SynTPEnh\\UltraNavPS2") };

I'd put the first key on its own line, and put the closing brace and semicolon on their own line.

I think a better type for trackPointKeys would be: const wchar_t* trackPointKeys[].

You should use L instead of TEXT().

> Source/WebKit2/UIProcess/win/WebView.cpp:744
> +    for (int i = 0; i < 5; ++i) {

WTF_ARRAY_LENGTH

> Source/WebKit2/UIProcess/win/WebView.cpp:746
> +        int readKeyResult = ::RegOpenKeyEx(HKEY_CURRENT_USER, trackPointKeys[i], 0, KEY_READ, &trackPointKey);

You should use ::RegOpenKeyExW.

> Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:336
> +void WebPage::scrollBy(uint32_t scrollDirection, uint32_t scrollGranularity)
> +{
> +    scroll(m_page.get(), static_cast<ScrollDirection>(scrollDirection), static_cast<ScrollGranularity>(scrollGranularity));
> +}

Why is this code duplicated in WebPageMac and WebPageWin? Just put it in WebPage.cpp.

We should probably check that the uint32_ts we get are in the correct range. You could use helper functions to do that (like toScrollDirection and toScrollGranularity).

[Early Warning System Bot]

Attachment 86809 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8235541

[Brian Weinstein]

Created attachment 86821 [details]
[PATCH] Fix v2

[Adam Roben (:aroben)]

Comment on attachment 86821 [details]
[PATCH] Fix v2

View in context: https://bugs.webkit.org/attachment.cgi?id=86821&action=review

> Source/WebKit2/UIProcess/win/WebView.cpp:294
> +        // scrollbars, so the WebView will receive WM_VSCROLl and WM_HSCROLL messages.

WM_VSCROLl should be WM_VSCROLL.

> Source/WebKit2/UIProcess/win/WebView.cpp:746
> +    for (int i = 0; i < WTF_ARRAY_LENGTH(trackPointKeys); ++i) {

i should be a size_t.

[Brian Weinstein]

Landed in r81895.

[Darin Adler]

Comment on attachment 86809 [details]
[PATCH] Fix

This breaks the Qt build by adding scrollBy but not implementing it for Qt. Probably same for GTK.

[Brian Weinstein]

(In reply to comment #8)
> (From update of attachment 86809 [details])
> This breaks the Qt build by adding scrollBy but not implementing it for Qt. Probably same for GTK.

The second patch I uploaded and landed put scrollBy in a cross-platform location and seemed to build fine on Qt. Thanks for catching that though!