From a418d6229e31dac254660da2a417b4306f066ae3 Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Wed, 28 Oct 2020 17:52:29 +0100 Subject: [PATCH] Fix a crash and extract unrelated changes The secondary UrlNavigator is now created when and only when: - split view mode is activated for the active tab OR - switching to a tab that has split view already enabled. This fixes a crash that occurs when the setting to always start in split view mode is enabled. An animation for activating split view is also removed from this and moved into a separate MR. Another unrelated name change left over from a previous commit (viewContainers() -> activeViewContainers()) is dropped. --- src/dolphinbookmarkhandler.cpp | 2 +- src/dolphinmainwindow.cpp | 3 ++- src/dolphinmainwindow.h | 6 ++--- src/dolphinnavigatorswidgetaction.h | 8 +++++- src/dolphintabpage.cpp | 41 +---------------------------- src/dolphintabpage.h | 2 -- src/dolphintabwidget.cpp | 8 +++--- 7 files changed, 18 insertions(+), 52 deletions(-) diff --git a/src/dolphinbookmarkhandler.cpp b/src/dolphinbookmarkhandler.cpp index 576a9314b..be4f447d8 100644 --- a/src/dolphinbookmarkhandler.cpp +++ b/src/dolphinbookmarkhandler.cpp @@ -68,7 +68,7 @@ bool DolphinBookmarkHandler::supportsTabs() const QList DolphinBookmarkHandler::currentBookmarkList() const { - const auto viewContainers = m_mainWindow->activeViewContainers(); + const auto viewContainers = m_mainWindow->viewContainers(); QList bookmarks; bookmarks.reserve(viewContainers.size()); for (const auto viewContainer : viewContainers) { diff --git a/src/dolphinmainwindow.cpp b/src/dolphinmainwindow.cpp index 6a93de078..8b389ce9b 100644 --- a/src/dolphinmainwindow.cpp +++ b/src/dolphinmainwindow.cpp @@ -213,7 +213,7 @@ DolphinMainWindow::~DolphinMainWindow() { } -QVector DolphinMainWindow::activeViewContainers() const +QVector DolphinMainWindow::viewContainers() const { QVector viewContainers; @@ -2178,6 +2178,7 @@ void DolphinMainWindow::connectViewSignals(DolphinViewContainer* container) auto navigators = static_cast (actionCollection()->action(QStringLiteral("url_navigators"))); + KUrlNavigator *navigator = m_tabWidget->currentTabPage()->primaryViewActive() ? navigators->primaryUrlNavigator() : navigators->secondaryUrlNavigator(); diff --git a/src/dolphinmainwindow.h b/src/dolphinmainwindow.h index 173e017c9..29ab6326d 100644 --- a/src/dolphinmainwindow.h +++ b/src/dolphinmainwindow.h @@ -65,12 +65,12 @@ public: * having a split view setup, the nonactive view * is usually shown in darker colors. */ - DolphinViewContainer *activeViewContainer() const; + DolphinViewContainer* activeViewContainer() const; /** - * Returns the active view containers of all tabs. + * Returns view container for all tabs */ - QVector activeViewContainers() const; + QVector viewContainers() const; /** * Opens each directory in \p dirs in a separate tab. If \a splitView is set, diff --git a/src/dolphinnavigatorswidgetaction.h b/src/dolphinnavigatorswidgetaction.h index c1808d68e..8046ce2dc 100644 --- a/src/dolphinnavigatorswidgetaction.h +++ b/src/dolphinnavigatorswidgetaction.h @@ -57,7 +57,13 @@ public: bool addToToolbarAndSave(KXmlGuiWindow *mainWindow); /** - * Different to the primary UrlNavigator, the secondary UrlNavigator is only created on-demand. + * The secondary UrlNavigator is only created on-demand. Such an action is not necessary + * for the primary UrlNavigator which is created preemptively. + * + * This method should preferably only be called when: + * - Split view is activated in the active tab + * OR + * - A switch to a tab that is already in split view mode is occuring */ void createSecondaryUrlNavigator(); diff --git a/src/dolphintabpage.cpp b/src/dolphintabpage.cpp index d2fd1d143..d196508a8 100644 --- a/src/dolphintabpage.cpp +++ b/src/dolphintabpage.cpp @@ -9,7 +9,6 @@ #include "dolphin_generalsettings.h" #include "dolphinviewcontainer.h" -#include #include #include @@ -70,7 +69,6 @@ void DolphinTabPage::setSplitViewEnabled(bool enabled, const QUrl &secondaryUrl) m_splitViewEnabled = enabled; if (enabled) { - int splitterTotalWidth = m_splitter->width(); const QUrl& url = (secondaryUrl.isEmpty()) ? m_primaryViewContainer->url() : secondaryUrl; m_secondaryViewContainer = createViewContainer(url); @@ -84,33 +82,8 @@ void DolphinTabPage::setSplitViewEnabled(bool enabled, const QUrl &secondaryUrl) m_splitter->addWidget(m_secondaryViewContainer); m_secondaryViewContainer->installEventFilter(this); - m_secondaryViewContainer->setActive(true); - - // opening animation - m_splitter->widget(1)->setMinimumWidth(1); - const QList splitterSizes = {m_splitter->width(), 0}; - m_splitter->setSizes(splitterSizes); - - // TODO: This is only here to test the robustness of DolphinNavigatorsWidgetAction! I still have to move it to another merge request! - m_splitViewAnimation = new QVariantAnimation(m_splitter); - m_splitViewAnimation->setDuration(200); // TODO: where do I get the animation times from again? - m_splitViewAnimation->setStartValue(splitterTotalWidth); - m_splitViewAnimation->setEndValue(splitterTotalWidth / 2); - m_splitViewAnimation->setEasingCurve(QEasingCurve::OutCubic); - - connect(m_splitViewAnimation, &QVariantAnimation::valueChanged, [=]() { - if (m_splitter->count() != 2) { - return; - } - int value = m_splitViewAnimation->currentValue().toInt(); - const QList splitterSizes = {value, m_splitter->width() - value}; - m_splitter->setSizes(splitterSizes); - if (value == m_splitViewAnimation->endValue().toInt()) { - m_splitter->widget(1)->setMinimumWidth(m_splitter->widget(1)->minimumSizeHint().width()); - } - }); - m_splitViewAnimation->start(QAbstractAnimation::DeleteWhenStopped); m_secondaryViewContainer->show(); + m_secondaryViewContainer->setActive(true); } else { m_navigatorsWidget->setSecondaryNavigatorVisible(false); m_secondaryViewContainer->disconnectUrlNavigator(); @@ -144,10 +117,6 @@ void DolphinTabPage::setSplitViewEnabled(bool enabled, const QUrl &secondaryUrl) m_primaryViewContainer->setActive(true); view->close(); view->deleteLater(); - if (m_splitViewAnimation) { - delete m_splitViewAnimation; - m_splitter->widget(0)->setMinimumWidth(m_splitter->widget(0)->minimumSizeHint().width()); - } } } } @@ -194,10 +163,6 @@ void DolphinTabPage::connectNavigators(DolphinNavigatorsWidgetAction *navigators m_primaryViewContainer->connectUrlNavigator(primaryNavigator); if (m_splitViewEnabled) { auto secondaryNavigator = navigatorsWidget->secondaryUrlNavigator(); - if (!secondaryNavigator) { - navigatorsWidget->createSecondaryUrlNavigator(); - secondaryNavigator = navigatorsWidget->secondaryUrlNavigator(); - } secondaryNavigator->setActive(!m_primaryViewActive); m_secondaryViewContainer->connectUrlNavigator(secondaryNavigator); } @@ -334,10 +299,6 @@ void DolphinTabPage::restoreState(const QByteArray& state) m_navigatorsWidget->primaryUrlNavigator()->setActive(false); } - if (m_splitViewAnimation) { - delete m_splitViewAnimation; - m_splitter->widget(0)->setMinimumWidth(m_splitter->widget(0)->minimumSizeHint().width()); - } QByteArray splitterState; stream >> splitterState; m_splitter->restoreState(splitterState); diff --git a/src/dolphintabpage.h b/src/dolphintabpage.h index 6a8801edd..650594214 100644 --- a/src/dolphintabpage.h +++ b/src/dolphintabpage.h @@ -14,7 +14,6 @@ class DolphinNavigatorsWidgetAction; class DolphinViewContainer; class QSplitter; -class QVariantAnimation; class KFileItemList; class DolphinTabPage : public QWidget @@ -175,7 +174,6 @@ private: QPointer m_navigatorsWidget; QPointer m_primaryViewContainer; QPointer m_secondaryViewContainer; - QPointer m_splitViewAnimation; bool m_primaryViewActive; bool m_splitViewEnabled; diff --git a/src/dolphintabwidget.cpp b/src/dolphintabwidget.cpp index a09a769d3..eb3f741ee 100644 --- a/src/dolphintabwidget.cpp +++ b/src/dolphintabwidget.cpp @@ -128,12 +128,9 @@ bool DolphinTabWidget::isUrlOpen(const QUrl &url) const void DolphinTabWidget::openNewActivatedTab() { std::unique_ptr oldNavigatorState; - if (currentTabPage()->primaryViewActive()) { + if (currentTabPage()->primaryViewActive() || !m_navigatorsWidget->secondaryUrlNavigator()) { oldNavigatorState = m_navigatorsWidget->primaryUrlNavigator()->visualState(); } else { - if (!m_navigatorsWidget->secondaryUrlNavigator()) { - m_navigatorsWidget->createSecondaryUrlNavigator(); - } oldNavigatorState = m_navigatorsWidget->secondaryUrlNavigator()->visualState(); } @@ -401,6 +398,9 @@ void DolphinTabWidget::currentTabChanged(int index) m_lastViewedTab->disconnectNavigators(); m_lastViewedTab->setActive(false); } + if (tabPage->splitViewEnabled() && !m_navigatorsWidget->secondaryUrlNavigator()) { + m_navigatorsWidget->createSecondaryUrlNavigator(); + } DolphinViewContainer* viewContainer = tabPage->activeViewContainer(); Q_EMIT activeViewChanged(viewContainer); Q_EMIT currentUrlChanged(viewContainer->url()); -- 2.47.3