From 46cb3a09f4eedb2d4c612a6d73e9e0f14237350c Mon Sep 17 00:00:00 2001
From: Markku Luukkainen <markku.luukkainen@digia.com>
Date: Mon, 8 Jun 2009 17:08:26 +0200
Subject: Fixes from code review

---
 src/gui/kernel/qaction.cpp           |  3 ++-
 src/gui/kernel/qaction.h             |  1 -
 src/gui/kernel/qapplication_s60.cpp  |  6 ++---
 src/gui/kernel/qwidget.cpp           | 39 ++++++++++++++++++++++++--------
 src/gui/kernel/qwidget.h             |  2 +-
 src/gui/kernel/qwidget_p.h           |  6 ++---
 src/gui/kernel/qwidget_s60.cpp       |  7 +++---
 src/gui/widgets/qkeyeventsoftkey.cpp | 44 ++++++++++++++++++------------------
 src/gui/widgets/qkeyeventsoftkey.h   |  4 ++--
 src/gui/widgets/qmainwindow.cpp      |  2 +-
 src/gui/widgets/qmenu_symbian.cpp    | 11 ++++-----
 src/gui/widgets/qmenubar_p.h         |  2 +-
 12 files changed, 70 insertions(+), 57 deletions(-)

diff --git a/src/gui/kernel/qaction.cpp b/src/gui/kernel/qaction.cpp
index e32416a..8263cbc 100644
--- a/src/gui/kernel/qaction.cpp
+++ b/src/gui/kernel/qaction.cpp
@@ -1353,7 +1353,7 @@ QAction::MenuRole QAction::menuRole() const
     \since 4.6
 
     This indicates what softkey action this action is. Usually used on mobile
-    platforms to map QActions no hardware keys.
+    platforms to map QActions to hardware keys.
     
     The softkey role can be changed any time.
 */
@@ -1372,6 +1372,7 @@ QAction::SoftKeyRole QAction::softKeyRole() const
     Q_D(const QAction);
     return d->softKeyRole;
 }
+
 /*!
     \property QAction::iconVisibleInMenu
     \brief Whether or not an action should show an icon in a menu
diff --git a/src/gui/kernel/qaction.h b/src/gui/kernel/qaction.h
index 6725f42..8791a5c 100644
--- a/src/gui/kernel/qaction.h
+++ b/src/gui/kernel/qaction.h
@@ -96,7 +96,6 @@ public:
                     EndEditSoftKey, RevertEditSoftKey, DeselectSoftKey, FinishSoftKey, 
                     MenuSoftKey, ContextMenuSoftKey, Key1SoftKey, Key2SoftKey, 
                     Key3SoftKey, Key4SoftKey, CustomSoftKey };
-    
     explicit QAction(QObject* parent);
     QAction(const QString &text, QObject* parent);
     QAction(const QIcon &icon, const QString &text, QObject* parent);
diff --git a/src/gui/kernel/qapplication_s60.cpp b/src/gui/kernel/qapplication_s60.cpp
index 6ee7899..0ae4c00 100644
--- a/src/gui/kernel/qapplication_s60.cpp
+++ b/src/gui/kernel/qapplication_s60.cpp
@@ -1,5 +1,5 @@
 /****************************************************************************
-1**
+**
 ** Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies).
 ** Contact: Qt Software Information (qt-info@nokia.com)
 **
@@ -1067,9 +1067,7 @@ void QApplication::symbianHandleCommand(int command)
             int index= command-SOFTKEYSTART;
             QWidget* focused = QApplication::focusWidget();
             const QList<QAction*>& softKeys = focused->softKeys();
-            if (index>=softKeys.count()) {
-            // Assert horrible state error
-            }
+            Q_ASSERT(index<softKeys.count());
             softKeys.at(index)->activate(QAction::Trigger);
         }
         else
diff --git a/src/gui/kernel/qwidget.cpp b/src/gui/kernel/qwidget.cpp
index 9d4f176..3a22ee9 100644
--- a/src/gui/kernel/qwidget.cpp
+++ b/src/gui/kernel/qwidget.cpp
@@ -4881,7 +4881,7 @@ void QWidget::render(QPainter *painter, const QPoint &targetOffset,
 }
 
 #if !defined(Q_WS_S60)
-void QWidgetPrivate::setNativeSoftKeys(const QList<QAction*> &softkeys)
+void QWidgetPrivate::setSoftKeys_sys(const QList<QAction*> &softkeys)
 {
     Q_UNUSED(softkeys)
 }
@@ -5723,7 +5723,7 @@ bool QWidget::hasFocus() const
 void QWidget::setFocus(Qt::FocusReason reason)
 {
     Q_D(QWidget);
-    d->setNativeSoftKeys(softKeys());
+    d->setSoftKeys_sys(softKeys());
 
     if (!isEnabled())
         return;
@@ -11520,19 +11520,33 @@ void QWidget::clearMask()
     setMask(QRegion());
 }
 
+/*!
+    Returns the (possibly empty) list of this widget's softkeys.
+    Returned list cannot be changed. Softkeys should be added
+    and removed via method called setSoftKeys
+
+    \sa setSoftKey(), setSoftKeys()
+*/
 const QList<QAction*>& QWidget::softKeys() const
 {
     Q_D(const QWidget);
     if( d->softKeys.count() > 0)
         return d->softKeys;
-    
     if (isWindow() || !parentWidget())
         return d->softKeys;
-      
+
     return parentWidget()->softKeys();
 }
 
