Fix behavior of QButtonGroup::addButton(QAbstractButton, int).

The method did not adhere to the documented behavior nor according to
its precedessor in Qt 3 times. Now, when being passed -1 as argument,
it will automatically assign ids. They will all be negative, starting
from -2.

Reviewed-by: mae <qt-info@nokia.com>

3 files changed, 70 insertions, 12 deletions

diff --git a/src/gui/widgets/qabstractbutton.cpp b/src/gui/widgets/qabstractbutton.cpp
index f2a9ceb..1900016 100644
--- a/src/gui/widgets/qabstractbutton.cpp
+++ b/src/gui/widgets/qabstractbutton.cpp
@@ -215,11 +215,8 @@ void QButtonGroup::setExclusive(bool exclusive)
     d->exclusive = exclusive;
 }
 
-/*!
-    Adds the given \a button to the end of the group's internal list of buttons.
 
-    \sa removeButton()
-*/
+// TODO: Qt 5: Merge with addButton(QAbstractButton *button, int id)
 void QButtonGroup::addButton(QAbstractButton *button)
 {
     addButton(button, -1);
@@ -232,8 +229,18 @@ void QButtonGroup::addButton(QAbstractButton *button, int id)
         previous->removeButton(button);
     button->d_func()->group = this;
     d->buttonList.append(button);
-    if (id != -1)
+    if (id == -1) {
+        QList<int> ids = d->mapping.values();
+        if (ids.isEmpty())
+           d->mapping[button] = -2;
+        else {
+            qSort(ids);
+            d->mapping[button] = ids.first()-1;
+        }
+    } else {
         d->mapping[button] = id;
+    }
+
     if (d->exclusive && button->isChecked())
         button->d_func()->notifyChecked();
 }
diff --git a/src/gui/widgets/qbuttongroup.cpp b/src/gui/widgets/qbuttongroup.cpp
index 06bcf1e..ebfafe3 100644
--- a/src/gui/widgets/qbuttongroup.cpp
+++ b/src/gui/widgets/qbuttongroup.cpp
@@ -176,11 +176,21 @@
 */
 
 /*!
-    \fn void QButtonGroup::addButton(QAbstractButton *button, int id = -1);
+    \fn void QButtonGroup::addButton(QAbstractButton *button);
+
+    Adds the given \a button to the end of the group's internal list of buttons.
+    An \a id will be assigned to the button by this QButtonGroup. Automatically
+    assigned ids are guaranteed to be negative, starting with -2. If you are also
+    assigning your own ids, use positive values to avoid conflicts.
+
+    \sa removeButton() buttons()
+*/
+
+/*!
+    \fn void QButtonGroup::addButton(QAbstractButton *button, int id);
 
     Adds the given \a button to the button group, with the given \a
-    id. If \a id is -1 (the default), an id will be assigned to the
-    button by this QButtonGroup.
+    id. It is recommended to assign only positive ids.
 
     \sa removeButton() buttons()
 */
diff --git a/tests/auto/qbuttongroup/tst_qbuttongroup.cpp b/tests/auto/qbuttongroup/tst_qbuttongroup.cpp
index 15cca56..4bb414c 100644
--- a/tests/auto/qbuttongroup/tst_qbuttongroup.cpp
+++ b/tests/auto/qbuttongroup/tst_qbuttongroup.cpp
@@ -92,11 +92,15 @@ private slots:
     void exclusive();
     void exclusiveWithActions();
     void testSignals();
-
     void checkedButton();
 
     void task106609();
 
+    // fixed for Qt 4.6.0
+#if QT_VERSION >= 0x040600
+    void autoIncrementId();
+#endif
+    
     void task209485_removeFromGroupInEventHandler_data();
     void task209485_removeFromGroupInEventHandler();
 };
@@ -329,13 +333,19 @@ void tst_QButtonGroup::testSignals()
 
     QCOMPARE(clickedSpy.count(), 1);
     QCOMPARE(clickedIdSpy.count(), 1);
-    QVERIFY(clickedIdSpy.takeFirst().at(0).toInt() == -1);
+    
+    int expectedId = -1;    
+#if QT_VERSION >= 0x040600
+    expectedId = -2;
+#endif
+    
+    QVERIFY(clickedIdSpy.takeFirst().at(0).toInt() == expectedId);
     QCOMPARE(pressedSpy.count(), 1);
     QCOMPARE(pressedIdSpy.count(), 1);
-    QVERIFY(pressedIdSpy.takeFirst().at(0).toInt() == -1);
+    QVERIFY(pressedIdSpy.takeFirst().at(0).toInt() == expectedId);
     QCOMPARE(releasedSpy.count(), 1);
     QCOMPARE(releasedIdSpy.count(), 1);
-    QVERIFY(releasedIdSpy.takeFirst().at(0).toInt() == -1);
+    QVERIFY(releasedIdSpy.takeFirst().at(0).toInt() == expectedId);
 
     clickedSpy.clear();
     clickedIdSpy.clear();
@@ -483,5 +493,36 @@ void tst_QButtonGroup::task209485_removeFromGroupInEventHandler()
     QCOMPARE(spy1.count() + spy2.count(), signalCount);
 }
 
+#if QT_VERSION >= 0x040600
+void tst_QButtonGroup::autoIncrementId()
+{
+    QDialog dlg(0);
+    QButtonGroup *buttons = new QButtonGroup(&dlg);
+    QVBoxLayout *vbox = new QVBoxLayout(&dlg);
+
+    QRadioButton *radio1 = new QRadioButton(&dlg);
+    radio1->setText("radio1");
+    QRadioButton *radio2 = new QRadioButton(&dlg);
+    radio2->setText("radio2");
+    QRadioButton *radio3 = new QRadioButton(&dlg);
+    radio3->setText("radio3");
+
+    buttons->addButton(radio1);
+    vbox->addWidget(radio1);
+    buttons->addButton(radio2);
+    vbox->addWidget(radio2);
+    buttons->addButton(radio3);
+    vbox->addWidget(radio3);
+
+    radio1->setChecked(true);
+
+    QVERIFY(buttons->id(radio1) == -2);
+    QVERIFY(buttons->id(radio2) == -3);
+    QVERIFY(buttons->id(radio3) == -4);
+
+    dlg.show();
+}
+#endif
+
 QTEST_MAIN(tst_QButtonGroup)
 #include "tst_qbuttongroup.moc"