Bug 43168 – [Qt] QtTestBrowser not setting preferredContentsSize for resizesToContents

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Antonio Gomes]

Using resizesToContents is working bad in our reference/demo browser, QtTestBrowser:

- we are not using this property together with setPreferredContentsSize (from QWebPage), which is explicitly adviced in our docs: http://doc.trolltech.com/4.7-snapshot/qgraphicswebview.html#resizesToContents-prop

- also the default value hardcoded when no preferred contents size is set is not good.

void QGraphicsWebViewPrivate::updateResizesToContentsForPage()
{
..
if (resizesToContents) {
  // resizes to contents mode requires preferred contents size to be set
  if (!page->preferredContentsSize().isValid())
    page->setPreferredContentsSize(QSize(960, 800));


** Also, scrolling gets broken on simple websites when we toggle it on, but it can be for another bug.

[Antonio Gomes]

Created attachment 63247 [details]
patch v1

Patch addresses the problem for QtTestBrowser by properly handle scene, webview and viewport sizes when toggle resizesToContents on/off.

[Kenneth Rohde Christiansen]

Comment on attachment 63247 [details]
patch v1

WebKitTools/QtTestBrowser/webview.cpp:91
 +          setVerticalScrollBarPolicy(Qt::ScrollBarAsNeeded);
Strange... if it resizes to contents, no scrollbars should be shown... why are they set  to as needed?

[Kenneth Rohde Christiansen]

Comment on attachment 63247 [details]
patch v1

WebKitTools/QtTestBrowser/webview.cpp:91
 +          setVerticalScrollBarPolicy(Qt::ScrollBarAsNeeded);
Strange... if it resizes to contents, no scrollbars should be shown... why are they set  to as needed?

WebKitTools/QtTestBrowser/webview.cpp:100
 +          scene()->setSceneRect(viewportRect);
shouldnt these be set automatically elsewhere so they are always consistent.

WebKitTools/QtTestBrowser/webview.cpp:93
 +          scene()->setSceneRect(QRectF());
why this?

WebKitTools/QtTestBrowser/launcherwindow.cpp:409
 +              toggleResizesToContents(true);
why not toggleResizesToContents(gResizesToContents) ?

[Antonio Gomes]

(In reply to comment #2)
> (From update of attachment 63247 [details])
> WebKitTools/QtTestBrowser/webview.cpp:91
>  +          setVerticalScrollBarPolicy(Qt::ScrollBarAsNeeded);
> Strange... if it resizes to contents, no scrollbars should be shown... why are they set  to as needed?

QGraphicsWebView *wont* have scrollbars as you said, but in order to make the content completely viewable (including offscreen content), the QGraphicsView still perform the scrolling, so it is set back to 'auto'.

QGraphicsView's viewport is what is going to be getting scrolled instead then.

> WebKitTools/QtTestBrowser/webview.cpp:100
>  +          scene()->setSceneRect(viewportRect);
> shouldnt these be set automatically elsewhere so they are always consistent.

In fact once setSceneRect(xxx) is called (see existing code in WebViewGraphicsBased::resizeEvent), scene will be sizing xxx until we call it with a invalid QRectF. 
 
> WebKitTools/QtTestBrowser/webview.cpp:93
>  +          scene()->setSceneRect(QRectF());
> why this?

See from the docs:

"The scene rectangle defines the extent of the scene. It is primarily used by QGraphicsView to determine the view's default scrollable area, and by QGraphicsScene to manage item indexing.
If unset, or if set to a null QRectF, sceneRect() will return the largest bounding rect of all items on the scene since the scene was created (i.e., a rectangle that grows when items are added to or moved in the scene, but never shrinks)."

[Antonio Gomes]

Created attachment 63256 [details]
patch v2

Same patch as attachment 63247 [details], but with more explainations in the code, as per discussion with Kenneth on IRC.

[Antonio Gomes]

Created attachment 63304 [details]
(committed r64556, r=kenneth) patch v2.1 - right patch

[Right patch this time].

[Antonio Gomes]

Comment on attachment 63304 [details]
(committed r64556, r=kenneth) patch v2.1 - right patch

Clearing flags on attachment: 63304

Committed r64556: <http://trac.webkit.org/changeset/64556>