-void QWidget::setSoftKeys(QAction *softKey)
+/*!
+    Sets the softkey \a softkey to this widget's list of softkeys,
+    Setting 0 as softkey will clear all the existing softkeys set
+    to the widget
+    A QWidget can have 0 or more softkeys
+
+    \sa softKeys(), setSoftKeys()
+*/
+void QWidget::setSoftKey(QAction *softKey)
 {
     Q_D(QWidget);
     qDeleteAll(d->softKeys);
@@ -11540,19 +11554,24 @@ void QWidget::setSoftKeys(QAction *softKey)
     if (softKey)
         d->softKeys.append(softKey);
     if (QApplication::focusWidget() == this)
-        d->setNativeSoftKeys(this->softKeys());
+        d->setSoftKeys_sys(this->softKeys());
 }
 
+/*!
+    Sets the list of softkeys \a softkeys to this widget's list of softkeys,
+    A QWidget can have 0 or more softkeys
+
+    \sa softKeys(), setSoftKey()
+*/
 void QWidget::setSoftKeys(const QList<QAction*> &softKeys)
 {
     Q_D(QWidget);
     qDeleteAll(d->softKeys);
     d->softKeys.clear();
-    for(int i = 0; i < softKeys.count(); i++)
-        d->softKeys.append(softKeys.at(i));
-    
+        d->softKeys = softKeys;
+
     if (QApplication::focusWidget() == this)
-        d->setNativeSoftKeys(this->softKeys());
+        d->setSoftKeys_sys(this->softKeys());
 }
 
 /*! \fn const QX11Info &QWidget::x11Info() const
diff --git a/src/gui/kernel/qwidget.h b/src/gui/kernel/qwidget.h
index 36a30ac..3f9caee 100644
--- a/src/gui/kernel/qwidget.h
+++ b/src/gui/kernel/qwidget.h
@@ -555,7 +555,7 @@ public:
     QList<QAction*> actions() const;
 #endif
     const QList<QAction*>& softKeys() const;
-    void setSoftKeys(QAction *softKey);
+    void setSoftKey(QAction *softKey);
     void setSoftKeys(const QList<QAction*> &softKeys);
 
     QWidget *parentWidget() const;
diff --git a/src/gui/kernel/qwidget_p.h b/src/gui/kernel/qwidget_p.h
index 2119849..084439c 100644
--- a/src/gui/kernel/qwidget_p.h
+++ b/src/gui/kernel/qwidget_p.h
@@ -85,8 +85,8 @@ class CCoeControl;
 class CAknTitlePane;
 class CAknContextPane;
 // The following 2 defines may only be needed for s60. To be seen.
-#define SOFTKEYSTART 5000
-#define SOFTKEYEND (5000 + Qt::Key_Context4-Qt::Key_Context1)
+const int SOFTKEYSTART=5000;
+const int SOFTKEYEND=5004;
 #endif
 
 QT_BEGIN_NAMESPACE
@@ -227,7 +227,7 @@ public:
     explicit QWidgetPrivate(int version = QObjectPrivateVersion);
     ~QWidgetPrivate();
 
-    void setNativeSoftKeys(const QList<QAction*> &softkeys);
+    void setSoftKeys_sys(const QList<QAction*> &softkeys);
     QWExtra *extraData() const;
     QTLWExtra *topData() const;
     QTLWExtra *maybeTopData() const;
diff --git a/src/gui/kernel/qwidget_s60.cpp b/src/gui/kernel/qwidget_s60.cpp
index 858320c..2a83fc7 100644
--- a/src/gui/kernel/qwidget_s60.cpp
+++ b/src/gui/kernel/qwidget_s60.cpp
@@ -86,9 +86,8 @@ static void mapSoftKeys(const QList <QAction*> &softkeys)
 */
 }
 
