From e97c050157890dd1adf14d98bbed4aa86af98354 Mon Sep 17 00:00:00 2001 From: Frank Reininghaus Date: Fri, 7 Dec 2012 22:15:32 +0100 Subject: [PATCH] Fix keyboard focus handling after renaming items inline This reverts 951cb9c35d7a9ef814b3de5b359915968da9b881 and 3143acc084d54d43df469b54762bfa10a7050a9f, and fixes the crash caused by nested event loops by delaying the deletion of the KItemListRoleEditor until the next item is renamed inline. BUG: 311206 FIXED-IN: 4.9.5 REVIEW: 107606 --- src/kitemviews/kstandarditemlistwidget.cpp | 31 ++++++----- src/kitemviews/kstandarditemlistwidget.h | 1 + .../private/kitemlistroleeditor.cpp | 52 ++----------------- src/kitemviews/private/kitemlistroleeditor.h | 12 ----- 4 files changed, 21 insertions(+), 75 deletions(-) diff --git a/src/kitemviews/kstandarditemlistwidget.cpp b/src/kitemviews/kstandarditemlistwidget.cpp index f92cab50f..af1695465 100644 --- a/src/kitemviews/kstandarditemlistwidget.cpp +++ b/src/kitemviews/kstandarditemlistwidget.cpp @@ -193,7 +193,8 @@ KStandardItemListWidget::KStandardItemListWidget(KItemListWidgetInformant* infor m_additionalInfoTextColor(), m_overlay(), m_rating(), - m_roleEditor(0) + m_roleEditor(0), + m_oldRoleEditor(0) { } @@ -203,6 +204,7 @@ KStandardItemListWidget::~KStandardItemListWidget() m_textInfo.clear(); delete m_roleEditor; + delete m_oldRoleEditor; } void KStandardItemListWidget::setLayout(Layout layout) @@ -609,13 +611,16 @@ void KStandardItemListWidget::editedRoleChanged(const QByteArray& current, const this, SLOT(slotRoleEditingCanceled(int,QByteArray,QVariant))); disconnect(m_roleEditor, SIGNAL(roleEditingFinished(int,QByteArray,QVariant)), this, SLOT(slotRoleEditingFinished(int,QByteArray,QVariant))); - // Do not delete the role editor using deleteLater() because we might be - // inside a nested event loop which has been started by one of its event - // handlers (contextMenuEvent() or drag&drop inside mouseMoveEvent()). - m_roleEditor->deleteWhenIdle(); + m_oldRoleEditor = m_roleEditor; + m_roleEditor->hide(); m_roleEditor = 0; } return; + } else if (m_oldRoleEditor) { + // Delete the old editor before constructing the new one to + // prevent a memory leak. + m_oldRoleEditor->deleteLater(); + m_oldRoleEditor = 0; } Q_ASSERT(!m_roleEditor); @@ -1267,21 +1272,19 @@ QRectF KStandardItemListWidget::roleEditingRect(const QByteArray& role) const void KStandardItemListWidget::closeRoleEditor() { + disconnect(m_roleEditor, SIGNAL(roleEditingCanceled(int,QByteArray,QVariant)), + this, SLOT(slotRoleEditingCanceled(int,QByteArray,QVariant))); + disconnect(m_roleEditor, SIGNAL(roleEditingFinished(int,QByteArray,QVariant)), + this, SLOT(slotRoleEditingFinished(int,QByteArray,QVariant))); + if (m_roleEditor->hasFocus()) { // If the editing was not ended by a FocusOut event, we have // to transfer the keyboard focus back to the KItemListContainer. scene()->views()[0]->parentWidget()->setFocus(); } - disconnect(m_roleEditor, SIGNAL(roleEditingCanceled(int,QByteArray,QVariant)), - this, SLOT(slotRoleEditingCanceled(int,QByteArray,QVariant))); - disconnect(m_roleEditor, SIGNAL(roleEditingFinished(int,QByteArray,QVariant)), - this, SLOT(slotRoleEditingFinished(int,QByteArray,QVariant))); - - // Do not delete the role editor using deleteLater() because we might be - // inside a nested event loop which has been started by one of its event - // handlers (contextMenuEvent() or drag&drop inside mouseMoveEvent()). - m_roleEditor->deleteWhenIdle(); + m_oldRoleEditor = m_roleEditor; + m_roleEditor->hide(); m_roleEditor = 0; } diff --git a/src/kitemviews/kstandarditemlistwidget.h b/src/kitemviews/kstandarditemlistwidget.h index 787722ddd..386f60e4b 100644 --- a/src/kitemviews/kstandarditemlistwidget.h +++ b/src/kitemviews/kstandarditemlistwidget.h @@ -241,6 +241,7 @@ private: QPixmap m_rating; KItemListRoleEditor* m_roleEditor; + KItemListRoleEditor* m_oldRoleEditor; friend class KStandardItemListWidgetInformant; // Accesses private static methods to be able to // share a common layout calculation diff --git a/src/kitemviews/private/kitemlistroleeditor.cpp b/src/kitemviews/private/kitemlistroleeditor.cpp index 78dbfe95b..1e4b5fd4e 100644 --- a/src/kitemviews/private/kitemlistroleeditor.cpp +++ b/src/kitemviews/private/kitemlistroleeditor.cpp @@ -26,9 +26,7 @@ KItemListRoleEditor::KItemListRoleEditor(QWidget *parent) : KTextEdit(parent), m_index(0), m_role(), - m_blockFinishedSignal(false), - m_eventHandlingLevel(0), - m_deleteAfterEventHandling(false) + m_blockFinishedSignal(false) { setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff); @@ -66,20 +64,6 @@ QByteArray KItemListRoleEditor::role() const return m_role; } -void KItemListRoleEditor::deleteWhenIdle() -{ - if (m_eventHandlingLevel > 0) { - // We are handling an event at the moment. It could be that we - // are in a nested event loop run by contextMenuEvent() or a - // call of mousePressEvent() which results in drag&drop. - // -> do not call deleteLater() to prevent a crash when we - // return from the nested event loop. - m_deleteAfterEventHandling = true; - } else { - deleteLater(); - } -} - bool KItemListRoleEditor::eventFilter(QObject* watched, QEvent* event) { if (watched == parentWidget() && event->type() == QEvent::Resize) { @@ -91,42 +75,13 @@ bool KItemListRoleEditor::eventFilter(QObject* watched, QEvent* event) bool KItemListRoleEditor::event(QEvent* event) { - ++m_eventHandlingLevel; - if (event->type() == QEvent::FocusOut) { QFocusEvent* focusEvent = static_cast(event); if (focusEvent->reason() != Qt::PopupFocusReason) { emitRoleEditingFinished(); } } - - const int result = KTextEdit::event(event); - --m_eventHandlingLevel; - - if (m_deleteAfterEventHandling && m_eventHandlingLevel == 0) { - // Schedule this object for deletion and make sure that we do not try - // to deleteLater() again when the DeferredDelete event is received. - deleteLater(); - m_deleteAfterEventHandling = false; - } - - return result; -} - -bool KItemListRoleEditor::viewportEvent(QEvent* event) -{ - ++m_eventHandlingLevel; - const bool result = KTextEdit::viewportEvent(event); - --m_eventHandlingLevel; - - if (m_deleteAfterEventHandling && m_eventHandlingLevel == 0) { - // Schedule this object for deletion and make sure that we do not try - // to deleteLater() again when the DeferredDelete event is received. - deleteLater(); - m_deleteAfterEventHandling = false; - } - - return result; + return KTextEdit::event(event); } void KItemListRoleEditor::keyPressEvent(QKeyEvent* event) @@ -144,8 +99,7 @@ void KItemListRoleEditor::keyPressEvent(QKeyEvent* event) return; case Qt::Key_Enter: case Qt::Key_Return: - // TODO: find a better way to fix the bug 309760 - clearFocus(); // emitRoleEditingFinished(); results in a crash + emitRoleEditingFinished(); event->accept(); return; default: diff --git a/src/kitemviews/private/kitemlistroleeditor.h b/src/kitemviews/private/kitemlistroleeditor.h index a2f705808..aa2c97754 100644 --- a/src/kitemviews/private/kitemlistroleeditor.h +++ b/src/kitemviews/private/kitemlistroleeditor.h @@ -47,15 +47,6 @@ public: void setRole(const QByteArray& role); QByteArray role() const; - /** - * Calls deleteLater() if no event is being handled at the moment. - * Otherwise, the deletion is deferred until the event handling is - * finished. This prevents that the deletion happens inside a nested - * event loop which might be run in contextMenuEvent() or - * mouseMoveEvent() because this would probably cause a crash. - */ - void deleteWhenIdle(); - virtual bool eventFilter(QObject* watched, QEvent* event); signals: @@ -64,7 +55,6 @@ signals: protected: virtual bool event(QEvent* event); - virtual bool viewportEvent(QEvent* event); virtual void keyPressEvent(QKeyEvent* event); private slots: @@ -85,8 +75,6 @@ private: int m_index; QByteArray m_role; bool m_blockFinishedSignal; - int m_eventHandlingLevel; - bool m_deleteAfterEventHandling; }; #endif -- 2.47.3