From 4bd9b6b212fddc84d8cafa85491827e1192ab908 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Sat, 8 Aug 2026 21:06:05 +1200 Subject: [PATCH] Fix crash changing a dynamic text's color and confirming with Enter Bugtracker #323: crash changing a label's color, but only when confirmed via Enter -- clicking the dialog's own OK button with the mouse doesn't crash. Reported on Windows 11 and Debian, with "QObject::installEventFilter(): Cannot filter events for objects in a different thread" immediately before the segfault. Root cause: DynamicTextItemDelegate::createEditor()'s color case constructed a QColorDialog and returned it directly as the item view's editor widget for the color cell -- unlike every other case in this same function, which returns a small inline widget (QSpinBox, QComboBox, or, for the adjacent font case, a plain placeholder). A QColorDialog is not designed to be used this way: it is not one of the objectNames this delegate's own eventFilter() special-cases, so Enter is handled by the base QStyledItemDelegate::eventFilter() as an ordinary "commit and destroy this small editor" trigger -- racing the dialog's own internal OK-button accept/close path, which on Windows can hand off to the native color picker. Clicking OK with the mouse doesn't go through the same key-press path, which is why only Enter crashed. Verified structurally: an embedded QColorDialog editor is a *child* widget of the view's viewport rather than a proper top-level dialog (confirmed with a standalone Qt program driving the real delegate through QAbstractItemView::edit() -- searching QApplication's top-level widgets never found it, only a search of the viewport's children did), which is the same "used as something it isn't" pattern, just observed a different way. Fix: mirror the font case immediately above -- resolve the color via the static, blocking QColorDialog::getColor() inside createEditor(), and hand back a plain QWidget with the result stashed in two properties (mirroring the font case's "ok" property) for setModelData() to read. By the time the view processes any commit trigger, the "editor" is an inert placeholder with no dialog state left to race. Verified end-to-end with the same standalone program: creates the model item, triggers editing, finds the real (top-level, this time) QColorDialog, clicks its actual OK button, confirms the color lands on the placeholder's properties, sends the editor a synthetic Enter keypress (the exact trigger from the bug report), and confirms the final committed value in the model matches the picked color. Also confirmed a full Release build (333/333) with no new warnings. --- sources/ui/dynamicelementtextmodel.cpp | 44 +++++++++++++++++++++----- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/sources/ui/dynamicelementtextmodel.cpp b/sources/ui/dynamicelementtextmodel.cpp index f33128729..e210ffba5 100644 --- a/sources/ui/dynamicelementtextmodel.cpp +++ b/sources/ui/dynamicelementtextmodel.cpp @@ -1686,9 +1686,37 @@ QWidget *DynamicTextItemDelegate::createEditor( } case DynamicElementTextModel::color: { - QColorDialog *cd = new QColorDialog(index.data(Qt::EditRole).value(), parent); - cd->setObjectName("color_dialog"); - return cd; + /* Like the font case above: run the dialog synchronously via + * its static convenience function and stash the result on a + * plain placeholder widget, rather than handing back the + * QColorDialog itself as the item view's "editor". + * + * The item view's own Enter/Escape handling (the base + * QStyledItemDelegate::eventFilter(), since "color_dialog" + * isn't one of the objectNames special-cased in this + * delegate's own eventFilter() above) treats whatever + * createEditor() returned as a small inline editor it + * commits and destroys directly on a key press. A QColorDialog + * is not that: on Windows it can hand off to the native + * color picker, whose own accept/close path then races the + * view's -- pressing Enter fired both, one tearing down an + * object the other was still using (case #323 on the bug + * tracker: "QObject::installEventFilter(): Cannot filter + * events for objects in a different thread" immediately + * followed by a segfault; clicking the dialog's own OK + * button with the mouse didn't reach the view's key + * handling, so it didn't crash). Resolving the dialog + * before returning removes the second, competing teardown + * path entirely. */ + QColor color = QColorDialog::getColor(index.data(Qt::EditRole).value(), parent); + QWidget *w = new QWidget(parent); + if (color.isValid()) + { + w->setProperty("color", color); + w->setProperty("ok", true); + } + w->setObjectName("color_dialog"); + return w; } case DynamicElementTextModel::pos: { @@ -1788,15 +1816,15 @@ void DynamicTextItemDelegate::setModelData( { if(QStandardItem *qsi = qsim->itemFromIndex(index)) { - QColorDialog *cd = static_cast (editor); - if (cd->result() == QDialog::Accepted) + if (editor->property("ok").toBool() == true) { - qsi->setData(cd->selectedColor(), Qt::EditRole); - qsi->setData(cd->selectedColor(), Qt::ForegroundRole); + QColor color = editor->property("color").value(); + qsi->setData(color, Qt::EditRole); + qsi->setData(color, Qt::ForegroundRole); } return; } - + } } else if (editor->objectName() == "info_text")