QItemDelegate Editor Crash

[mclark]

Greetings,

I am using Qt4.2.2 on Windows. I am seeing a crash when trying to validate a string in a QLineEdit object created in a QItemDelegate.

There are 6 characters that are invalid for this string. My thought was to create a delegate that would handle this (based off of the TrackEditor example in the "C++ GUI Programming With Qt4" book.

If the user enters an invalid character in the string, '(' for example, a message will be displayed to inform them that the string will be changed to replace the '(' with '['. The string is then changed and displayed.

This all happens after the user presses the Tab or Enter keys. Pressing the Tab key causes this to work as designed. Pressing the Enter key causes a crash. It is crashing after the processing in the SLOT_itemChanged() call, but before the commitData() call in delegate function, commitAndCloseEditor().

My table also has eventHandler code to determine if the user is exiting the QTableWidgetItem via Tab or Enter key press. This code is not hit, the error occurs before this is called.

It appears that the pointer to the actual string in the QLineEdit object is getting corrupted by me trying to change it. The fact that it works when a Tab is pressed but not when Enter is pressed is bothersome.

Has anyone else seen this behavior? Or, might there be a better way to validate a string in a QTableWidget cell?

Thanks for taking the time to look at this.

In my table ctor:

m_pTableCellKeyHandler = new TableCellKeyHandler( this );

setItemDelegate( new DMXTableCellDelegate( this ) );

connect( this, SIGNAL( itemChanged( QTableWidgetItem* ) ),
this, SLOT( SLOT_itemChanged( QTableWidgetItem* ) ) );



QWidget* DMXTableCellDelegate::createEditor( QWidget* parent,
const QStyleOptionViewItem& option,
const QModelIndex& index ) const
{
if ( index.column() == COL_Name )
{
QLineEdit* lineEdit = new QLineEdit( parent );
lineEdit->setMaxLength( DEVICE_NAME_MAX );

connect( lineEdit, SIGNAL( editingFinished() ),
this, SLOT( commitAndCloseEditor() ) );

return lineEdit;
}
else if ( index.column() == COL_Next )
{
...
}
else
return QItemDelegate::createEditor( parent, option, index );
}


void DMXTableCellDelegate::commitAndCloseEditor( void )
{
QLineEdit* editor = qobject_cast<QLineEdit*>( sender() );
emit commitData( editor );
emit closeEditor( editor );
}


void DMXTable::SLOT_itemChanged( QTableWidgetItem* pItem )
{
int nRow = pItem->row();
int nCol = pItem->column();

CDMXGateway* pDevice = static_cast<CDMXGateway*>( GetDeviceData( nRow ) );
if ( pDevice == NULL )
return;

// Turn off sorting
m_pDMXCfgTable->setSortingEnabled( false );

// Process device-specific cells
bool bValueChanged = false;
QString sNewName;
bool bNameChanged = false;

switch ( nCol )
{
case COL_Name:
{
sNewName = pItem->text();

if ( (bNameChanged = updateNameItem( pItem, sNewName, static_cast<CNode*>( pDevice ) )) )
bValueChanged = true;

break;
}
...
}

m_pDMXCfgTable->setSortingEnabled( true );
}


bool Table::updateNameItem( QTableWidgetItem* pItem,
QString& sNewName, CNode* pNode )
{
bool bNameChanged = false;

QString sOldName = pNode->GetDeviceName();

// Compare new name to old name
if ( sOldName.compare( sNewName, Qt::CaseSensitive ) != 0 )
{
QString sTypedName( sNewName );
if ( !m_parent->ValidateDeviceName( sNewName ) )
{
// Restrict length
if ( sNewName.length() > DEVICE_NAME_MAX )
sNewName.truncate( DEVICE_NAME_MAX );

if ( !m_parent->ShowRestrictedCharMsg( sTypedName, sOldName, sNewName ) )
{
pItem->setText( sOldName );
return false;
}

pItem->setText( sNewName );
}

pNode->SetDeviceName( sNewName );
bNameChanged = true;

if ( !m_bIsCfgTable && pNode->IsOnline() ) // update and save now
m_parent->m_pController->UpdateDeviceName( pNode->GetDevType(), sNewName, pNode, true );
}

return bNameChanged;
}


bool MainWin::ValidateDeviceName( QString& sName )
{
bool bCharIsValid = true;
QChar c;

// Substitute for illegal Discovery characters
for ( int i = 0; i < sName.length(); i++ )
{
c = sName.at( i );

if ( c == '(' )
{
sName.replace( i, 1, "[" );
bCharIsValid = false;
}
...
}

return bCharIsValid;
}


bool TableCellKeyHandler::eventFilter( QObject* obj, QEvent* pEvent )
{
if ( pEvent->type() == QEvent::KeyPress )
{
QKeyEvent* pKeyEvent = static_cast<QKeyEvent*>( pEvent );
int nKey = pKeyEvent->key();

// Check for enter keys: 0x01000004 || 0x01000005
if ( (nKey == Qt::Key_Return) || (nKey == Qt::Key_Enter) )
{
int nRow = m_pParent->currentRow();
int nCol = m_pParent->currentColumn();
qDebug( "eventFilter(): ( %u, %u )", nRow, nCol );

...
}
}
}

[wysota]

How about simply installing a QValidator subclass instance on the editor and letting it handle the validation for you? Alternatively you can handle validation in the model itself - in its setData() method (or in setData() of the item itself in case you're using convenience widgets and not the model approach).

[mclark]

Thanks for the reply.

I want to process the entire (finished) string. Using a QValidator subclass forces handling each restricted character as it is typed -- too cumbersome for what I want to do.

I am somewhat of a newbie with Qt and don't understand how to access the setData() method you reference while using a QTableWidget. Is there an example or reference you could point me to in Qt Assistant or the Qt 4 book I mentioned?

[wysota]

I am somewhat of a newbie with Qt and don't understand how to access the setData() method you reference while using a QTableWidget. Is there an example or reference you could point me to in Qt Assistant or the Qt 4 book I mentioned?

You see, there are different ways of manipulating the edited data depending on what exactly you want to achieve. Basically using the model approach gives you more control over what is happening with your data. If you want to stick with the convenience approach (using *Widget instead of *View), you need to implement your own item class and reimplement its setData() method. In that method you can change the data supplied by the user so that it is valid for your model (with the model approach you could also reject the change). Alternatively you can manipulate the data by reimplementing setModelData() method of the delegate to achieve a similar effect.

[mclark]

wysota, thanks for helping me with this.

Since I inherited the QTableWidget, release deadlines require me to continue its use, although from everything I've read I really need a QTableView. Since I'm already using a delegate to handle other table cell validation it would seem to make sense to explore the setModelData() route.

Does something like this appear appropriate (provided I don't mess up the validation code;))?


void DMXTableCellDelegate::setModelData( QWidget* editor, QAbstractItemModel* model,
const QModelIndex& index ) const
{
if ( index.column() == COL_Name )
{
QLineEdit* lineEdit = qobject_cast<QLineEdit*>( editor );

if ( lineEdit != NULL && !lineEdit->text().isEmpty() )
{
QString sValidStr = lineEdit->text();

// validate will return true if the string was modified
if ( validate( sValidStr ) == true )
{
// update the model with the modified string
model->setData( index, sValidStr );
return;
}
}
}

// validate did NOT modify the string
QItemDelegate::setModelData( editor, model, index );
}

[wysota]

In general this is ok, but I think that setModelData will only be called if the contents of the editor is modified (I'm not sure about it though). But, as I said, your approach is correct. Note that the only problem is, that you have to have a valid piece of data here. At worst you can just ignore setData just like you seem to do. Oh, and remember about using the appropriate role! In most cases (but not always!) it will be Qt::EditRole.

[mclark]

Thanks wysota, I'll give this a try.

[mclark]

I subclassed QItemDelegate::setModelData() and I am now getting multiple calls into it when the QTableWidgetItem loses focus. The end result is a crash in a moc_** file when pressing 'Enter'.

Mouse-clicking, Tab and Enter all cause a double entry into the setModelData() function. Only the Enter key causes a crash.

I am baffled. Can anyone see something wrong with my implementation of setModelData()?


int Table::qt_metacall(QMetaObject::Call _c, int _id, void **_a)
{
--->_id = QTableWidget::qt_metacall(_c, _id, _a);
. . .



void DMXTableCellDelegate::setModelData( QWidget* editor, QAbstractItemModel* model,
const QModelIndex& index ) const
{
if ( index.column() == COL_Name )
{
QLineEdit* lineEdit = qobject_cast<QLineEdit*>( editor );

// Validate the Name string
if ( (lineEdit != NULL) || !lineEdit->text().isEmpty() )
{
QString sTypedName = lineEdit->text();

// Substitute for illegal characters
QString sNewName = "";
bool bIsValid = true;
QChar c;

for ( int i = 0; i < sTypedName.length(); i++ )
{
c = sTypedName.at( i );

if ( (c == '(') || (c == '<') )
{
c = '[';
bIsValid = false;
}
else if ( (c == ')') || (c == '>') )
{
. . .
}

sNewName.append( c );
}

if ( !bIsValid )
{
QString sOrigName = index.data( Qt::EditRole ).toString();

int rVal = QMessageBox::warning( qobject_cast<QWidget*>( m_parent ),
"Restricted Characters Found"),
"Message String"),
QMessageBox::Ok, QMessageBox::Cancel );
if ( rVal == QMessageBox::Cancel )
model->setData( index, QVariant( sOrigName ) );
else
model->setData( index, QVariant( sNewName ) );
}
}

return;
}

QItemDelegate::setModelData( editor, model, index );
}

[wysota]

Try not to fetch any data from the model.

[mclark]

Originally Posted by wysota
Try not to fetch any data from the model.

Are you speaking of the QModelIndex object?

I only use this string, index.data( Qt::EditRole ).toString(), to form the message for the warning. But, even if I don't use this string I get the same results!

[mclark]

All the problems I'm having are related to the QWidget* parameter in the QMessageBox() call that I'm using to warn the user. If I comment out only the QMessageBox call I do not get another call to setModelData(), nor do I get a crash.

These will cause the crash in setModelData():

QMessageBox::warning( editor,
"Message Title",
"Message String",
QMessageBox::Ok, QMessageBox::NoButton );

QMessageBox::warning( qobject_cast<QWidget*>( m_parent ),
"Message Title",
"Message String",
QMessageBox::Ok, QMessageBox::NoButton );


Is the problem that I cannot call QMessageBox from within the setModelData() function?

If this is NOT the problem, what QWidget* pointer should I be using when I call QMessageBox? The above examples use the QWidget* editor parameter and the delegate owners pointer. What else could I use here?

[wysota]

In general you shouldn't do that because setModelData can be called without the editor. If you really need such a mechanism, use a custom event or something like that to trigger the message box after the new data is already in the model and the flow returns to the event loop.

[mclark]

I am unfamiliar with customEvent. Since I already have an event filter for my base Table class, can I use it to receive the custom events?

I'm posting custom events from my setModelData() function but they are never received by my customEvent() handler. Clearly I'm misunderstanding how I need to connect these together. Any suggestions?

These are my class definitions:

class TableEvent : public QEvent
{
public:
TableEvent( Type type, const QString& sOrigValue ) : QEvent( type )
{
m_sOrigValue = sOrigValue;
}

public:
QString m_sOrigValue;

};


class TableKeyHandler : public QObject
{
Q_OBJECT

public:
TableKeyHandler( Table* parent = 0 ) { m_pParent = parent; }

protected:
bool eventFilter( QObject* obj, QEvent* pEvent );
void customEvent( TableEvent* pEvent );

private:
Table* m_pParent;

};


And this is how I'm attempting to process events?

bool TableKeyHandler::eventFilter( QObject* obj, QEvent* pEvent )
{
if ( pEvent->type() == QEvent::KeyPress )
{
QKeyEvent* pKeyEvent = static_cast<QKeyEvent*>( pEvent );
int nKey = pKeyEvent->key();
. . .
}
}


void TableCellKeyHandler::customEvent( TableEvent* pEvent )
{
TableEvent* pTableEvent = static_cast<TableEvent*>( pEvent );

if ( pEvent->type() == EVENT_NAME_CHANGE )
{
// do validation of Name here
pEvent->accept();
}
else

pEvent->ignore();
}

And this is how I'm attempting to post a custom event:

void DMXTableCellDelegate::setModelData( QWidget* editor, QAbstractItemModel* model,
const QModelIndex& index ) const
{
if ( index.column() == COL_Name )
{
QLineEdit* lineEdit = qobject_cast<QLineEdit*>( editor );

// Validate the Name string
if ( (lineEdit != NULL) || !lineEdit->text().isEmpty() )
{
QString sOrigName = index.data( Qt::EditRole ).toString();
TableEvent* pEvent = new TableEvent( EVENT_NAME_CHANGE, sOrigName );
QObject* pObj = qobject_cast<QObject*>(m_parent);
static_cast<DMXTable*>( m_parent )->m_parent->m_pApp->postEvent( pObj, pEvent );
}
}
. . .
}

[edice]

Hello there!

I know this is an ancient thread, but don't you hate it when a thread is full of questions and not answers?

I have seen a crash similar to what you describe, but might not be exactly the same.
The problem is due to the QMessageBox holding up the commit sequence, AND the model being reset at the same time.

When you call QMessageBox::warning(), it will NOT return from the warning() call until the user has pressed OK on the warning box that appears.

To make this happen, QMessageBox runs its own QT-event-processor loop within ::warning(), and that processes the gui drawing calls (to make the dialog appear and be interactive) AS WELL AS distributes and processes other events in the system.

In my case, one of those posted events was updating the data in the table,
which called model->reset(),
which reset the editor,
which called editor->deleteLater(),
and then the DeleteLater event was processed, my editor was deleted.
The trap is set.

When the OK in QMessageBox is pressed, delegate->setModelData() can finally return.
The very next call (in qabstractitemview.cpp) is editor->installEventFilter().
So it calls a method on a stale and invalid pointer. FAIL.

So my solution was to avoid opening a QMessageBox during setModelData().
Instead, if there are error messages to display, I post a message to the tableview / dialog / something else
and they can display the error message.

Takeaway message: setModelData() should not start a new event loop.