After reloading some pages, wheel events will temporarily fail to work until the page has been scrolled by some other means. The problem is restricted to ports using resizesToContents, possibly only Qt.
Created attachment 160415 [details] Patch
Comment on attachment 160415 [details] Patch How do we test this? We can do reloads and synthesized wheel events in DRT....
(In reply to comment #2) > (From update of attachment 160415 [details]) > How do we test this? We can do reloads and synthesized wheel events in DRT.... If we can set the fixed visible rect and resizes to content, then it should be possible to test it simple by reloading a page that is larger than the visible rect, and checking the size of the fixed visible rect after the reload. I will take a look to see if it is possible.
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 160415 [details] [details]) > > How do we test this? We can do reloads and synthesized wheel events in DRT.... > > If we can set the fixed visible rect and resizes to content, then it should be possible to test it simple by reloading a page that is larger than the visible rect, and checking the size of the fixed visible rect after the reload. > > I will take a look to see if it is possible. It should at least be possible to test this with a QML test, see tst_wheelEventHandling.qml.
Created attachment 160941 [details] Patch Now with a QML test. LayoutTests does not support fixedVisibleContentRect
Comment on attachment 160941 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160941&action=review > Source/WebKit2/ChangeLog:8 > + Set fixed visible content rect using, WebPage::viewportSize instead of WebPage::size when available. There's a stray comma in the sentence :) > Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_wheelEventHandling.qml:49 > + var position = webView.contentY This seems to be an unused variable. > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:1243 > + if (shouldUseFixedLayout) { > +#if USE(TILED_BACKING_STORE) > + m_frame->coreFrame()->view()->setFixedVisibleContentRect(WebCore::IntRect(WebCore::IntPoint(), webPage->viewportSize())); > +#else > m_frame->coreFrame()->view()->setFixedVisibleContentRect(webPage->bounds()); > +#endif I admit that I don't know this code well enough to feel truly comfortable reviewing this. I would prefer if Kenneth could do that.
Comment on attachment 160941 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160941&action=review >> Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:1243 >> + if (shouldUseFixedLayout) { >> +#if USE(TILED_BACKING_STORE) >> + m_frame->coreFrame()->view()->setFixedVisibleContentRect(WebCore::IntRect(WebCore::IntPoint(), webPage->viewportSize())); >> +#else >> m_frame->coreFrame()->view()->setFixedVisibleContentRect(webPage->bounds()); >> +#endif > > I admit that I don't know this code well enough to feel truly comfortable reviewing this. I would prefer if Kenneth could do that. I don't think shouldUseFixedLayout will ever be true if USE(TILED_BACKING_STORE) is false. Also I think that logically the visible rect that should be applied is the one last sent by the UI process, anything else is artificial. Would it be possible to do this in Frame::createView and transfer the fixedVisibleContentRect form the previously attached FrameView when creating the new one? Or do it from the outside here rather than guessing it from the page which might still be incomplete at this point?
(In reply to comment #7) > I don't think shouldUseFixedLayout will ever be true if USE(TILED_BACKING_STORE) is false. > Maybe not, but API-wise we need to atleast handle it somehow, since shouldUseFixedLayout can be set without TILED_BACKING_STORE being active. > Also I think that logically the visible rect that should be applied is the one last sent by the UI process, anything else is artificial. Would it be possible to do this in Frame::createView and transfer the fixedVisibleContentRect form the previously attached FrameView when creating the new one? Or do it from the outside here rather than guessing it from the page which might still be incomplete at this point? The page here is both the old page and the new page, and reading viewPortSize reads the last size that used to set the fixedVisibleContentRect size. At least as far as I know, the fixed visible content rect is always set as a consequence of WebPage::setViewportSize(). The only thing missing from the fixedVisibleRect the position, but the position is restored separately someplace else.
> > I don't think shouldUseFixedLayout will ever be true if USE(TILED_BACKING_STORE) is false. That is the case for BlackBerry: fixed layout: true,backing store: false (we use our own).
Created attachment 161228 [details] Patch Changed to use WebFrame::visibleContentBounds which is always available, cleaned up changelog and test-case
(In reply to comment #9) > > > > I don't think shouldUseFixedLayout will ever be true if USE(TILED_BACKING_STORE) is false. > > That is the case for BlackBerry: fixed layout: true,backing store: false (we use our own). Ah, good to know!
(In reply to comment #11) > (In reply to comment #9) > > > > > > I don't think shouldUseFixedLayout will ever be true if USE(TILED_BACKING_STORE) is false. > > > > That is the case for BlackBerry: fixed layout: true,backing store: false (we use our own). > > Ah, good to know! Of course BlackBerry does not use WebKit2 (yet), so you are both right ;)
Comment on attachment 161228 [details] Patch Clearing flags on attachment: 161228 Committed r127420: <http://trac.webkit.org/changeset/127420>
All reviewed patches have been landed. Closing bug.