26352 – Remove void QWebFrame::renderContents(...)

Andre Pedralho

Reported 2009-06-12 12:44:56 PDT

Remove method void QWebFrame::renderContents(...) as decided during the API review.

Andre Pedralho

Comment 1 2009-06-12 12:48:45 PDT

Created attachment 31208 [details] Removing QWebFrame::renderContents and adding Q_PROPERTY(bool clipRenderToViewport).

Adam Treat

Comment 2 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.

Antonio Gomes

Comment 3 2009-06-15 08:20:38 PDT

pedralho, as per ariya's comment on IRC, please add tests , probably in qwebframe.

Eric Seidel (no email)

Comment 4 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?

Andre Pedralho

Comment 5 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.

Adam Treat

Comment 6 2009-07-14 18:17:46 PDT

Are you still working on this?

Kenneth Rohde Christiansen

Comment 7 2009-07-23 12:00:29 PDT

Pedralho just confirmed to me that he is indeed still working on this bug

Andre Pedralho

Comment 8 2009-07-24 05:43:41 PDT

Created attachment 33431 [details] Added tests and qdoc headers. Patch updated according to the suggestions.

Adam Treat

Comment 9 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?

Adam Treat

Comment 10 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?

Andre Pedralho

Comment 11 2009-07-24 07:02:53 PDT

Created attachment 33437 [details] Fixed variable name and update ToT.

Antonio Gomes

Comment 12 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

Andre Pedralho

Comment 13 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?

Adam Treat

Comment 14 2009-07-24 07:17:45 PDT

Comment on attachment 33437 [details] Fixed variable name and update ToT. Looks good.

Adam Barth

Comment 15 2009-07-25 14:34:03 PDT

This appears to have already landed.

Attachments
Removing QWebFrame::renderContents and adding Q_PROPERTY(bool clipRenderToViewport). (5.25 KB, patch) 2009-06-12 12:48 PDT, Andre Pedralho	apedralho: review-	Details Formatted Diff Diff
Added tests and qdoc headers. (7.85 KB, patch) 2009-07-24 05:43 PDT, Andre Pedralho	no flags	Details Formatted Diff Diff
Fixed variable name and update ToT. (7.85 KB, patch) 2009-07-24 07:02 PDT, Andre Pedralho	manyoso: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.