[webkit-reviews] review denied: [Bug 29678] Conditionally guard cursor code with !QT_NO_CURSOR : [Attachment 39991] Patch for QT_NO_CURSOR guard

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 23 06:15:30 PDT 2009


Simon Hausmann <hausmann at webkit.org> has denied Norbert Leser
<norbert.leser at nokia.com>'s request for review:
Bug 29678: Conditionally guard cursor code with !QT_NO_CURSOR
https://bugs.webkit.org/show_bug.cgi?id=29678

Attachment 39991: Patch for QT_NO_CURSOR guard
https://bugs.webkit.org/attachment.cgi?id=39991&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>

> Index: WebKit/qt/Api/qwebgraphicsitem.cpp
> ===================================================================
> --- WebKit/qt/Api/qwebgraphicsitem.cpp	(revision 48670)
> +++ WebKit/qt/Api/qwebgraphicsitem.cpp	(working copy)
> @@ -95,12 +95,16 @@ void QWebGraphicsItemPrivate::update(con
>  
>  QCursor QWebGraphicsItemPrivate::cursor() const
>  {
> +#ifndef QT_NO_CURSOR
>      return q->cursor();
> +#endif
>  }
>  
>  void QWebGraphicsItemPrivate::updateCursor(const QCursor& cursor)
>  {
> +#ifndef QT_NO_CURSOR
>      q->setCursor(cursor);
> +#endif
>  }

The patch looks good to me, expect for these two hunks. Instead of placing the
#ifdef into the function it should be placed
around the definition/declaration. In other words: If QT_NO_CURSOR is defined,
then those functions not not be declared
in the first place, similar to the guards around QWidget::setCursor()/cursor()
in qwidget.h.


More information about the webkit-reviews mailing list