Bug 96111 – Wheel-events sometimes reset scroll-position

• Component: WebKit2
• Status: RESOLVED INVALID

[Allan Sandfeld Jensen]

If a web-page catches a scroll or wheel event and changes the document, the resulting change in contentSize will cause PageViewportController to rescale the page and lose the scroll-position.

It can be fixed by setting HadUserInteraction when the PageViewportController receives a scroll request.

[Allan Sandfeld Jensen]

Created attachment 162775 [details]
Patch

[Allan Sandfeld Jensen]

Can be observed on http://heltnormalt.dk/striben

[Andras Becsi]

Comment on attachment 162775 [details]
Patch

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

> Source/WebKit2/UIProcess/PageViewportController.cpp:152
> +    // This may not be a user interaction, but in any case we do not want to lose the scroll position later due to resizing.
> +    setHadUserInteraction(true);

I wonder if it wouldn't be better to set the had user interaction flag from QQuickWebView::wheelEvent, since a wheel scroll is in fact a user interaction.
Although this would need the PageViewportControllerClientQt to expose a setter function.

[Allan Sandfeld Jensen]

(In reply to comment #3)
> (From update of attachment 162775 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=162775&action=review
> 
> > Source/WebKit2/UIProcess/PageViewportController.cpp:152
> > +    // This may not be a user interaction, but in any case we do not want to lose the scroll position later due to resizing.
> > +    setHadUserInteraction(true);
> 
> I wonder if it wouldn't be better to set the had user interaction flag from QQuickWebView::wheelEvent, since a wheel scroll is in fact a user interaction.
> Although this would need the PageViewportControllerClientQt to expose a setter function.

I guess the same problem could be replicated some way using an anchor. Jump to anchor, modify page, and the UI-process scrolls back to 0,0.

[Andras Becsi]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 162775 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=162775&action=review
> > 
> > > Source/WebKit2/UIProcess/PageViewportController.cpp:152
> > > +    // This may not be a user interaction, but in any case we do not want to lose the scroll position later due to resizing.
> > > +    setHadUserInteraction(true);
> > 
> > I wonder if it wouldn't be better to set the had user interaction flag from QQuickWebView::wheelEvent, since a wheel scroll is in fact a user interaction.
> > Although this would need the PageViewportControllerClientQt to expose a setter function.
> 
> I guess the same problem could be replicated some way using an anchor. Jump to anchor, modify page, and the UI-process scrolls back to 0,0.

I see. In this case I think it would be good to also have a QML testcase so we do not reintroduce this issue.

[Allan Sandfeld Jensen]

This issue does not appear to be valid anymore. It was probably fixed by refactoring in how resizing works.

[Eric Seidel (no email)]

Comment on attachment 162775 [details]
Patch

Cleared review? from attachment 162775 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).