-static bool isSame(const QList<QAction*>& a, const QList<QAction*>& b)
+static bool isEqual(const QList<QAction*>& a, const QList<QAction*>& b)
 {
-    bool isSame=true;
     if ( a.count() != b.count())
         return false;
     int index=0;
@@ -103,12 +102,12 @@ static bool isSame(const QList<QAction*>& a, const QList<QAction*>& b)
 }
 
 
-void QWidgetPrivate::setNativeSoftKeys(const QList<QAction*> &softkeys)
+void QWidgetPrivate::setSoftKeys_sys(const QList<QAction*> &softkeys)
 {
     Q_Q(QWidget);
     if (QApplication::focusWidget() && q!=QApplication::focusWidget()) {
         QList<QAction *> old = QApplication::focusWidget()->softKeys();
-        if (isSame(old, softkeys ))
+        if (isEqual(old, softkeys ))
             return;
     }
     
diff --git a/src/gui/widgets/qkeyeventsoftkey.cpp b/src/gui/widgets/qkeyeventsoftkey.cpp
index bb236cc..f5f10fc 100644
--- a/src/gui/widgets/qkeyeventsoftkey.cpp
+++ b/src/gui/widgets/qkeyeventsoftkey.cpp
@@ -56,26 +56,26 @@ QKeyEventSoftKey::QKeyEventSoftKey(QAction *softKeyAction, Qt::Key key, QObject
 QString QKeyEventSoftKey::roleText(QAction::SoftKeyRole role)
 {
     switch (role) {
-        case QAction::OptionsSoftKey:
-            return QAction::tr("Options");
-        case QAction::SelectSoftKey:
-            return QAction::tr("Select");
-        case QAction::BackSoftKey:
-            return QAction::tr("Back");
-        case QAction::NextSoftKey:
-            return QAction::tr("Next");
-        case QAction::PreviousSoftKey:
-            return QAction::tr("Previous");
-        case QAction::OkSoftKey:
-            return QAction::tr("Ok");
-        case QAction::CancelSoftKey:
-            return QAction::tr("Cancel");
-        case QAction::EditSoftKey:
-            return QAction::tr("Edit");
-        case QAction::ViewSoftKey:
-            return QAction::tr("View");
-        default:
-            return QString();
+    case QAction::OptionsSoftKey:
+        return QAction::tr("Options");
+    case QAction::SelectSoftKey:
+        return QAction::tr("Select");
+    case QAction::BackSoftKey:
+        return QAction::tr("Back");
+    case QAction::NextSoftKey:
+        return QAction::tr("Next");
+    case QAction::PreviousSoftKey:
+        return QAction::tr("Previous");
+    case QAction::OkSoftKey:
+        return QAction::tr("Ok");
+    case QAction::CancelSoftKey:
+        return QAction::tr("Cancel");
+    case QAction::EditSoftKey:
+        return QAction::tr("Edit");
+    case QAction::ViewSoftKey:
+        return QAction::tr("View");
+    default:
+        return QString();
     };
 }
 void QKeyEventSoftKey::addSoftKey(QAction::SoftKeyRole standardRole, Qt::Key key, QWidget *actionWidget)
@@ -86,12 +86,12 @@ void QKeyEventSoftKey::addSoftKey(QAction::SoftKeyRole standardRole, Qt::Key key
     QKeyEventSoftKey *softKey = new QKeyEventSoftKey(action, key, actionWidget);
     connect(action, SIGNAL(triggered()), softKey, SLOT(sendKeyEvent()));
     connect(action, SIGNAL(destroyed()), softKey, SLOT(deleteLater()));
-    actionWidget->setSoftKeys(action);
+    actionWidget->setSoftKey(action);
 }
 
 void QKeyEventSoftKey::removeSoftkey(QWidget *focussedWidget)
 {
-    focussedWidget->setSoftKeys(0);
+    focussedWidget->setSoftKey(0);
 }
 
 void QKeyEventSoftKey::sendKeyEvent()
diff --git a/src/gui/widgets/qkeyeventsoftkey.h b/src/gui/widgets/qkeyeventsoftkey.h
index 83449c7..0b95efb 100644
--- a/src/gui/widgets/qkeyeventsoftkey.h
+++ b/src/gui/widgets/qkeyeventsoftkey.h
@@ -39,8 +39,8 @@
 **
 ****************************************************************************/
 
-#ifndef QKEYEVENSOFTKEY_H
-#define QKEYEVENSOFTKEY_H
+#ifndef QKEYEVENTSOFTKEY_H
+#define QKEYEVENTSOFTKEY_H
 
 #include <QtCore/qobject.h>
 #include "QtGui/qaction.h"
diff --git a/src/gui/widgets/qmainwindow.cpp b/src/gui/widgets/qmainwindow.cpp
index 90c5db9..2f3b412 100644
--- a/src/gui/widgets/qmainwindow.cpp
+++ b/src/gui/widgets/qmainwindow.cpp
@@ -484,7 +484,7 @@ void QMainWindow::setMenuBar(QMenuBar *menuBar)
     if (menuBar) {
         QAction* menu = new QAction(QString::fromLatin1("Menu"), this);
         menu->setSoftKeyRole(QAction::MenuSoftKey);
-        setSoftKeys(menu);
+        setSoftKey(menu);
     }
 }
 
diff --git a/src/gui/widgets/qmenu_symbian.cpp b/src/gui/widgets/qmenu_symbian.cpp
index ed2ea46..49bcc21 100644
--- a/src/gui/widgets/qmenu_symbian.cpp
+++ b/src/gui/widgets/qmenu_symbian.cpp
@@ -40,8 +40,6 @@
 
 #include "qmenu.h"
 #include "qapplication.h"
-#include "qmainwindow.h"
-#include "qtoolbar.h"
 #include "qevent.h"
 #include "qstyle.h"
 #include "qdebug.h"
@@ -221,13 +219,12 @@ static void rebuildMenu()
     widgetWithContextMenu = 0;
     QMenuBarPrivate *mb = 0;
     QWidget *w = qApp->activeWindow();
-    QMainWindow *mainWindow = qobject_cast<QMainWindow*>(w);
     QWidget* focusWidget = QApplication::focusWidget();
     if (focusWidget) {
         if (hasContextMenu(focusWidget))
             widgetWithContextMenu = focusWidget;
     }
-    
+
     if (w) {
         mb = menubars()->value(w);
         qt_symbian_menu_static_cmd_id = QT_FIRST_MENU_ITEM;
@@ -389,7 +386,7 @@ void QMenuBarPrivate::QSymbianMenuBarPrivate::removeAction(QSymbianMenuAction *a
     rebuild();
 }
 
-void QMenuBarPrivate::QSymbianMenuBarPrivate::InsertNativeMenuItems(const QList<QAction*> &actions)
+void QMenuBarPrivate::QSymbianMenuBarPrivate::insertNativeMenuItems(const QList<QAction*> &actions)
 {
     for (int i = 0; i <actions.size(); ++i) {
         QSymbianMenuAction *symbianActionTopLevel = new QSymbianMenuAction;
@@ -408,14 +405,14 @@ void QMenuBarPrivate::QSymbianMenuBarPrivate::rebuild()
     qt_symbian_menu_static_cmd_id = QT_FIRST_MENU_ITEM;
     deleteAll( &symbianMenus );
     if (d)
-        InsertNativeMenuItems(d->actions);
+        insertNativeMenuItems(d->actions);
 
     contextMenuActionList.clear();
     if (widgetWithContextMenu) {
         contexMenuCommand = qt_symbian_menu_static_cmd_id;
         contextAction.setText(QString("Actions"));
         contextMenuActionList.append(&contextAction);
-        InsertNativeMenuItems(contextMenuActionList);
+        insertNativeMenuItems(contextMenuActionList);
     }
         
 }
diff --git a/src/gui/widgets/qmenubar_p.h b/src/gui/widgets/qmenubar_p.h
index 429605a..9e7d511 100644
--- a/src/gui/widgets/qmenubar_p.h
+++ b/src/gui/widgets/qmenubar_p.h
@@ -257,7 +257,7 @@ public:
             }
             return 0;
         }
-        void InsertNativeMenuItems(const QList<QAction*> &actions);
+        void insertNativeMenuItems(const QList<QAction*> &actions);
 
     } *symbian_menubar;
     static void symbianCommands(int command);
-- 
cgit v0.12