Bug 31292 – [Chromium][Linux] Support Gtk scrollwheel behavior on Linux Chromium

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Kinuko Yasuda]

Scrollwheel on horizontal scrollbars should slide horizontally on Linux Chromium too.  The change for Gtk has been made in r48735 (bug 29348), but it does not cover Linux Chromium.

Chromium side's bug is: http://code.google.com/p/chromium/issues/detail?id=24175

[Kinuko Yasuda]

Created attachment 42863 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium

[Adam Langley]

In general: LGTM. Note that I am not a WebKit reviewer and you need a real review first.

However, your patch is lacking ChangeLog entry which is required for WebKit patches:

http://webkit.org/coding/contributing.html

If you don't wish to deal with ChangeLogs etc you can also ask someone to take up the patch and carry it to landing, but thanks for the code!

[Evan Martin]

These complicated ifdefs always make me a little scared.

Eric, what's the precedent on WebKit for adding more detailed names for features?  E.g. should we instead #define HORIZONAL_MOUSEWHEEL somewhere?

[Eric Seidel (no email)]

Comment on attachment 42863 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium

Every change requires a ChangeLog.  See http://webkit.org/coding/contributing.html

Otherwise looks fine.

the code comes
 160 // from gtk/EventHandlerGtk.cpp.

Sad we can't share more code with EventHanderGtk...

r- for lack of ChangeLog.

[Kinuko Yasuda]

(In reply to comment #4)
> (From update of attachment 42863 [details])
> Every change requires a ChangeLog.  See
> http://webkit.org/coding/contributing.html
> 
> Otherwise looks fine.
> 
> the code comes
>  160 // from gtk/EventHandlerGtk.cpp.
> 
> Sad we can't share more code with EventHanderGtk...
> 
> r- for lack of ChangeLog.

Sorry forgot to include the ChangeLog changes.  Attaching a new patch...

[Kinuko Yasuda]

Created attachment 42917 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.2)

[Eric Seidel (no email)]

Comment on attachment 42917 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.2)

horizontall is misspelled.  horizontal is what you meant.

[Kinuko Yasuda]

Created attachment 42920 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.3)

[Kinuko Yasuda]

(In reply to comment #7)
> (From update of attachment 42917 [details])
> horizontall is misspelled.  horizontal is what you meant.

Hmm, good catch, I updated the patch.  Thanks!

[David Levin]

Comment on attachment 42920 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.3)

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> [phone]  Kinuko Yasuda  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Support Gtk scrollwheel behavior for horizontal scrollbars on Linux
> +        Chromium too.

Please include a link to the bug here. (Perhaps you can get another committer another you to land this and fix this one issue.)

[Kinuko Yasuda]

Created attachment 43027 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.4, added a link to the bug)

[WebKit Commit Bot]

Comment on attachment 43027 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.4, added a link to the bug)

Clearing flags on attachment: 43027

Committed r50871: <http://trac.webkit.org/changeset/50871>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.