Fix height of combo popup when the list view has non-zero spacing.

QListView::spacing() is the space around the item (layout margin),
so the effective spacing is twice as big.
This differs conceptionally from QTableView, which has a spacing of 1
and a line on top/bottom. Split up QComboBoxPrivateContainer::spacing()
into functions return spacing and top/bottom margins to reflect this.

Task-number: QTBUG-37865
Change-Id: I1ff812e7856e00a53f1119ef3304956cbb7cbfca
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@theqtcompany.com>

src/widgets/widgets/qcombobox.cpp

@@ -479,9 +479,9 @@ void QComboBoxPrivateContainer::updateScrollers()
         view->verticalScrollBar()->minimum() < view->verticalScrollBar()->maximum()) {
 
         bool needTop = view->verticalScrollBar()->value()
-                       > (view->verticalScrollBar()->minimum() + spacing());
+                       > (view->verticalScrollBar()->minimum() + topMargin());
         bool needBottom = view->verticalScrollBar()->value()
-                          < (view->verticalScrollBar()->maximum() - spacing()*2);
+                          < (view->verticalScrollBar()->maximum() - bottomMargin() - topMargin());
         if (needTop)
             top->show();
         else
@@ -571,6 +571,20 @@ void QComboBoxPrivateContainer::setItemView(QAbstractItemView *itemView)
             this, SLOT(viewDestroyed()));
 }
 
+/*!
+    Returns the top/bottom vertical margin of the view.
+*/
+int QComboBoxPrivateContainer::topMargin() const
+{
+    if (const QListView *lview = qobject_cast<const QListView*>(view))
+        return lview->spacing();
+#ifndef QT_NO_TABLEVIEW
+    if (const QTableView *tview = qobject_cast<const QTableView*>(view))
+        return tview->showGrid() ? 1 : 0;
+#endif
+    return 0;
+}
+
 /*!
     Returns the spacing between the items in the view.
 */
@@ -578,7 +592,7 @@ int QComboBoxPrivateContainer::spacing() const
 {
     QListView *lview = qobject_cast<QListView*>(view);
     if (lview)
-        return lview->spacing();
+        return 2 * lview->spacing(); // QListView::spacing is the padding around the item.
 #ifndef QT_NO_TABLEVIEW
     QTableView *tview = qobject_cast<QTableView*>(view);
     if (tview)
@@ -2528,7 +2542,7 @@ void QComboBox::showPopup()
                 QModelIndex idx = d->model->index(i, d->modelColumn, parent);
                 if (!idx.isValid())
                     continue;
-                listHeight += view()->visualRect(idx).height() + container->spacing();
+                listHeight += view()->visualRect(idx).height();
 #ifndef QT_NO_TREEVIEW
                 if (d->model->hasChildren(idx) && treeView && treeView->isExpanded(idx))
                     toCheck.push(idx);
@@ -2540,12 +2554,14 @@ void QComboBox::showPopup()
                 }
             }
         }
+        if (count > 1)
+            listHeight += (count - 1) * container->spacing();
         listRect.setHeight(listHeight);
     }
 
     {
         // add the spacing for the grid on the top and the bottom;
-        int heightMargin = 2*container->spacing();
+        int heightMargin = container->topMargin()  + container->bottomMargin();
 
         // add the frame of the container
         int marginTop, marginBottom;

src/widgets/widgets/qcombobox_p.h

@@ -214,6 +214,8 @@ public:
     QAbstractItemView *itemView() const;
     void setItemView(QAbstractItemView *itemView);
     int spacing() const;
+    int topMargin() const;
+    int bottomMargin() const { return topMargin(); }
     void updateTopBottomMargin();
 
     QTimer blockMouseReleaseTimer;

tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp

@@ -152,6 +152,7 @@ private slots:
     void resetModel();
     void keyBoardNavigationWithMouse();
     void task_QTBUG_1071_changingFocusEmitsActivated();
+    void maxVisibleItems_data();
     void maxVisibleItems();
     void task_QTBUG_10491_currentIndexAndModelColumn();
     void highlightedSignal();
@@ -2749,8 +2750,18 @@ void tst_QComboBox::task_QTBUG_1071_changingFocusEmitsActivated()
     QTRY_COMPARE(spy.count(), 1);
 }
 
+void tst_QComboBox::maxVisibleItems_data()
+{
+    QTest::addColumn<int>("spacing");
+    QTest::newRow("Default")  << -1;
+    QTest::newRow("No spacing") << 0;
+    QTest::newRow("20")  << -1;
+}
+
 void tst_QComboBox::maxVisibleItems()
 {
+    QFETCH(int, spacing);
+
     QComboBox comboBox;
     QCOMPARE(comboBox.maxVisibleItems(), 10); //default value.
 
@@ -2771,15 +2782,18 @@ void tst_QComboBox::maxVisibleItems()
     QTRY_VERIFY(comboBox.view());
     QTRY_VERIFY(comboBox.view()->isVisible());
 
-    QAbstractItemView *v = comboBox.view();
-    int itemHeight = v->visualRect(v->model()->index(0,0)).height();
-    QListView *lv = qobject_cast<QListView*>(v);
-    if (lv)
-        itemHeight += lv->spacing();
+    QListView *listView = qobject_cast<QListView*>(comboBox.view());
+    QVERIFY(listView);
+    if (spacing >= 0)
+        listView->setSpacing(spacing);
+
+    const int itemHeight = listView->visualRect(listView->model()->index(0,0)).height()
+        + 2 * listView->spacing();
+
     QStyleOptionComboBox opt;
     opt.initFrom(&comboBox);
     if (!comboBox.style()->styleHint(QStyle::SH_ComboBox_Popup, &opt))
-        QCOMPARE(v->viewport()->height(), itemHeight * comboBox.maxVisibleItems());
+        QCOMPARE(listView->viewport()->height(), itemHeight * comboBox.maxVisibleItems());
 }
 
 void tst_QComboBox::task_QTBUG_10491_currentIndexAndModelColumn()