Bug 31358 - [Qt] Various doc fixes
Summary: [Qt] Various doc fixes
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Enhancement
Assignee: Nobody
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2009-11-11 09:46 PST by Jocelyn Turcotte
Modified: 2009-11-19 09:49 PST (History)
4 users (show)

See Also:


Attachments
(landed in r50815) DocPatch 1 (914 bytes, patch)
2009-11-11 09:47 PST, Jocelyn Turcotte
no flags Details | Formatted Diff | Diff
(landed in r50858) DocPatch 2 (4.66 KB, patch)
2009-11-11 09:47 PST, Jocelyn Turcotte
no flags Details | Formatted Diff | Diff
(landed in r50817) DocPatch 3 (1.40 KB, patch)
2009-11-11 09:48 PST, Jocelyn Turcotte
no flags Details | Formatted Diff | Diff
more doc fixes. (1.87 KB, patch)
2009-11-11 14:29 PST, Antonio Gomes
no flags Details | Formatted Diff | Diff
(landed in 50860) more doc fixes v0.2 (1.87 KB, patch)
2009-11-11 14:36 PST, Antonio Gomes
no flags Details | Formatted Diff | Diff
(landed in r50879) more doc fixes v0.3 (1.56 KB, patch)
2009-11-11 19:48 PST, Antonio Gomes
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jocelyn Turcotte 2009-11-11 09:46:58 PST
3 patches that were lying in qt trunk.
Comment 1 Jocelyn Turcotte 2009-11-11 09:47:29 PST
Created attachment 42969 [details]
(landed in r50815) DocPatch 1
Comment 2 Jocelyn Turcotte 2009-11-11 09:47:57 PST
Created attachment 42970 [details]
(landed in r50858) DocPatch 2
Comment 3 Jocelyn Turcotte 2009-11-11 09:48:25 PST
Created attachment 42971 [details]
(landed in r50817) DocPatch 3
Comment 4 Kenneth Rohde Christiansen 2009-11-11 09:51:48 PST
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?
Comment 5 WebKit Commit Bot 2009-11-11 10:14:22 PST
Comment on attachment 42969 [details]
(landed in r50815) DocPatch 1

Clearing flags on attachment: 42969

Committed r50815: <http://trac.webkit.org/changeset/50815>
Comment 6 WebKit Commit Bot 2009-11-11 10:21:45 PST
Comment on attachment 42971 [details]
(landed in r50817) DocPatch 3

Clearing flags on attachment: 42971

Committed r50817: <http://trac.webkit.org/changeset/50817>
Comment 7 WebKit Commit Bot 2009-11-11 10:21:49 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Jocelyn Turcotte 2009-11-11 10:23:57 PST
Dahh! The commit bot is probably drunk.
Comment 9 Jocelyn Turcotte 2009-11-11 10:24:22 PST
(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...?
Comment 10 Eric Seidel (no email) 2009-11-11 10:30:46 PST
(In reply to comment #8)
> Dahh! The commit bot is probably drunk.

Nah, this is bug 28230.
Comment 11 Antonio Gomes 2009-11-11 10:46:38 PST
(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);
}
Comment 12 Kenneth Rohde Christiansen 2009-11-11 10:47:28 PST
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?
Comment 13 Antonio Gomes 2009-11-11 12:17:50 PST
(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();
}
Comment 14 Kenneth Rohde Christiansen 2009-11-11 12:22:26 PST
Antonio, do you feel like updating the patch?
Comment 15 Antonio Gomes 2009-11-11 14:29:31 PST
Created attachment 43003 [details]
more doc fixes.
Comment 16 Antonio Gomes 2009-11-11 14:36:56 PST
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
Comment 17 Kenneth Rohde Christiansen 2009-11-11 18:07:58 PST
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!
Comment 18 Kenneth Rohde Christiansen 2009-11-11 18:08:48 PST
Comment on attachment 42970 [details]
(landed in r50858) DocPatch 2

r+'ing as there is a follow up patch.
Comment 19 Kenneth Rohde Christiansen 2009-11-11 18:11:54 PST
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.
Comment 20 Antonio Gomes 2009-11-11 18:47:44 PST
ok, that is easy to fix.
Comment 21 Antonio Gomes 2009-11-11 19:18:04 PST
Comment on attachment 42970 [details]
(landed in r50858) DocPatch 2

i will land it manually
Comment 22 Antonio Gomes 2009-11-11 19:48:24 PST
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"
Comment 23 Jan Alonzo 2009-11-12 02:26:55 PST
Comment on attachment 43026 [details]
(landed in r50879) more doc fixes v0.3

Looks fine.
Comment 24 Jocelyn Turcotte 2009-11-19 09:49:35 PST
Seems like we are done with these patches