Bug 34885 - [Qt] QGraphicsWebView should have a mode that makes it automatically resize itself to the content size
Summary: [Qt] QGraphicsWebView should have a mode that makes it automatically resize i...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P3 Normal
Assignee: Nobody
URL:
Keywords: Qt
Depends on:
Blocks: 31552
  Show dependency treegraph
 
Reported: 2010-02-12 04:04 PST by Antti Koivisto
Modified: 2010-02-16 18:19 PST (History)
3 users (show)

See Also:


Attachments
add QGraphicsWebView resizesToContent mode (8.16 KB, patch)
2010-02-12 04:12 PST, Antti Koivisto
kenneth: review+
kenneth: commit-queue-
Details | Formatted Diff | Diff
updated patch (8.59 KB, patch)
2010-02-12 06:46 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
Fix build with Qt 4.5 (1.45 KB, patch)
2010-02-16 13:32 PST, Ariya Hidayat
laszlo.gombos: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2010-02-12 04:04:00 PST
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.
Comment 1 Antti Koivisto 2010-02-12 04:12:06 PST
Created attachment 48634 [details]
add QGraphicsWebView resizesToContent mode
Comment 2 Kenneth Rohde Christiansen 2010-02-12 04:21:46 PST
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.
Comment 3 Simon Hausmann 2010-02-12 04:23:48 PST
 +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?
Comment 4 Antti Koivisto 2010-02-12 06:46:03 PST
Created attachment 48642 [details]
updated patch

- rename resizesToContent to resizesToContents
- comment cleanup
- disconnect the signal when needed
Comment 5 Antti Koivisto 2010-02-12 06:49:51 PST
> > +        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.
Comment 6 Antti Koivisto 2010-02-12 06:51:05 PST
(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 7 Simon Hausmann 2010-02-12 06:56:44 PST
Comment on attachment 48642 [details]
updated patch

LGTM
Comment 8 Antti Koivisto 2010-02-15 01:45:53 PST
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.
Comment 9 Antti Koivisto 2010-02-15 01:48:05 PST
> ... and you should add the bug to the 4.7 API tracker bug 31552.

done
Comment 10 Ariya Hidayat 2010-02-16 13:32:37 PST
Created attachment 48822 [details]
Fix build with Qt 4.5
Comment 11 Laszlo Gombos 2010-02-16 15:30:52 PST
Changed the status to REOPENED to make sure commit-bot will be happy to commit attachment 48822 [details].
Comment 12 Ariya Hidayat 2010-02-16 18:19:49 PST
Manually committed in r54854: http://trac.webkit.org/changeset/54854