Bug 75006 – [Qt] Scrolling in touch mode

• Component: WebKit2
• Status: RESOLVED FIXED

[Allan Sandfeld Jensen]

This bug is created to track several related problem with scrolling in Qt Webkit2 touch-interface mode.

When web elements are set as scrollable with overflow:auto/scroll, they are currently neither displayed as scrollable, nor actually scrollable from the touch interface in Qt Webkit2. There is a patch 
https://gitorious.org/+qtwebkit-webkit2-dev/webkit/qtwebkit-webkit2-dev/commit/6c2e96afc4b384bfcb40008abb4eee6e3ad8ac81
that enables twofinger scrolling in these elements, but still doesn't indicate the elements are scrollable, nor make them scrollable by mouse or keyboard.

A related problem is that the even the root web-element/page is not scrollable by keyboard in WebKit2 touch-mode. 

The central problem is that traditional desktop scrolling is handled by WebCore, while touch-scrolling is handled in the Qt Platform code of the UI-process. WebCore ignores all the scroll-events in touch mode because scrollbars are absent which in WebCore indicates non-scrollable elements (overflow: hidden/visible), and the Qt Platform code does not yet duplicate all WebCore scrolling.

There needs to be a closer cooperation between the two scrolling implementations, or alternatively the touch interface implementation needs to be greatly expanded.

[Kenneth Rohde Christiansen]

I cannot agree more. Something related it looking into using a separate AC layer for div overflow and use our own interaction/physics engine with it.

I believe that the latest iOS WebKit code dump have patches related to this. RIM have also done something similar and it would be possible to talk to Antonio about that (cc'ing him).

[Allan Sandfeld Jensen]

Created attachment 120931 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 120931 [details]
Patch

Thinking a bit more about this, I am not sure this is the best approach.

It would be better to move the smooth scrolling to the UI side for our case and check whether an event listener was added for the wheel event/page up/page down etc. And as long as that is not added we should just scroll on the ui side immediately. If one of these events are being listened to, we need to send the event to WebCore and if it is not prevented act on it on the UI side.

Doing it this way would make it very similar to how we were handling touch events in the N9 browser.

[Allan Sandfeld Jensen]

Just to clarify. There is no smooth scrolling in the this patch. It is only keyboard and mouse-wheel scrolling. 

It works by letting the events be handled by webcore, but having webcore request the final scrolling if any from the UI process.

[Antonio Gomes]

(In reply to comment #4)
> Just to clarify. There is no smooth scrolling in the this patch. It is only keyboard and mouse-wheel scrolling. 
> 
> It works by letting the events be handled by webcore, but having webcore request the final scrolling if any from the UI process.

that sounds similar to what we did, though not so much. I will take a deeper look at the patch soon.

[Allan Sandfeld Jensen]

Comment on attachment 120931 [details]
Patch

Currently being revised. Removing review.

[Allan Sandfeld Jensen]

Created attachment 131312 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 131312 [details]
Patch

Doesn't this mean that it will scroll in different steps depending on your zoom level?

[Allan Sandfeld Jensen]

(In reply to comment #8)
> (From update of attachment 131312 [details])
> Doesn't this mean that it will scroll in different steps depending on your zoom level?

Yes, it will always try to scroll 3 lines, which can be a small step on fully zoomed out page. But keep in mind that this is for a special case of getting actual wheel-mouse events on a touch-based browser (desktop/touch hybrid). It could be mouse over a 'overflow:scroll' element that is only a part of a large page. Should this sub-element scroll faster because the page is zoomed out?

Additionally I think some of the code (even the DOM wheel-event spec) demands the mouse-wheel delta is in multipla of 120. So zooming the scroll-delta might not be that doable without breaking some assumptions and the DOM event spec.

[Kenneth Rohde Christiansen]

(In reply to comment #9)
> (In reply to comment #8)
> > (From update of attachment 131312 [details] [details])
> > Doesn't this mean that it will scroll in different steps depending on your zoom level?
> 
> Yes, it will always try to scroll 3 lines, which can be a small step on fully zoomed out page. But keep in mind that this is for a special case of getting actual wheel-mouse events on a touch-based browser (desktop/touch hybrid). It could be mouse over a 'overflow:scroll' element that is only a part of a large page. Should this sub-element scroll faster because the page is zoomed out?
> 
> Additionally I think some of the code (even the DOM wheel-event spec) demands the mouse-wheel delta is in multipla of 120. So zooming the scroll-delta might not be that doable without breaking some assumptions and the DOM event spec.

So does it actually scroll larger distances for larger fonts?

The spec could be considered to deal with a scale of 1.0 as this probably wasn't considered when it was written. Scrolling quite slowly when zoomed out, seems quite weird to me.

[Kenneth Rohde Christiansen]

Comment on attachment 131312 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=131312&action=review

> Source/WebCore/platform/ScrollView.cpp:370
>      repaintFixedElementsAfterScrolling();
> +#if USE(TILED_BACKING_STORE)
> +    if (delegatesScrolling()) {
> +        hostWindow()->delegatedScrollRequested(IntPoint(newOffset));
> +        return;
> +    }
> +#endif
>      scrollContents(scrollDelta);

Is this part specific to this patch?

[Allan Sandfeld Jensen]

(In reply to comment #11)
> (From update of attachment 131312 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=131312&action=review
> 
> > Source/WebCore/platform/ScrollView.cpp:370
> >      repaintFixedElementsAfterScrolling();
> > +#if USE(TILED_BACKING_STORE)
> > +    if (delegatesScrolling()) {
> > +        hostWindow()->delegatedScrollRequested(IntPoint(newOffset));
> > +        return;
> > +    }
> > +#endif
> >      scrollContents(scrollDelta);
> 
> Is this part specific to this patch?

Yes, this is heart of the patch. It takes redirects scrolling done by webcore and requests it from the hostWindow if scrolling is delegated to the hostWindow.

[Allan Sandfeld Jensen]

To support scaling wheel-events I first needed to fix several bugs in our support of fractional wheel-events. There is a patch for it in #84243, once in, the patch here can be updated.

[Allan Sandfeld Jensen]

Created attachment 141727 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 141727 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=141727&action=review

> Source/WebCore/page/FrameView.cpp:1725
> +    bool visibleSizeChanged = false;

I think we should use visibleSizeDidChange as that seems more common today in common code

[Allan Sandfeld Jensen]

Created attachment 141939 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 141939 [details]
Patch for landing

Clearing flags on attachment: 141939

Committed r117069: <http://trac.webkit.org/changeset/117069>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.