commit 620f9670af432c8c7a79fd47789da1788d7d6b17 Author: Frank Reininghaus Date: Fri Dec 7 22:15:32 2012 +0100 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 diff --git a/dolphin/src/kitemviews/kstandarditemlistwidget.cpp b/dolphin/src/kitemviews/kstandarditemlistwidget.cpp index f92cab5..af16954 100644 --- a/dolphin/src/kitemviews/kstandarditemlistwidget.cpp +++ b/dolphin/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/dolphin/src/kitemviews/kstandarditemlistwidget.h b/dolphin/src/kitemviews/kstandarditemlistwidget.h index 787722d..386f60e 100644 --- a/dolphin/src/kitemviews/kstandarditemlistwidget.h +++ b/dolphin/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/dolphin/src/kitemviews/private/kitemlistroleeditor.cpp b/dolphin/src/kitemviews/private/kitemlistroleeditor.cpp index 78dbfe9..1e4b5fd 100644 --- a/dolphin/src/kitemviews/private/kitemlistroleeditor.cpp +++ b/dolphin/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/dolphin/src/kitemviews/private/kitemlistroleeditor.h b/dolphin/src/kitemviews/private/kitemlistroleeditor.h index a2f7058..aa2c977 100644 --- a/dolphin/src/kitemviews/private/kitemlistroleeditor.h +++ b/dolphin/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