From 4bd9b6b212fddc84d8cafa85491827e1192ab908 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Sat, 8 Aug 2026 21:06:05 +1200 Subject: [PATCH 1/2] 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") From b0b5345e158d5ed56565146b2b5d3ff94543f663 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Mon, 10 Aug 2026 20:29:44 +1200 Subject: [PATCH 2/2] Commit font/color edits immediately instead of waiting for an unrelated click plc-user on PR #693: the crash is fixed, but the color/font field and the on-diagram text no longer update until you leave the properties list and click in the diagram -- previously it updated as soon as you clicked OK. That's a side effect of the crash fix itself. The old, crashing code returned a *live* QColorDialog as the item view's editor; clicking its OK button called accept()/hide() on it, and hiding the active editor happens to trip the base delegate's own focus-lost commit path -- so the value applied immediately, racily, as a side effect of the same mechanism that crashed on Enter. The fix (commit 4bd9b6b21) replaced that with running the dialog synchronously inside createEditor() and returning an inert placeholder with the result stashed in a property. Correct for the crash, but it also removed that accidental commit trigger: the placeholder never had focus to lose, so nothing tells the view to read the value back until some unrelated interaction (clicking away) incidentally triggers it. Fix: explicitly emit commitData()/closeEditor() for the resolved editor, deferred via QTimer::singleShot(0, ...) since the view only registers createEditor()'s return value as "the active editor" after createEditor() itself returns -- emitting synchronously, before returning, would target a widget the view doesn't know about yet. Applied to both font and color, since both share the exact same "resolve synchronously in createEditor(), return an inert placeholder" shape and thus the exact same gap; font just hadn't been reported. Verified with the same standalone harness from the crash fix (real QTreeView + DynamicTextItemDelegate + QAbstractItemView::edit()), this time deliberately *not* sending the synthetic Enter keypress the crash-fix verification needed: clicks the dialog's real OK button, lets the event loop run, and confirms the picked color lands in the model on its own. Also reconfirmed the crash fix itself still holds (clean exit, no synthetic-Enter needed either way now) and did a full Release build (504/504) with no new warnings. --- sources/ui/dynamicelementtextmodel.cpp | 12 ++++++++++++ sources/ui/dynamicelementtextmodel.h | 15 +++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/sources/ui/dynamicelementtextmodel.cpp b/sources/ui/dynamicelementtextmodel.cpp index e210ffba5..6ad12570a 100644 --- a/sources/ui/dynamicelementtextmodel.cpp +++ b/sources/ui/dynamicelementtextmodel.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include static int src_txt_row = 0; @@ -1595,6 +1596,15 @@ DynamicTextItemDelegate::DynamicTextItemDelegate(QObject *parent) : QStyledItemDelegate(parent) {} +void DynamicTextItemDelegate::commitAndCloseDeferred(QWidget *editor) const +{ + auto *self = const_cast(this); + QTimer::singleShot(0, self, [self, editor]() { + emit self->commitData(editor); + emit self->closeEditor(editor); + }); +} + QWidget *DynamicTextItemDelegate::createEditor( QWidget *parent, const QStyleOptionViewItem &option, @@ -1682,6 +1692,7 @@ QWidget *DynamicTextItemDelegate::createEditor( w->setProperty("ok", ok); } w->setObjectName("font_dialog"); + commitAndCloseDeferred(w); return w; } case DynamicElementTextModel::color: @@ -1716,6 +1727,7 @@ QWidget *DynamicTextItemDelegate::createEditor( w->setProperty("ok", true); } w->setObjectName("color_dialog"); + commitAndCloseDeferred(w); return w; } case DynamicElementTextModel::pos: diff --git a/sources/ui/dynamicelementtextmodel.h b/sources/ui/dynamicelementtextmodel.h index 67b42e23e..013d9318b 100644 --- a/sources/ui/dynamicelementtextmodel.h +++ b/sources/ui/dynamicelementtextmodel.h @@ -155,6 +155,21 @@ class DynamicTextItemDelegate : public QStyledItemDelegate private: QStringList availableInfo(DynamicElementTextItem *deti) const; + /** + @brief commitAndCloseDeferred + Schedule commitData()/closeEditor() for @a editor on the next + event loop iteration. For editors resolved synchronously inside + createEditor() (font/color, both run their picker dialog before + returning) there is no user interaction left to drive the base + QStyledItemDelegate::eventFilter()'s usual Enter/focus-out commit + path, so without this the value sits picked-but-uncommitted + until something unrelated (e.g. clicking elsewhere) happens to + trigger it. Deferred rather than called immediately: the view + only registers the widget createEditor() returns as "the active + editor" *after* createEditor() itself returns, so emitting here + would target an editor the view doesn't know about yet. + */ + void commitAndCloseDeferred(QWidget *editor) const; }; #endif // DYNAMICELEMENTTEXTMODEL_H