Bug 29348

Summary: [Gtk] Scrollwheel on horizontal scrollbars should slide horizontally
Product: WebKit Reporter: Reinout van Schouwen <reinouts>
Component: WebKitGTKAssignee: Gustavo Noronha (kov) <gustavo>
Status: RESOLVED FIXED    
Severity: Minor CC: gustavo, xan.lopez
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 29693    
Bug Blocks:    
Attachments:
Description Flags
proposed fix
gustavo: commit-queue-
proposed fix
gustavo: commit-queue-
patch using proper signal emission technique
oliver: review-, gustavo: commit-queue-
losing deltaX in favor of deltaY, this time oliver: review+, gustavo: commit-queue-

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!