[Webkit-unassigned] [Bug 101516] Handle gesture events on scrollbars.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=101516





--- Comment #8 from Robert Flack <flackr at chromium.org>  2012-11-13 15:34:37 PST ---
Thanks again.

(In reply to comment #6)
> (From update of attachment 173915 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=173915&action=review
> 
> Did you check how does this added test behavior for ports that disable scrollbars?
> 
> > Source/WebCore/platform/Scrollbar.cpp:377
> > +    if (evt.type() == PlatformEvent::GestureTapDown) {
> > +        setPressedPart(theme()->hitTest(this, evt.position()));
> > +        m_pressedPos = (orientation() == HorizontalScrollbar ? convertFromContainingWindow(evt.position()).x() : convertFromContainingWindow(evt.position()).y());
> > +        return true;
> > +    }
> > +    if (m_pressedPart == ThumbPart && (evt.type() == PlatformEvent::GestureTapDownCancel || evt.type() == PlatformEvent::GestureScrollBegin))
> > +        return true;
> > +    if (m_pressedPart == ThumbPart && evt.type() == PlatformEvent::GestureScrollUpdate) {
> > +        moveThumb(m_pressedPos + (m_orientation == HorizontalScrollbar ? evt.deltaX() : evt.deltaY()), false);
> > +        return true;
> > +    }
> > +    if (evt.type() == PlatformEvent::GestureTap) {
> > +        if (m_pressedPart == ThumbPart || m_pressedPart == NoPart)
> > +            return false;
> > +        if (m_scrollableArea)
> > +            return m_scrollableArea->scroll(pressedPartScrollDirection(), pressedPartScrollGranularity());
> > +    }
> 
> By looking at this, would it look nicer if you handle all type's in a switch/case block instead?

Done. Also, I there was some incorrect handling of the gesture events. Originally I had used the event position, but when running LayoutTests sending scroll updates doesn't change the test event position and this wouldn't work well if we wanted to do things like fling. Deltas are however incremental so I have to accumulate these.

> 
> > LayoutTests/fast/events/touch/gesture/gesture-scrollbar.html:51
> > +function runTest()
> > +{
> > +    internals.settings.setMockScrollbarsEnabled(true);
> 
> Did you check how this added test behavior for ports that disable scrollbars? AH, maybe setMockScrollbarsEnabled take care of it...

Are there ports which disable scrollbars or are we talking about overlay scrollbars (in which case cr-android seems to be passing)? It seems there is a mouse thumb dragging test in scrollbars/scrollbar-drag-thumb-with-large-content.html and no platform specific expectations / exclusions. setMockScrollbarsEnabled as far as I can see from the layout test it only changes the style of the root scrollbars, so I've added a style to the scrollbars to ensure consistent sizing in all platforms.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.