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.

diff --git a/src/dolphinbookmarkhandler.cpp b/src/dolphinbookmarkhandler.cpp
index 576a9314b15edadbd4c55ec6e8e75251eae995a5..be4f447d86a77935a47328fa68d39413115671c5 100644 (file)
--- a/src/dolphinbookmarkhandler.cpp
+++ b/src/dolphinbookmarkhandler.cpp
@@ -68,7 +68,7 @@ bool DolphinBookmarkHandler::supportsTabs() const
 
 QList<KBookmarkOwner::FutureBookmark> DolphinBookmarkHandler::currentBookmarkList() const
 {
-    const auto viewContainers = m_mainWindow->activeViewContainers();
+    const auto viewContainers = m_mainWindow->viewContainers();
     QList<FutureBookmark> bookmarks;
     bookmarks.reserve(viewContainers.size());
     for (const auto viewContainer : viewContainers) {

diff --git a/src/dolphinmainwindow.cpp b/src/dolphinmainwindow.cpp
index 6a93de0780c1cf8997886b4adf45acceac239966..8b389ce9ba665f0b0173ea037cb2835472fdd1cc 100644 (file)
--- a/src/dolphinmainwindow.cpp
+++ b/src/dolphinmainwindow.cpp
@@ -213,7 +213,7 @@ DolphinMainWindow::~DolphinMainWindow()
 {
 }
 
-QVector<DolphinViewContainer *> DolphinMainWindow::activeViewContainers() const
+QVector<DolphinViewContainer *> DolphinMainWindow::viewContainers() const
 {
     QVector<DolphinViewContainer*> viewContainers;
 
@@ -2178,6 +2178,7 @@ void DolphinMainWindow::connectViewSignals(DolphinViewContainer* container)
 
     auto navigators = static_cast<DolphinNavigatorsWidgetAction *>
         (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 173e017c9a75ce4032f64133f25e31938d2cd947..29ab6326dac20128fc7aa9bdff59b7f74ff52f16 100644 (file)
--- 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<DolphinViewContainer *> activeViewContainers() const;
+    QVector<DolphinViewContainer*> 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 c1808d68e6a7ddd68d864a9f93d6ffcef0efae28..8046ce2dcb066ab9e200180dc88ff96f4f38e6bc 100644 (file)
--- 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 d2fd1d1437ce5c1d1ca235404f4396869458ae6d..d196508a8756e71a9e44054f22a59bb1dd9d6e92 100644 (file)
--- a/src/dolphintabpage.cpp
+++ b/src/dolphintabpage.cpp
@@ -9,7 +9,6 @@
 #include "dolphin_generalsettings.h"
 #include "dolphinviewcontainer.h"
 
-#include <QPropertyAnimation>
 #include <QSplitter>
 #include <QVBoxLayout>
 
@@ -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<int> 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<int> 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 6a8801eddcbadf414485ea169cefab559a963464..650594214a1df20a43b3d117a1ba722e19938a67 100644 (file)
--- 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<DolphinNavigatorsWidgetAction> m_navigatorsWidget;
     QPointer<DolphinViewContainer> m_primaryViewContainer;
     QPointer<DolphinViewContainer> m_secondaryViewContainer;
-    QPointer<QVariantAnimation> m_splitViewAnimation;
 
     bool m_primaryViewActive;
     bool m_splitViewEnabled;

diff --git a/src/dolphintabwidget.cpp b/src/dolphintabwidget.cpp
index a09a769d37c7497a9e2f185821daa93c713b1158..eb3f741ee741be46e8a067313f95e62364c4925c 100644 (file)
--- 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<DolphinUrlNavigator::VisualState> 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());