Summary: | [Qt] Various doc fixes | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jocelyn Turcotte <jturcotte> | ||||||||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | CLOSED FIXED | ||||||||||||||||
Severity: | Enhancement | CC: | commit-queue, eric, kenneth, tonikitoo | ||||||||||||||
Priority: | P2 | Keywords: | Qt | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | PC | ||||||||||||||||
OS: | All | ||||||||||||||||
Attachments: |
|
Description
Jocelyn Turcotte
2009-11-11 09:46:58 PST
Created attachment 42969 [details] (landed in r50815) DocPatch 1 Created attachment 42970 [details] (landed in r50858) DocPatch 2 Created attachment 42971 [details] (landed in r50817) DocPatch 3 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 on attachment 42969 [details] (landed in r50815) DocPatch 1 Clearing flags on attachment: 42969 Committed r50815: <http://trac.webkit.org/changeset/50815> Comment on attachment 42971 [details] (landed in r50817) DocPatch 3 Clearing flags on attachment: 42971 Committed r50817: <http://trac.webkit.org/changeset/50817> All reviewed patches have been landed. Closing bug. Dahh! The commit bot is probably drunk. (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...? (In reply to comment #8) > Dahh! The commit bot is probably drunk. Nah, this is bug 28230. (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); } 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? (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(); } Antonio, do you feel like updating the patch? Created attachment 43003 [details]
more doc fixes.
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 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 on attachment 42970 [details] (landed in r50858) DocPatch 2 r+'ing as there is a follow up patch. 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. ok, that is easy to fix. Comment on attachment 42970 [details] (landed in r50858) DocPatch 2 i will land it manually 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 on attachment 43026 [details] (landed in r50879) more doc fixes v0.3 Looks fine. Seems like we are done with these patches |