dev65
dev65

Reputation: 1605

QListWidget crash when reusing the widget associated with item

I have a custom simple widget inheriting from QWidget and I add it to a QListWidget like this :

void MainWindow::AddToWidgetList(const QString &tag, const QString &html)
{
    HtmlItem *html_item = new HtmlItem();
    html_item->set_tag(tag);
    html_item->set_html(html);
    connect(html_item, SIGNAL(RemoveIt(uintptr_t)), this, SLOT(on_RmBtn_clicked(uintptr_t)));
    QListWidgetItem *list_item = new QListWidgetItem();
    html_item->set_list_item(list_item);
    list_item->setSizeHint(html_item->sizeHint());
    ui->CodeBlocks->addItem(list_item);
    ui->CodeBlocks->setItemWidget(list_item, html_item);
}

I then want to move the selected element up when a button is pressed

void MainWindow::on_UpArrowBtn_clicked()
{
    if (ui->CodeBlocks->count() < 2)
        return;
    int current_row = ui->CodeBlocks->currentRow();
    if (current_row == 0)
        return;
    HtmlItem *item_widget = (HtmlItem*)ui->CodeBlocks->itemWidget(ui->CodeBlocks->item(current_row));
    QListWidgetItem *item = ui->CodeBlocks->takeItem(current_row);
    ui->CodeBlocks->insertItem(current_row - 1, item);
    ui->CodeBlocks->setItemWidget(item, item_widget);
}

but I get crash in this line :

ui->CodeBlocks->setItemWidget(item, item_widget);

Upvotes: 1

Views: 770

Answers (1)

Aleph0
Aleph0

Reputation: 6084

The following example shows what is happening. Basically, the rules are like this:

Calling setItemWidget transfers the ownership of the Item-Widget to the QListWidget instance. Hence, it is QListWidget's responsibility to destroy the set Item-Widget.

Now, QListWidget has no member, which allows to withdraw the ownership of a set Item-Widget. The only option one has is to create a new Item-Widget with the same properties like Item-Widget about to removed.

Note, that the Item-Widget ist deleted later after returning to the event loop, which happens by calling deleteLater() inside of takeItem. Hence, it is valid to access label till the end of the slot.

If you are not happy with this behavior, you are still able to switch to a QListView class with your own delegate. Albeit this seems to be more work, it is the more extensible approach.

#include <QApplication>
#include <QHBoxLayout>
#include <QPushButton>
#include <QListWidget>
#include <QLabel>
#include <QDebug>

int main(int argc, char** args) {
    QApplication app(argc, args);
    auto frame = new QFrame;
    auto listWidget = new QListWidget;
    for (auto iter=0; iter<10; iter++)
    {
        auto label = new QLabel(QString("Item-%1").arg(iter));
        auto item = new QListWidgetItem();      
        listWidget->addItem(item);
        listWidget->setItemWidget(item, label); // listWidget becomes the owner of label
    }
    auto moveUp = new QPushButton("Move Up");
    frame->setLayout(new QHBoxLayout);
    frame->layout()->addWidget(listWidget);
    frame->layout()->addWidget(moveUp);
    frame->show();
    QObject::connect(moveUp, &QPushButton::clicked, [&]()
    {
            auto row = listWidget->currentRow();
            auto item=listWidget->currentItem();
            if (!item) return;
            if (row == 0) return;

            auto label = qobject_cast<QLabel*>(listWidget->itemWidget(item));
            if (!label) return;

            QObject::connect(label, &QLabel::destroyed, []()
                {
                    qDebug() << "Destroyed"; // takeItem calls deleteLater on itemWidget
                });
            auto myItem=listWidget->takeItem(row);
            listWidget->insertItem(row-1,myItem);
            listWidget->setItemWidget(item, new QLabel(label->text())); // copy content of itemWidget and create new widget
            listWidget->setCurrentRow(row-1);
    });
    app.exec();
}

Upvotes: 1

Related Questions