Bug 90550 – Wheel events on a page with frames are not handled in fixed layout

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Andras Becsi]

Since r117069 we also use ScrollView::updateScrollbars() if scroll is delegated (invisible scrollbars). This results in erroneous behavior in fixed layout mode on pages which use frames.
This is due to fixed layout enabling frame flattening in which case updateScrollbars disables the scrollbars on the page since the document size is equal to the frame size.

This is reproducible with MiniBrowser on the given URL by double tapping to zoom in and then trying to scroll.

[Andras Becsi]

Created attachment 150797 [details]
proposed patch

[Andras Becsi]

Comment on attachment 150797 [details]
proposed patch

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

> Source/WebCore/ChangeLog:17
> +        Need to figure out a way to test this, probably with
> +        a QML API test since WTR does not use frame flattening.

Turns out that there is testRunner.setFrameFlatteningEnabled(true), and there are layout tests for this. Need to check the Skipped list as well.

[Andras Becsi]

This patch in combination with the patch of bug 90547 also fixes the scrolling in a simple QML WebView loading a simple local html.

[Andras Becsi]

Comment on attachment 150797 [details]
proposed patch

Even though the patch fixes the scrolling issue it does not seem to be the right solution. I'll revisit this later. Removing patch from review queue.

[Allan Sandfeld Jensen]

Comment on attachment 150797 [details]
proposed patch

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

> Source/WebCore/platform/ScrollView.cpp:509
>          IntSize frameSize = frameRect().size();
> +        bool usesFixedLayout = useFixedLayout();
>  
>          if (hScroll == ScrollbarAuto) {
>              newHasHorizontalScrollbar = docSize.width() > visibleWidth();
> -            if (newHasHorizontalScrollbar && !m_updateScrollbarsPass && docSize.width() <= frameSize.width() && docSize.height() <= frameSize.height())
> +            if (newHasHorizontalScrollbar && !m_updateScrollbarsPass && !usesFixedLayout && docSize.width() <= frameSize.width() && docSize.height() <= frameSize.height())

I think the real problem here is the use of frameRect().size() since with fixedLayout the frameRect is always the size of the document. It should use visibleContentRect(true) instead.

[Allan Sandfeld Jensen]

Created attachment 154364 [details]
Patch

A quick alternative patch based on my observersion. This solves most of the problems, though some frames does not create the invisible scroll-bars until the page has been manipulated in some way. Though that is probably a separate bug with a missing updateScrollbar() call somewhere.

[Antonio Gomes]

Comment on attachment 154364 [details]
Patch

why is this Qt specific?

[Allan Sandfeld Jensen]

(In reply to comment #7)
> (From update of attachment 154364 [details])
> why is this Qt specific?

I guess it isn't as such, it is only specific to the way Qt uses FrameView, anyone using it the same way would have the same problem.

[Allan Sandfeld Jensen]

But besides the commit title, is there any other comments to the solution, or can it go in?

[Antonio Gomes]

By any chance, do you set fixedLayout for non-mainframe ScrollViews?

[Allan Sandfeld Jensen]

(In reply to comment #10)
> By any chance, do you set fixedLayout for non-mainframe ScrollViews?

I don't think so, not unless it is a part of frame-flattening (I haven't checked that code in that much detail yet).

[Allan Sandfeld Jensen]

Committed r124024: <http://trac.webkit.org/changeset/124024>