Bug 29348 - [Gtk] Scrollwheel on horizontal scrollbars should slide horizontally
Summary: [Gtk] Scrollwheel on horizontal scrollbars should slide horizontally
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Minor
Assignee: Gustavo Noronha (kov)
URL:
Keywords: Gtk
Depends on: 29693
Blocks:
  Show dependency treegraph
 
Reported: 2009-09-17 14:51 PDT by Reinout van Schouwen
Modified: 2009-09-24 14:59 PDT (History)
2 users (show)

See Also:


Attachments
proposed fix (15.44 KB, patch)
2009-09-18 06:15 PDT, Gustavo Noronha (kov)
gns: commit-queue-
Details | Formatted Diff | Diff
proposed fix (16.39 KB, patch)
2009-09-20 18:18 PDT, Gustavo Noronha (kov)
gns: commit-queue-
Details | Formatted Diff | Diff
patch using proper signal emission technique (16.36 KB, patch)
2009-09-23 14:13 PDT, Gustavo Noronha (kov)
oliver: review-
gns: commit-queue-
Details | Formatted Diff | Diff
losing deltaX in favor of deltaY, this time (16.36 KB, patch)
2009-09-23 17:31 PDT, Gustavo Noronha (kov)
oliver: review+
gns: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Reinout van Schouwen 2009-09-17 14:51:27 PDT
Prompted by https://bugzilla.gnome.org/show_bug.cgi?id=595493 filed against the Gecko backend, but valid for Webkit as well.
The Gtk scrollbar behaviour should be followed so that using the scroll wheel on horizontal scrollbars slides horizontally, instead of vertically.
Comment 1 Gustavo Noronha (kov) 2009-09-17 14:55:14 PDT
Notice that the wrong behavior only happens with scrollbars created by webcore - if you try scrolling like that with scrollbars handled by GtkScrolledWindow (which you get by making the browser window too small on this bug report, for example) it will work.
Comment 2 Gustavo Noronha (kov) 2009-09-18 06:15:10 PDT
Created attachment 39757 [details]
proposed fix
Comment 3 Gustavo Noronha (kov) 2009-09-20 18:18:46 PDT
Created attachment 39838 [details]
proposed fix

Patch updated addressing suggestion by olliej - avoiding an #if PLATFORM inside the handler itself, calling a method that can be overriden by the platform instead.
Comment 4 Xan Lopez 2009-09-21 12:18:48 PDT
(In reply to comment #3)
> Created an attachment (id=39838) [details]
> proposed fix
> 
> Patch updated addressing suggestion by olliej - avoiding an #if PLATFORM inside
> the handler itself, calling a method that can be overriden by the platform
> instead.

Just a small comment, as mentioned in bug #29203 we should be at least using gtk_widget_event here, which at least emits all the signals needed to correctly create events.
Comment 5 Gustavo Noronha (kov) 2009-09-23 14:13:20 PDT
Created attachment 40017 [details]
patch using proper signal emission technique
Comment 6 Oliver Hunt 2009-09-23 15:48:08 PDT
Comment on attachment 40017 [details]
patch using proper signal emission technique


> +        void turnVerticalTicksIntoHorizontal()
> +        {
> +            m_deltaX += m_deltaY;
> +            m_deltaY = 0;
> +
> +            m_wheelTicksX += m_wheelTicksY;
> +            m_wheelTicksY = 0;
> +        }

I'm unhappy with this function, either it is dropping deltax entirely -- m_deltaX = m_deltaY -- or it is rotating the orientation 90 degrees, in which case x and y delta, etc should be swapped.
Comment 7 Gustavo Noronha (kov) 2009-09-23 16:08:08 PDT
(In reply to comment #6)
> (From update of attachment 40017 [details])
> 
> > +        void turnVerticalTicksIntoHorizontal()
> > +        {
> > +            m_deltaX += m_deltaY;
> > +            m_deltaY = 0;
> > +
> > +            m_wheelTicksX += m_wheelTicksY;
> > +            m_wheelTicksY = 0;
> > +        }
> 
> I'm unhappy with this function, either it is dropping deltax entirely --
> m_deltaX = m_deltaY -- or it is rotating the orientation 90 degrees, in which
> case x and y delta, etc should be swapped.

I can do any of those, but that will not match what GTK+ does (GTK+ does sum the y ticks into the x ones in my tests), so we need to customize it somehow. What do you suggest in this case? Drop deltax for the default implementation, and make a GTK+-specific implementation that behaves like GTK+?
Comment 8 Gustavo Noronha (kov) 2009-09-23 17:31:02 PDT
Created attachment 40032 [details]
losing deltaX in favor of deltaY, this time
Comment 9 Gustavo Noronha (kov) 2009-09-23 17:31:35 PDT
(In reply to comment #8)
> Created an attachment (id=40032) [details]
> losing deltaX in favor of deltaY, this time

As I commented on IRC:

I did not find the code that handles this, but changing the code to lose deltaX doesn't really change my feel - GTK+ isn't really able to represent both directions in the same event, tbh, so maybe the point is moot after all.
Comment 10 Oliver Hunt 2009-09-24 14:10:33 PDT
Comment on attachment 40032 [details]
losing deltaX in favor of deltaY, this time

r=me
Comment 11 Gustavo Noronha (kov) 2009-09-24 14:59:53 PDT
Landed as r48735, thanks!