diff options
author | Tobias Markmann <tm@ayena.de> | 2015-05-18 20:10:57 (GMT) |
---|---|---|
committer | Kevin Smith <kevin.smith@isode.com> | 2015-05-26 16:07:14 (GMT) |
commit | 0c5d33fdc77226ea1a447211ea4b2198197ec715 (patch) | |
tree | f626def04c6abcb7d688c64f48fb6e6d4fd8be27 | |
parent | e544a3ad5a8d3cdf67554384f53895fad34ff9bc (diff) | |
download | swift-0c5d33fdc77226ea1a447211ea4b2198197ec715.zip swift-0c5d33fdc77226ea1a447211ea4b2198197ec715.tar.bz2 |
Fix trellis related bugs
Bugs like:
* Tab title not matching shown chat window
* Duplicated tab titles after tab movement
Swift is also subject to QTBUG-36455, which is fixed for Qt >= 5.3.0.
This commit removes the use of application wide focus handlers in
QtChatWindow class, the QtChatWindow::qAppFocusChanged method.
The reason for this is due to the way QTabBar::moveTab is implemented
in Qt which we use for the Trellis feature.
Internally QTabBar::moveTab first adjusts its tab bar labels, then
removes the tab from its old location in the internal QStackedLayout
and then inserts it in the new location. After the remove Qt gives
focus to another widget via a focus change that does not go through
the event loop of the application and is not interceptable with
eventFilters.
Previously we would set the focus and call other signals in the
application wide focus change handler which then used the currently
inconsistent QTabBar/QTabWidget.
Test-Information:
Tested tab switching and movement on Windows 8 (Qt 5.3.2),
OS X 10.9.5 (Qt 5.4.3) and Ubuntu 14.04.2 LTS (Qt 5.4.3).
Change-Id: Ief423c4add58a90279109f72fac95fc58cb71111
-rw-r--r-- | Swift/QtUI/QtChatWindow.cpp | 24 | ||||
-rw-r--r-- | Swift/QtUI/QtChatWindow.h | 3 | ||||
-rw-r--r-- | Swift/QtUI/QtTextEdit.cpp | 22 | ||||
-rw-r--r-- | Swift/QtUI/QtTextEdit.h | 16 | ||||
-rw-r--r-- | Swift/QtUI/Trellis/QtDynamicGridLayout.cpp | 60 | ||||
-rw-r--r-- | Swift/QtUI/Trellis/QtDynamicGridLayout.h | 3 |
6 files changed, 95 insertions, 33 deletions
diff --git a/Swift/QtUI/QtChatWindow.cpp b/Swift/QtUI/QtChatWindow.cpp index 0b9b1af..ffcbf15 100644 --- a/Swift/QtUI/QtChatWindow.cpp +++ b/Swift/QtUI/QtChatWindow.cpp @@ -143,6 +143,8 @@ QtChatWindow::QtChatWindow(const QString& contact, QtChatTheme* theme, UIEventSt inputBarLayout->addWidget(correctingLabel_); correctingLabel_->hide(); + connect(input_, SIGNAL(receivedFocus()), this, SLOT(handleTextInputReceivedFocus())); + connect(input_, SIGNAL(lostFocus()), this, SLOT(handleTextInputLostFocus())); QPushButton* emoticonsButton_ = new QPushButton(this); emoticonsButton_->setIcon(QIcon(":/emoticons/smile.png")); connect(emoticonsButton_, SIGNAL(clicked()), this, SLOT(handleEmoticonsButtonClicked())); @@ -171,7 +173,6 @@ QtChatWindow::QtChatWindow(const QString& contact, QtChatTheme* theme, UIEventSt logRosterSplitter_->setFocusProxy(input_); midBar_->setFocusProxy(input_); messageLog_->setFocusProxy(input_); - connect(qApp, SIGNAL(focusChanged(QWidget*, QWidget*)), this, SLOT(qAppFocusChanged(QWidget*, QWidget*))); connect(messageLog_, SIGNAL(gotFocus()), input_, SLOT(setFocus())); resize(400,300); connect(messageLog_, SIGNAL(fontResized(int)), this, SIGNAL(fontResized(int))); @@ -458,17 +459,6 @@ void QtChatWindow::convertToMUC(MUCType mucType) { subject_->setVisible(!impromptu_); } -void QtChatWindow::qAppFocusChanged(QWidget* /*old*/, QWidget* now) { - if (now && isWidgetSelected()) { - lastLineTracker_.setHasFocus(true); - input_->setFocus(); - onAllMessagesRead(); - } - else { - lastLineTracker_.setHasFocus(false); - } -} - void QtChatWindow::setOnline(bool online) { isOnline_ = online; if (!online) { @@ -657,6 +647,16 @@ void QtChatWindow::handleEmoticonClicked(QString emoticonAsText) { input_->textCursor().insertText(emoticonAsText); } +void QtChatWindow::handleTextInputReceivedFocus() { + lastLineTracker_.setHasFocus(true); + input_->setFocus(); + onAllMessagesRead(); +} + +void QtChatWindow::handleTextInputLostFocus() { + lastLineTracker_.setHasFocus(false); +} + void QtChatWindow::handleActionButtonClicked() { QMenu contextMenu; QAction* changeSubject = NULL; diff --git a/Swift/QtUI/QtChatWindow.h b/Swift/QtUI/QtChatWindow.h index 06c6064..19046c9 100644 --- a/Swift/QtUI/QtChatWindow.h +++ b/Swift/QtUI/QtChatWindow.h @@ -149,7 +149,6 @@ namespace Swift { void fontResized(int); protected slots: - void qAppFocusChanged(QWidget* old, QWidget* now); void closeEvent(QCloseEvent* event); void resizeEvent(QResizeEvent* event); void moveEvent(QMoveEvent* event); @@ -173,6 +172,8 @@ namespace Swift { void handleCurrentLabelChanged(int); void handleEmoticonsButtonClicked(); void handleEmoticonClicked(QString emoticonAsText); + void handleTextInputReceivedFocus(); + void handleTextInputLostFocus(); private: void updateTitleWithUnreadCount(); diff --git a/Swift/QtUI/QtTextEdit.cpp b/Swift/QtUI/QtTextEdit.cpp index a381cbe..fed8819 100644 --- a/Swift/QtUI/QtTextEdit.cpp +++ b/Swift/QtUI/QtTextEdit.cpp @@ -1,28 +1,28 @@ /* - * Copyright (c) 2010-2014 Isode Limited. + * Copyright (c) 2010-2015 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ #include <Swift/QtUI/QtTextEdit.h> -#include <boost/tuple/tuple.hpp> #include <boost/algorithm/string.hpp> #include <boost/bind.hpp> +#include <boost/tuple/tuple.hpp> #include <QApplication> #include <QFontMetrics> #include <QKeyEvent> -#include <QDebug> #include <QMenu> +#include <QTime> #include <Swiften/Base/foreach.h> -#include <SwifTools/SpellCheckerFactory.h> -#include <SwifTools/SpellChecker.h> - #include <Swift/Controllers/SettingConstants.h> +#include <SwifTools/SpellChecker.h> +#include <SwifTools/SpellCheckerFactory.h> + #include <Swift/QtUI/QtSpellCheckerWindow.h> #include <Swift/QtUI/QtSwiftUtil.h> #include <Swift/QtUI/QtUtilities.h> @@ -75,6 +75,16 @@ void QtTextEdit::keyPressEvent(QKeyEvent* event) { } } +void QtTextEdit::focusInEvent(QFocusEvent* event) { + receivedFocus(); + QTextEdit::focusInEvent(event); +} + +void QtTextEdit::focusOutEvent(QFocusEvent* event) { + lostFocus(); + QTextEdit::focusOutEvent(event); +} + void QtTextEdit::handleTextChanged() { QSize previous(maximumSize()); setMaximumSize(QSize(maximumWidth(), sizeHint().height())); diff --git a/Swift/QtUI/QtTextEdit.h b/Swift/QtUI/QtTextEdit.h index 5b645ba..67826ba 100644 --- a/Swift/QtUI/QtTextEdit.h +++ b/Swift/QtUI/QtTextEdit.h @@ -1,20 +1,20 @@ /* - * Copyright (c) 2010-2014 Isode Limited. + * Copyright (c) 2010-2015 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ #pragma once -#include <SwifTools/SpellParser.h> +#include <QPointer> +#include <QTextEdit> -#include <Swift/Controllers/Settings/SettingsProvider.h> #include <Swift/Controllers/SettingConstants.h> +#include <Swift/Controllers/Settings/SettingsProvider.h> -#include <Swift/QtUI/QtSpellCheckHighlighter.h> +#include <SwifTools/SpellParser.h> -#include <QTextEdit> -#include <QPointer> +#include <Swift/QtUI/QtSpellCheckHighlighter.h> namespace Swift { class SpellChecker; @@ -31,12 +31,16 @@ namespace Swift { void wordCorrected(QString& word); void returnPressed(); void unhandledKeyPressEvent(QKeyEvent* event); + void receivedFocus(); + void lostFocus(); public slots: void handleSettingChanged(const std::string& settings); protected: virtual void keyPressEvent(QKeyEvent* event); + virtual void focusInEvent(QFocusEvent* event); + virtual void focusOutEvent(QFocusEvent* event); virtual void contextMenuEvent(QContextMenuEvent* event); private slots: diff --git a/Swift/QtUI/Trellis/QtDynamicGridLayout.cpp b/Swift/QtUI/Trellis/QtDynamicGridLayout.cpp index cc3208b..e2b6e27 100644 --- a/Swift/QtUI/Trellis/QtDynamicGridLayout.cpp +++ b/Swift/QtUI/Trellis/QtDynamicGridLayout.cpp @@ -10,18 +10,20 @@ #include <QApplication> #include <QEvent> -#include <QLayoutItem> #include <QGridLayout> +#include <QLayoutItem> #include <QtDebug> +#include <Swiften/Base/Log.h> + #include <Swift/QtUI/QtSwiftUtil.h> -#include <Swift/QtUI/QtTabbable.h> #include <Swift/QtUI/QtTabWidget.h> +#include <Swift/QtUI/QtTabbable.h> #include <Swift/QtUI/Trellis/QtDNDTabBar.h> namespace Swift { -QtDynamicGridLayout::QtDynamicGridLayout(QWidget* parent, bool enableDND) : QWidget(parent), dndEnabled_(enableDND) { +QtDynamicGridLayout::QtDynamicGridLayout(QWidget* parent, bool enableDND) : QWidget(parent), dndEnabled_(enableDND), movingTab_(NULL) { gridLayout_ = new QGridLayout(this); setContentsMargins(0,0,0,0); setDimensions(QSize(1,1)); @@ -107,6 +109,10 @@ int QtDynamicGridLayout::indexOf(const QWidget* widget) const { } void QtDynamicGridLayout::handleApplicationFocusChanged(QWidget*, QWidget* newFocus) { + if (movingTab_) { + return; + } + if (newFocus) { if (isAncestorOf(newFocus)) { QtTabbable *newTab = dynamic_cast<QtTabbable*>(newFocus->parentWidget()); @@ -142,6 +148,29 @@ void QtDynamicGridLayout::removeTab(int index) { } } +/** + * This event filter serves the purpose of filtering out all QEvent::Show events targeted at + * all widgets excepts the currently moving widget. + * It is required because of the way Qt internally implements the QTabBar::moveTab method. + * It does not move the actual tab in the underlying structure, but instead removes it from + * a stacked layout and later adds it again. + * Both the remove and insert produce a lot signal emission and focus changes. Most of which + * the application MUST NOT react on because of the QTabBar and the corresponding QTabWidget + * being out of sync in an inconsistent state. + */ +bool QtDynamicGridLayout::eventFilter(QObject* object, QEvent* event) { + QtTabbable* tab = qobject_cast<QtTabbable*>(object); + if (!tab) { + return false; + } + if (tab && (tab != movingTab_)) { + if (event->type() == QEvent::Show) { + return true; + } + } + return false; +} + QWidget* QtDynamicGridLayout::currentWidget() const { QWidget* current = NULL; current = focusWidget(); @@ -394,6 +423,10 @@ void QtDynamicGridLayout::handleTabCloseRequested(int index) { } void QtDynamicGridLayout::handleTabCurrentChanged(int index) { + if (movingTab_) { + return; + } + if (index >= 0) { QTabWidget* sendingTabWidget = dynamic_cast<QTabWidget*>(sender()); assert(sendingTabWidget); @@ -417,14 +450,25 @@ void QtDynamicGridLayout::updateTabPositions() { } void QtDynamicGridLayout::moveTab(QtTabWidget* tabWidget, int oldIndex, int newIndex) { - tabWidget->widget(oldIndex)->blockSignals(true); - tabWidget->widget(newIndex)->blockSignals(true); #if QT_VERSION >= 0x040500 - tabWidget->tabBar()->moveTab(oldIndex, newIndex); + SWIFT_LOG_ASSERT(movingTab_ == NULL, error) << std::endl; + movingTab_ = qobject_cast<QtTabbable*>(tabWidget->widget(oldIndex)); + SWIFT_LOG_ASSERT(movingTab_ != NULL, error) << std::endl; + + if (movingTab_) { + // Install event filter that filters out events issued during the internal movement of the + // tab but not targeted at the moving tab. + qApp->installEventFilter(this); + + tabWidget->tabBar()->moveTab(oldIndex, newIndex); + + qApp->removeEventFilter(this); + SWIFT_LOG_ASSERT(movingTab_ == tabWidget->widget(newIndex), error) << std::endl; + } + movingTab_ = NULL; + tabWidget->widget(newIndex)->setFocus(); #else #warning Qt 4.5 or later is needed. Trying anyway, some things will be disabled. - tabWidget->widget(oldIndex)->blockSignals(false); - tabWidget->widget(newIndex)->blockSignals(false); #endif } diff --git a/Swift/QtUI/Trellis/QtDynamicGridLayout.h b/Swift/QtUI/Trellis/QtDynamicGridLayout.h index 616ee5e..ed8a9fc 100644 --- a/Swift/QtUI/Trellis/QtDynamicGridLayout.h +++ b/Swift/QtUI/Trellis/QtDynamicGridLayout.h @@ -45,6 +45,8 @@ namespace Swift { QHash<QString, QPoint> getTabPositions() const; void setTabPositions(const QHash<QString, QPoint> positions); + bool eventFilter(QObject* object, QEvent* event); + signals: void tabCloseRequested(int index); void onCurrentIndexChanged(int newIndex); @@ -73,5 +75,6 @@ namespace Swift { QGridLayout *gridLayout_; bool dndEnabled_; QHash<QString, QPoint> tabPositions_; + QtTabbable* movingTab_; }; } |