Bug 31358 – [Qt] Various doc fixes

• Status: CLOSED
• Resolution: FIXED
• Component: Tools / Tests

[Jocelyn Turcotte]

3 patches that were lying in qt trunk.

[Jocelyn Turcotte]

Created attachment 42969 [details]
(landed in r50815) DocPatch 1

[Jocelyn Turcotte]

Created attachment 42970 [details]
(landed in r50858) DocPatch 2

[Jocelyn Turcotte]

Created attachment 42971 [details]
(landed in r50817) DocPatch 3

[Kenneth Rohde Christiansen]

Comment on attachment 42970 [details]
(landed in r50858) DocPatch 2


 167 If the width and height of the item is not set, they will dynamically adjust to
 168 a size appropriate for the content. This width may be large (e.g., 980 pixels or
 169 more) for typical online Web pages. 

Is this really true? does it do that?

[WebKit Commit Bot]

Comment on attachment 42969 [details]
(landed in r50815) DocPatch 1

Clearing flags on attachment: 42969

Committed r50815: <http://trac.webkit.org/changeset/50815>

[WebKit Commit Bot]

Comment on attachment 42971 [details]
(landed in r50817) DocPatch 3

Clearing flags on attachment: 42971

Committed r50817: <http://trac.webkit.org/changeset/50817>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Jocelyn Turcotte]

Dahh! The commit bot is probably drunk.

[Jocelyn Turcotte]

(In reply to comment #4)
> (From update of attachment 42970 [details])
> 
>  167 If the width and height of the item is not set, they will dynamically
> adjust to
>  168 a size appropriate for the content. This width may be large (e.g., 980
> pixels or
>  169 more) for typical online Web pages. 
> 
> Is this really true? does it do that?

Humm, I don't know, by looking at blame, if you don't know about it, I guess
Antonio should...?

[Eric Seidel (no email)]

(In reply to comment #8)
> Dahh! The commit bot is probably drunk.

Nah, this is bug 28230.

[Antonio Gomes]

(In reply to comment #9)
> (In reply to comment #4)
> > (From update of attachment 42970 [details] [details])
> > 
> >  167 If the width and height of the item is not set, they will dynamically
> > adjust to
> >  168 a size appropriate for the content. This width may be large (e.g., 980
> > pixels or
> >  169 more) for typical online Web pages. 
> > 
> > Is this really true? does it do that?
> 
> Humm, I don't know, by looking at blame, if you don't know about it, I guess
> Antonio should...?

I guess it is not true. I could had been a copy&paste error the original
implementation in the kinectic implementation. see
http://qt.gitorious.org/+qt-kinetic-developers/qt/kinetic/blobs/kinetic-declarativeui-gv/src/declarative/fx/qfxwebview.cpp#line172

before simon's sizeHint commit, QGraphicsWebView instances were getting an
invalid size if not explicitly specified. now it gets 800x600.

QSizeF QGraphicsWebView::sizeHint(Qt::SizeHint which, const QSizeF& constraint)
const
{
    if (which == Qt::PreferredSize)
        return QSizeF(800, 600); // ###
    return QGraphicsWidget::sizeHint(which, constraint);
}

[Kenneth Rohde Christiansen]

I'm afraid that that code was changed maybe before it got integrated. So we
need to test it to make sure the docs are ok. Antonio can you do that?

[Antonio Gomes]

(In reply to comment #12)
> I'm afraid that that code was changed maybe before it got integrated. So we
> need to test it to make sure the docs are ok. Antonio can you do that?

So i just tested it w/ a basic app (see paste below), whereas no size is specified to "webview".  When it opens it sizes 800x600, that is the preferred size specified by QGraphicsWebView::sizeHint method. If I comment out this method, QGraphicsWebView instance will have an invalid size. So it is independent on the containing view.

So

"If the width and height of the item is not set, they will dynamically
adjust to a size appropriate for the content. This width may be large (e.g., 980 pixels or more) for typical online Web pages."

is wrong imo.





#include <QApplication>
#include "qgraphicswebview.h"
#include <QGraphicsView>

int main(int argc, char *argv[])
{
    QApplication::setGraphicsSystem(QLatin1String("raster"));
    QApplication app(argc, argv);
    QGraphicsScene scene;
    QGraphicsView view(&scene);

    view.setOptimizationFlags(QGraphicsView::DontSavePainterState
                            | QGraphicsView::DontAdjustForAntialiasing);

    view.setViewportUpdateMode(QGraphicsView::MinimalViewportUpdate);
    view.setFrameShape(QFrame::NoFrame);

    QGraphicsWebView *webView = new QGraphicsWebView;
    webView->setFocus();

    scene.addItem(webView);
    scene.setActiveWindow(webView);

    view.resize(600, 480);
    view.show();

    webView->load(QUrl("http://google.com"));

    return app.exec();
}

[Kenneth Rohde Christiansen]

Antonio, do you feel like updating the patch?

[Antonio Gomes]

Created attachment 43003 [details]
more doc fixes.

[Antonio Gomes]

Created attachment 43005 [details]
(landed in 50860) more doc fixes v0.2

(In reply to comment #14)
> Antonio, do you feel like updating the patch?
yes.

patch address behavior described in comment #13

[Kenneth Rohde Christiansen]

OK, it would have been better if you had fixed DocPatch 2, which was r-'ed.

I will r+ it just to get it in and then add yours, but please think of this in the future!

[Kenneth Rohde Christiansen]

Comment on attachment 42970 [details]
(landed in r50858) DocPatch 2

r+'ing as there is a follow up patch.

[Kenneth Rohde Christiansen]

Ah, Antonio, you didn't even do it on top of the other patch. Please rebase it after DocPatch 2 has been committed. rs=me.

[Antonio Gomes]

ok, that is easy to fix.

[Antonio Gomes]

Comment on attachment 42970 [details]
(landed in r50858) DocPatch 2

i will land it manually

[Antonio Gomes]

Created attachment 43026 [details]
(landed in r50879) more doc fixes v0.3

patch fixes wrong "QWebView" citation in QWebPage constructor

"Constructs an empty QWebView with parent"

[Jan Alonzo]

Comment on attachment 43026 [details]
(landed in r50879) more doc fixes v0.3

Looks fine.

[Jocelyn Turcotte]

Seems like we are done with these patches