Bug 34885

Created attachment 48634 [details] add QGraphicsWebView resizesToContent mode

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.

+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?

Created attachment 48642 [details] updated patch - rename resizesToContent to resizesToContents - comment cleanup - disconnect the signal when needed

> > + 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.

(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)

Comment on attachment 48642 [details] updated patch LGTM

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.

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

Created attachment 48822 [details] Fix build with Qt 4.5

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

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