[webkit-reviews] review granted: [Bug 34885] [Qt] QGraphicsWebView should have a mode that makes it automatically resize itself to the content size : [Attachment 48634] add QGraphicsWebView resizesToContent mode

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Antti Koivisto
<koivisto at iki.fi>'s request for review:
Bug 34885: [Qt] QGraphicsWebView should have a mode that makes it automatically
resize itself to the content size
https://bugs.webkit.org/show_bug.cgi?id=34885

Attachment 48634: add QGraphicsWebView resizesToContent mode
https://bugs.webkit.org/attachment.cgi?id=48634&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
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.