Conditionally guard cursor code with !QT_NO_CURSOR. Otherwise, it is inconsistent with class declaration of QCursor.
Created attachment 39991 [details] Patch for QT_NO_CURSOR guard
Comment on attachment 39991 [details] Patch for QT_NO_CURSOR guard > 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.
(In reply to comment #2) > (From update of attachment 39991 [details]) > > > 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. That is your call, or whoever implemented the classes/functions. In the case of conditionally guarding the functions itself, you may end up with a multitude of conditional statements in the client code. Otherwise, it would be transparent (though, perhaps unexpected behavior - i.e., updateCursor etc would do nothing, unless there is another default implementation). Anyway, could you or the implementer of QWebGraphicsItemPrivate, QWebViewPrivate, and QWebPageClient take it from here? - Thanks.
I have to agree with Simon here. Even if you would return empty QCursor (which you don't), client still needs to be aware of that cursor is not enabled. I'm saying this even if we are in huge trouble with ordinal exports and conditional exporting in Symbian.
Created attachment 40513 [details] updated patch for bug 29678 Patch updated according to Simon's comments (enclosed whole functions within conditional clause. This should not affect the API since these are functions of a private class.
(In reply to comment #5) > This should not affect the API since these are functions of a private class. Right, I've been way too much in QtCore and QtGui where private classes are exported.
Comment on attachment 40513 [details] updated patch for bug 29678 r=me. Thanks!
Comment on attachment 40513 [details] updated patch for bug 29678 Landed as http://trac.webkit.org/changeset/49023