Bug 34885 – [Qt] QGraphicsWebView should have a mode that makes it automatically resize itself to the content size

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

[Antti Koivisto]

This is useful for cases where the client wants to implement page panning and zooming by manipulating the graphics item.

You can do this already in the client by listening to QWebPage::contentsSizeChanged signal but WebKit internal implementation has more optimization possibilities in the future.

[Antti Koivisto]

Created attachment 48634 [details]
add QGraphicsWebView resizesToContent mode

[Kenneth Rohde Christiansen]

Comment on attachment 48634 [details]
add QGraphicsWebView resizesToContent mode

Please fix the comment before committing

> +/*!
> +    \property QGraphicsWebView::resizesToContent
> +    \brief whether the QGraphicsView size changes to match the content size

whether the size of the QGraphics_Web_View and it's viewport changes to match the contents size. 

> +
> +    If this property is set, the QGraphicsWebView will automatically change its
> +    size to match the size of the main frame content. As a result the top level frame
> +    will never have scrollbars.
> +
> +    This property should be used in conjunction with the QWebPage::preferredContentsSize
> +    property.

If not the preferredContentsSize is automatically set to a resonable size (?)

> +
> +    \sa QWebPage::setPreferredContentsSize
> +*/

Needs a \since 4.7 and you should add the bug to the 4.7 API tracker bug 31552.

It might be a good idea to tell that the preferredContentsSize is set to a default value
if not set on before hand.

> +void QGraphicsWebView::setResizesToContent(bool enabled)

Better use contents with s! as it is called preferredContent_s_Size etc. This should be fixed everywhere.

> +{
> +    d->resizesToContent = enabled;
> +    if (enabled && d->page)
> +        d->enableResizesToContentForPage();
> +}
> +
> +bool QGraphicsWebView::resizesToContent() const
> +{
> +    return d->resizesToContent;
> +}
> +
>  /*! \reimp
>  */
>  void QGraphicsWebView::hoverMoveEvent(QGraphicsSceneHoverEvent* ev)
> Index: WebKit/qt/Api/qgraphicswebview.h
> ===================================================================
> --- WebKit/qt/Api/qgraphicswebview.h	(revision 54651)
> +++ WebKit/qt/Api/qgraphicswebview.h	(working copy)
> @@ -45,6 +45,7 @@ class QWEBKIT_EXPORT QGraphicsWebView : 
>      Q_PROPERTY(QUrl url READ url WRITE setUrl NOTIFY urlChanged)
>  
>      Q_PROPERTY(bool modified READ isModified)
> +    Q_PROPERTY(bool resizesToContent READ resizesToContent WRITE setResizesToContent)
>  
>  public:
>      explicit QGraphicsWebView(QGraphicsItem* parent = 0);
> @@ -79,6 +80,9 @@ public:
>  
>      bool findText(const QString& subString, QWebPage::FindFlags options = 0);
>  
> +    bool resizesToContent() const;
> +    void setResizesToContent(bool enabled);
> +
>      virtual void setGeometry(const QRectF& rect);
>      virtual void updateGeometry();
>      virtual void paint(QPainter*, const QStyleOptionGraphicsItem* options, QWidget* widget = 0);
> @@ -137,6 +141,7 @@ private:
>      // we don't want to change the moc based on USE() macro, so this function is here
>      // but will be empty if ACCLERATED_COMPOSITING is disabled
>      Q_PRIVATE_SLOT(d, void syncLayers())
> +    Q_PRIVATE_SLOT(d, void _q_contentsSizeChanged(QSize))
>  
>      QGraphicsWebViewPrivate* const d;
>      friend class QGraphicsWebViewPrivate;
> Index: WebKit/qt/QGVLauncher/main.cpp
> ===================================================================
> --- WebKit/qt/QGVLauncher/main.cpp	(revision 54651)
> +++ WebKit/qt/QGVLauncher/main.cpp	(working copy)
> @@ -74,6 +74,8 @@ public:
>      {
>          if (QApplication::instance()->arguments().contains("--cacheWebView"))
>              setCacheMode(QGraphicsItem::DeviceCoordinateCache);
> +        if (QApplication::instance()->arguments().contains("--resizesToContent"))
> +            setResizesToContent(true);

Would be cool to have this support in the QtLauncher now it supports the -graphicsbased, but can be done in another commit.


> +        if (!QApplication::instance()->arguments().contains("--resizesToContent")) {
> +            setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
> +            setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
> +        }

Maybe we should turn these off automatically when enabling the mode? Fits with the documentation even.

[Simon Hausmann]

 +void QGraphicsWebViewPrivate::_q_contentsSizeChanged(QSize size)

Preference: I'd use a const QSize& here


> --- WebKit/qt/QGVLauncher/main.cpp	(revision 54651)
> +++ WebKit/qt/QGVLauncher/main.cpp	(working copy)
> @@ -74,6 +74,8 @@ public:
>      {
>          if (QApplication::instance()->arguments().contains("--cacheWebView"))
>              setCacheMode(QGraphicsItem::DeviceCoordinateCache);
> +        if (QApplication::instance()->arguments().contains("--resizesToContent"))
> +            setResizesToContent(true);

I think it would be nice to tie this feature to an action in the GUI instead of
the commandline. The commandline is "hard" to use on Symbian. I find it easier
to tap through the menu. For example it could be a simple checkable action in
the "Developer" menu.


> +void QGraphicsWebView::setResizesToContent(bool enabled)
> +{
> +    d->resizesToContent = enabled;
> +    if (enabled && d->page)
> +        d->enableResizesToContentForPage();
> +}

When the feature is disabled, shouldn't we disconnect from the
contentsSizeChanged signal?

[Antti Koivisto]

Created attachment 48642 [details]
updated patch

- rename resizesToContent to resizesToContents
- comment cleanup
- disconnect the signal when needed

[Antti Koivisto]

> > +        if (!QApplication::instance()->arguments().contains("--resizesToContent")) {
> > +            setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
> > +            setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
> > +        }
> 
> Maybe we should turn these off automatically when enabling the mode? Fits with
> the documentation even.

Not sure what you mean here. QGraphicsWebView does not really know about the QGraphicsView it is in and should definitely not change any settings there.

[Antti Koivisto]

(In reply to comment #3)
>  +void QGraphicsWebViewPrivate::_q_contentsSizeChanged(QSize size)
> 
> Preference: I'd use a const QSize& here

fixed

> I think it would be nice to tie this feature to an action in the GUI instead of
> the commandline. The commandline is "hard" to use on Symbian. I find it easier
> to tap through the menu. For example it could be a simple checkable action in
> the "Developer" menu.

yeah, but better handled separately

> When the feature is disabled, shouldn't we disconnect from the
> contentsSizeChanged signal?

fixed (though this is probably usually one time thing)

[Simon Hausmann]

Comment on attachment 48642 [details]
updated patch

LGTM

[Antti Koivisto]

Sending        WebKit/qt/Api/qgraphicswebview.cpp
Sending        WebKit/qt/Api/qgraphicswebview.h
Sending        WebKit/qt/ChangeLog
Sending        WebKit/qt/QGVLauncher/main.cpp
Transmitting file data ....
Committed revision 54767.

[Antti Koivisto]

> ... and you should add the bug to the 4.7 API tracker bug 31552.

done

[Ariya Hidayat]

Created attachment 48822 [details]
Fix build with Qt 4.5

[Laszlo Gombos]

Changed the status to REOPENED to make sure commit-bot will be happy to commit attachment 48822 [details].

[Ariya Hidayat]

Manually committed in r54854: http://trac.webkit.org/changeset/54854