| Summary: | [Chromium][Linux] Support Gtk scrollwheel behavior on Linux Chromium | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Kinuko Yasuda <kinuko> |
| Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Minor | CC: | agl, commit-queue, eric, evan |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Attachments: | |||
|
Description
Kinuko Yasuda
2009-11-10 07:11:32 PST
Created attachment 42863 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium
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! 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 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. (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... Created attachment 42917 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.2)
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.
Created attachment 42920 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.3)
(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 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.) Created attachment 43027 [details]
Support Gtk scrollwheel behavior for horizontal scrollbars on Linux Chromium (Rev.4, added a link to the bug)
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> All reviewed patches have been landed. Closing bug. |