Bug 31292 - [Chromium][Linux] Support Gtk scrollwheel behavior on Linux Chromium
Summary: [Chromium][Linux] Support Gtk scrollwheel behavior on Linux Chromium
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Minor
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-10 07:11 PST by Kinuko Yasuda
Modified: 2009-11-12 01:03 PST (History)
4 users (show)

See Also:


Attachments
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (1.22 KB, patch)
2009-11-10 07:19 PST, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.2) (1.85 KB, patch)
2009-11-10 19:41 PST, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.3) (1.85 KB, patch)
2009-11-10 20:28 PST, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.4, added a link to the bug) (1.90 KB, patch)
2009-11-11 20:04 PST, Kinuko Yasuda
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kinuko Yasuda 2009-11-10 07:11:32 PST
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
Comment 1 Kinuko Yasuda 2009-11-10 07:19:03 PST
Created attachment 42863 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium
Comment 2 Adam Langley 2009-11-10 12:45:59 PST
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!
Comment 3 Evan Martin 2009-11-10 13:05:57 PST
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?
Comment 4 Eric Seidel (no email) 2009-11-10 13:10:47 PST
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.
Comment 5 Kinuko Yasuda 2009-11-10 19:39:40 PST
(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...
Comment 6 Kinuko Yasuda 2009-11-10 19:41:34 PST
Created attachment 42917 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.2)
Comment 7 Eric Seidel (no email) 2009-11-10 20:15:08 PST
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.
Comment 8 Kinuko Yasuda 2009-11-10 20:28:04 PST
Created attachment 42920 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.3)
Comment 9 Kinuko Yasuda 2009-11-10 20:46:21 PST
(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!
Comment 10 David Levin 2009-11-11 16:47:04 PST
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
> +2009-11-10  Kinuko Yasuda  <kinuko@google.com>
> +
> +        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.)
Comment 11 Kinuko Yasuda 2009-11-11 20:04:59 PST
Created attachment 43027 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.4, added a link to the bug)
Comment 12 WebKit Commit Bot 2009-11-12 01:03:40 PST
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>
Comment 13 WebKit Commit Bot 2009-11-12 01:03:46 PST
All reviewed patches have been landed.  Closing bug.