RESOLVED FIXED 31292
[Chromium][Linux] Support Gtk scrollwheel behavior on Linux Chromium
https://bugs.webkit.org/show_bug.cgi?id=31292
Summary [Chromium][Linux] Support Gtk scrollwheel behavior on Linux Chromium
Kinuko Yasuda
Reported 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
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
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
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
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
Kinuko Yasuda
Comment 1 2009-11-10 07:19:03 PST
Created attachment 42863 [details] Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium
Adam Langley
Comment 2 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!
Evan Martin
Comment 3 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?
Eric Seidel (no email)
Comment 4 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.
Kinuko Yasuda
Comment 5 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...
Kinuko Yasuda
Comment 6 2009-11-10 19:41:34 PST
Created attachment 42917 [details] Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.2)
Eric Seidel (no email)
Comment 7 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.
Kinuko Yasuda
Comment 8 2009-11-10 20:28:04 PST
Created attachment 42920 [details] Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.3)
Kinuko Yasuda
Comment 9 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!
David Levin
Comment 10 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.)
Kinuko Yasuda
Comment 11 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)
WebKit Commit Bot
Comment 12 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>
WebKit Commit Bot
Comment 13 2009-11-12 01:03:46 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.