Bug 26352

Summary: Remove void QWebFrame::renderContents(...)
Product: WebKit Reporter: Andre Pedralho <apedralho>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, kenneth, manyoso, tonikitoo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Removing QWebFrame::renderContents and adding Q_PROPERTY(bool clipRenderToViewport).
apedralho: review-
Added tests and qdoc headers.
none
Fixed variable name and update ToT. manyoso: review+

Description Andre Pedralho 2009-06-12 12:44:56 PDT
Remove method void QWebFrame::renderContents(...) as decided during the API review.
Comment 1 Andre Pedralho 2009-06-12 12:48:45 PDT
Created attachment 31208 [details]
Removing QWebFrame::renderContents and adding Q_PROPERTY(bool clipRenderToViewport).
Comment 2 Adam Treat 2009-06-15 08:04:21 PDT
The qdoc should be changed to reflect the new property and make sure to explicitly say that the property modifies the behavior of the 'render' method.
Comment 3 Antonio Gomes 2009-06-15 08:20:38 PDT
pedralho, as per ariya's comment on IRC, please add tests , probably in qwebframe.
Comment 4 Eric Seidel (no email) 2009-06-15 17:23:41 PDT
I'm confused.  This patch is r+, but there are two requests in the comments above.  The submitter is not a committer, so they have no way to correct them when landing.

Shouldn't this patch be r-'d asking for a patch with the above noted corrections?
Comment 5 Andre Pedralho 2009-06-19 13:21:20 PDT
Comment on attachment 31208 [details]
Removing QWebFrame::renderContents and adding Q_PROPERTY(bool clipRenderToViewport).

Set patch r- as per previous comments.
Comment 6 Adam Treat 2009-07-14 18:17:46 PDT
Are you still working on this?
Comment 7 Kenneth Rohde Christiansen 2009-07-23 12:00:29 PDT
Pedralho just confirmed to me that he is indeed still working on this bug
Comment 8 Andre Pedralho 2009-07-24 05:43:41 PDT
Created attachment 33431 [details]
Added tests and qdoc headers.

Patch updated according to the suggestions.
Comment 9 Adam Treat 2009-07-24 06:14:07 PDT
Comment on attachment 33431 [details]
Added tests and qdoc headers.

> +void tst_QWebFrame::render()
> +{
> +    QString html("<html>" \
> +                    "<head><style>" \
> +                       "body, iframe { margin: 0px; border: none; }" \
> +                    "</style></head>" \
> +                    "<body><iframe width='100px' height='100px'/></body>" \
> +                 "</html>");
> +
> +    QWebPage page;
> +    page.mainFrame()->setHtml(html);
> +
> +    QList<QWebFrame*> frames = page.mainFrame()->childFrames();
> +    QWebFrame *frame = frames.at(0);
> +    QString innerHtml("<body style='margin: 0px;'><img src='qrc:/image.png'/></body>");
> +    frame->setHtml(innerHtml);
> +
> +    QPicture picture;
> +
> +    // render clipping to Viewport
> +    frame->setClipRenderToViewport(true);
> +    QPainter painter1(&picture);
> +    frame->render(&painter1);
> +    painter1.end();
> +
> +    QSize size = page.mainFrame()->contentsSize();
> +    page.setViewportSize(size);
> +    QCOMPARE(size.width(), image.boundingRect().width());   // 100px
> +    QCOMPARE(size.height(), image.boundingRect().height()); // 100px

Where is this 'image' variable at?  Do you mean 'picture' here?  Will this fail to compile?
Comment 10 Adam Treat 2009-07-24 07:00:39 PDT
Comment on attachment 33431 [details]
Added tests and qdoc headers.

> +void tst_QWebFrame::render()
> +{
> +    QString html("<html>" \
> +                    "<head><style>" \
> +                       "body, iframe { margin: 0px; border: none; }" \
> +                    "</style></head>" \
> +                    "<body><iframe width='100px' height='100px'/></body>" \
> +                 "</html>");
> +
> +    QWebPage page;
> +    page.mainFrame()->setHtml(html);
> +
> +    QList<QWebFrame*> frames = page.mainFrame()->childFrames();
> +    QWebFrame *frame = frames.at(0);
> +    QString innerHtml("<body style='margin: 0px;'><img src='qrc:/image.png'/></body>");
> +    frame->setHtml(innerHtml);
> +
> +    QPicture picture;
> +
> +    // render clipping to Viewport
> +    frame->setClipRenderToViewport(true);
> +    QPainter painter1(&picture);
> +    frame->render(&painter1);
> +    painter1.end();
> +
> +    QSize size = page.mainFrame()->contentsSize();
> +    page.setViewportSize(size);
> +    QCOMPARE(size.width(), image.boundingRect().width());   // 100px
> +    QCOMPARE(size.height(), image.boundingRect().height()); // 100px

Where is this 'image' variable at?  Do you mean 'picture' here?  Will this fail to compile?
Comment 11 Andre Pedralho 2009-07-24 07:02:53 PDT
Created attachment 33437 [details]
Fixed variable name and update ToT.
Comment 12 Antonio Gomes 2009-07-24 07:09:29 PDT
@pedralho, in render method header (in qwebframe.cpp), i'd add a "\sa clipRenderToViewport , setClipRenderToViewport" ... and it would go the the docs that both functions are related
Comment 13 Andre Pedralho 2009-07-24 07:13:03 PDT
(In reply to comment #12)
> @pedralho, in render method header (in qwebframe.cpp), i'd add a "\sa
> clipRenderToViewport , setClipRenderToViewport" ... and it would go the the
> docs that both functions are related

Actually, there are some docs that can be updated accordingly, including the one in QWebPage titled "Using QWebPage in a Widget-less Environment".

Should I do it, or there is a specific person to update that documents?
Comment 14 Adam Treat 2009-07-24 07:17:45 PDT
Comment on attachment 33437 [details]
Fixed variable name and update ToT.

Looks good.
Comment 15 Adam Barth 2009-07-25 14:34:03 PDT
This appears to have already landed.