[webkit-reviews] review denied: [Bug 26584] [Qt] Printing extra information : [Attachment 31643] A better patch

[bugzilla-daemon at webkit.org]

Adam Treat <treat at kde.org> has denied Jakob Truelsen <antialize at gmail.com>'s
request for review:
Bug 26584: [Qt] Printing extra information
https://bugs.webkit.org/show_bug.cgi?id=26584

Attachment 31643: A better patch
https://bugs.webkit.org/attachment.cgi?id=31643&action=review

------- Additional Comments from Adam Treat <treat at kde.org>
I'll just say at the outlet that I don't think this new API is appropriate or
necessary.  I think the answer you are seeking is to just forgo the convenience
API that QWebView/QWebFrame allows for printing and do the heavy lifting
yourself.  It sounds like you are writing a specific application that has
demands above and beyond what the convenience API allows.  I believe you can
still accomplish your goals by using QWebFrame::render directly to draw the
entire page to a QImage for instance.  And then, in your client code, you can
do the heavy lifting of breaking up this QImage and sending it to the QPrinter.


That said, there were a number of problems with the patch still:

> +	   Added an overloared QWebFrame::print allowing one to use an
> +	   existing painter object.  

Spelling mistake.

> +unsigned int QWebFrame::countPages(QPrinter *printer) const {

I guess this should return a quint32?

> +    This allows to print multiple pages into on job.

Spelling mistake.

> +void QWebFrame::print(QPrinter* printer, QPainter* painter) const
> +{
> +    if(!painter->isActive() && !painter.begin(printer))  
>	   return;

This looks wrong.  Why the check for painter->isActive() and why the && logic?

> +    This signal is emitted when when a page is printed. This allows one

Typo.

> +    void printingNewPage(QPrinter* printer, int fromPage, int toPage, int
page) const;

Coding style issues with * decoration.