[Webkit-unassigned] [Bug 26584] [Qt] Printing extra information

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26584


zecke at selfish.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31614|review?                     |review-
               Flag|                            |




------- Comment #3 from zecke at selfish.org  2009-06-22 00:18 PDT -------
(From update of attachment 31614)
Please see:
   http://webkit.org/coding/coding-style.html (specially for the way you handle
if statements)





> Index: WebKit/qt/ChangeLog
> ===================================================================
> --- WebKit/qt/ChangeLog	(revision 44911)
> +++ WebKit/qt/ChangeLog	(working copy)
> @@ -1,3 +1,16 @@
> +2009-06-21  Jakob Truelsen  <antialize at gmail.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Fix: https://bugs.webkit.org/show_bug.cgi?id=26584
> +
> +        Allow one to count page numbers and to print extra information on pages.

maybe mention that you add API?


> +	
> +        * Api/qwebframe.cpp:
> +        (QWebFrame::countPages):
> +        (QWebFrame::print):
> +        * Api/qwebframe.h:
> +
>  2009-06-19  Daniel Teske <qt-info at nokia.com>
>  
>          Reviewed by Simon Hausmann.
> Index: WebKit/qt/Api/qwebframe.cpp
> ===================================================================
> --- WebKit/qt/Api/qwebframe.cpp	(revision 44911)
> +++ WebKit/qt/Api/qwebframe.cpp	(working copy)
> @@ -1033,15 +1033,44 @@ bool QWebFrame::event(QEvent *e)
>  
>  #ifndef QT_NO_PRINTER
>  /*!
> -    Prints the frame to the given \a printer.
> +    \since 4.6
> +    Count the number of pages, that will be printed by print.
> +
> +    \sa print
> +*/
> +uint QWebFrame::countPages(QPrinter *printer) {
> +    const qreal zoomFactorX = printer->logicalDpiX() / qt_defaultDpi();
> +    const qreal zoomFactorY = printer->logicalDpiY() / qt_defaultDpi();
> +
> +    PrintContext printContext(d->frame);
> +    float pageHeight = 0;
> +
> +    QRect qprinterRect = printer->pageRect();
> +
> +    IntRect pageRect(0, 0,
> +                     int(qprinterRect.width() / zoomFactorX),
> +                     int(qprinterRect.height() / zoomFactorY));
> +
> +    printContext.begin(pageRect.width());
> +    printContext.computePageRects(pageRect, 0, 0, 1.0, pageHeight);
> +    uint count = printContext.pageCount();
> +    printContext.end();
> +    return count;
> +}

If print() is const, countPages should be const as well... then please check of
uint is ever used in Qt API. I think it is not the case and you should either
go for unsigned int or such... And it seems a bit wasteful to use a
PrintContext for that...



> +/*!
> +    Prints the frame to the given \a printer. If \a painter is
> +    provided print with this painter, this allows to print multiple
> +    pages into on job.
>  
>      \sa render()
>  */
> -void QWebFrame::print(QPrinter *printer) const
> +void QWebFrame::print(QPrinter *printer, QPainter * painter) const

You can not simply remove a paramater (+ style guideline violation). You will
need to add a new overload...


>  {
> -    QPainter painter;
> -    if (!painter.begin(printer))
> -        return;
> +    QPainter * innerPainter = painter;
> +    if(!innerPainter) innerPainter = new QPainter();
> +    if(!innerPainter->isActive()) innerPainter->begin(printer);

style guide violation...


>      for (int i = 0; i < docCopies; ++i) {
>          int page = fromPage;
> @@ -1101,10 +1130,13 @@ void QWebFrame::print(QPrinter *printer)
>                      return;
>                  }
>                  printContext.spoolPage(ctx, page - 1, pageRect.width());
> -                if (j < pageCopies - 1)
> +                if (j < pageCopies - 1) {
> +                    emit printingNewPage(printer,fromPage,toPage,page);
>                      printer->newPage();
> +                }
>              }
> -
> +            emit printingNewPage(printer,fromPage,toPage,page);
> +        
>              if (page == toPage)
>                  break;
>  
> @@ -1121,6 +1153,8 @@ void QWebFrame::print(QPrinter *printer)
>      }
>  
>      printContext.end();
> +    innerPainter->restore();
> +    if(innerPainter != painter) delete innerPainter;
>  }
>  #endif // QT_NO_PRINTER
>  
> @@ -1229,6 +1263,18 @@ QWebFrame* QWebFramePrivate::kit(WebCore
>  */
>  
>  /*!
> +  \fn void QWebFrame::printingNewPage(QPrinter *printer, int fromPage, int toPage, int Page) const
> +  \since 4.6
> +
> +  This signal is emitted when when a page is printed. This allows one
> +  to add extra contest 

contest? did you meant conent?


>  public Q_SLOTS:
>      QVariant evaluateJavaScript(const QString& scriptSource);
>  #ifndef QT_NO_PRINTER
> -    void print(QPrinter *printer) const;
> +    void print(QPrinter *printer, QPainter * painter=NULL) const;

Don't use NULL in C++, and due ABI compability requirements you can not do
that...


and in general I'm not sure if this is the best way of achieving this....


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.