Help: Replace annoying URL syncing by explicit Open in Edit Mode

Qt Creator was syncing the current page in Help mode to the page shown
in the side-by-side view in edit mode.
This doesn't really make sense because context is completely lost.
Instead provide the explicit option to open a help page in edit mode,
also from the external help window.

Fixes: QTCREATORBUG-19198
Change-Id: I00698bb431d5c116dd1e0e1cbdc5fbd7421ac267
Reviewed-by: Leena Miettinen <riitta-leena.miettinen@qt.io>
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/help/helpplugin.cpp

@@ -119,8 +119,6 @@ public:
     void saveExternalWindowSettings();
     void showLinksInCurrentViewer(const QMap<QString, QUrl> &links, const QString &key);
 
-    void updateSideBarSource(const QUrl &newUrl);
-
     void setupHelpEngineIfNeeded();
 
     HelpViewer *showHelpUrl(const QUrl &url, Core::HelpManager::HelpViewerLocation location);
@@ -206,8 +204,6 @@ HelpPluginPrivate::HelpPluginPrivate()
     }
 
     m_centralWidget = createHelpWidget(Context("Help.CentralHelpWidget"), HelpWidget::ModeWidget);
-    connect(m_centralWidget, &HelpWidget::sourceChanged,
-            this, &HelpPluginPrivate::updateSideBarSource);
     connect(HelpManager::instance(), &HelpManager::helpRequested,
             this, &HelpPluginPrivate::showHelpUrl);
     connect(&m_searchTaskHandler, &SearchTaskHandler::search,
@@ -369,18 +365,16 @@ HelpWidget *HelpPluginPrivate::createHelpWidget(const Context &context, HelpWidg
 {
     auto widget = new HelpWidget(context, style);
 
-    connect(widget, &HelpWidget::openHelpMode, this, [this](const QUrl &url) {
-        showHelpUrl(url, Core::HelpManager::HelpModeAlways);
-    });
+    connect(widget, &HelpWidget::requestShowHelpUrl, this, &HelpPluginPrivate::showHelpUrl);
     connect(LocalHelpManager::instance(),
             &LocalHelpManager::returnOnCloseChanged,
             widget,
             &HelpWidget::updateCloseButton);
     connect(widget, &HelpWidget::closeButtonClicked, this, [this, widget] {
-        if (widget->viewerCount() == 1 && LocalHelpManager::returnOnClose())
-            ModeManager::activateMode(Core::Constants::MODE_EDIT);
         if (widget->widgetStyle() == HelpWidget::SideBarWidget)
             RightPaneWidget::instance()->setShown(false);
+        else if (widget->viewerCount() == 1 && LocalHelpManager::returnOnClose())
+            ModeManager::activateMode(Core::Constants::MODE_EDIT);
     });
     connect(widget, &HelpWidget::aboutToClose,
             this, &HelpPluginPrivate::saveExternalWindowSettings);
@@ -464,17 +458,6 @@ void HelpPluginPrivate::modeChanged(Core::Id mode, Core::Id old)
     }
 }
 
-void HelpPluginPrivate::updateSideBarSource(const QUrl &newUrl)
-{
-    if (m_rightPaneSideBarWidget) {
-        // This is called when setSource on the central widget is called.
-        // Avoid nested setSource calls (even of different help viewers) by scheduling the
-        // sidebar viewer update on the event loop (QTCREATORBUG-12742)
-        QMetaObject::invokeMethod(m_rightPaneSideBarWidget->currentViewer(), "setSource",
-                                  Qt::QueuedConnection, Q_ARG(QUrl, newUrl));
-    }
-}
-
 void HelpPluginPrivate::setupHelpEngineIfNeeded()
 {
     LocalHelpManager::setEngineNeedsUpdate();
@@ -535,6 +518,7 @@ HelpViewer *HelpPluginPrivate::viewerForHelpViewerLocation(
 
     if (actualLocation == Core::HelpManager::SideBySideAlways) {
         createRightPaneContextViewer();
+        ModeManager::activateMode(Core::Constants::MODE_EDIT);
         RightPaneWidget::instance()->setWidget(m_rightPaneSideBarWidget);
         RightPaneWidget::instance()->setShown(true);
         return m_rightPaneSideBarWidget->currentViewer();

src/plugins/help/helpviewer.h

@@ -63,8 +63,7 @@ public:
     virtual QString title() const = 0;
 
     virtual QUrl source() const = 0;
-    // metacall in HelpPlugin::updateSideBarSource
-    Q_INVOKABLE virtual void setSource(const QUrl &url) = 0;
+    virtual void setSource(const QUrl &url) = 0;
 
     virtual void setHtml(const QString &html) = 0;
 

src/plugins/help/helpwidget.cpp

@@ -100,11 +100,6 @@ QVariant OpenPagesModel::data(const QModelIndex &index, int role) const
     return QVariant();
 }
 
-static void openUrlInWindow(const QUrl &url)
-{
-    HelpPlugin::showHelpUrl(url, Core::HelpManager::ExternalHelpAlways);
-}
-
 static bool isBookmarkable(const QUrl &url)
 {
     return !url.isEmpty() && url != QUrl(Help::Constants::AboutBlank);
@@ -207,7 +202,9 @@ HelpWidget::HelpWidget(const Core::Context &context, WidgetStyle style, QWidget
     if (style != ModeWidget) {
         m_switchToHelp = new QAction(tr("Open in Help Mode"), toolBar);
         cmd = Core::ActionManager::registerAction(m_switchToHelp, Constants::CONTEXT_HELP, context);
-        connect(m_switchToHelp, &QAction::triggered, this, &HelpWidget::helpModeButtonClicked);
+        connect(m_switchToHelp, &QAction::triggered, this, [this] {
+            postRequestShowHelpUrl(Core::HelpManager::HelpModeAlways);
+        });
         layout->addWidget(Core::Command::toolButtonWithAppendedShortcut(m_switchToHelp, cmd));
     }
 
@@ -336,6 +333,12 @@ HelpWidget::HelpWidget(const Core::Context &context, WidgetStyle style, QWidget
 
     if (m_switchToHelp)
         openMenu->addAction(m_switchToHelp);
+    if (style != SideBarWidget) {
+        QAction *openSideBySide = openMenu->addAction(tr("Open in Edit Mode"));
+        connect(openSideBySide, &QAction::triggered, this, [this]() {
+            postRequestShowHelpUrl(Core::HelpManager::SideBySideAlways);
+        });
+    }
     if (supportsPages()) {
         QAction *openPage = openMenu->addAction(tr("Open in New Page"));
         connect(openPage, &QAction::triggered, this, [this]() {
@@ -346,11 +349,7 @@ HelpWidget::HelpWidget(const Core::Context &context, WidgetStyle style, QWidget
     if (style != ExternalWindow) {
         QAction *openExternal = openMenu->addAction(tr("Open in Window"));
         connect(openExternal, &QAction::triggered, this, [this]() {
-            if (HelpViewer *viewer = currentViewer()) {
-                openUrlInWindow(viewer->source());
-                if (m_style == SideBarWidget)
-                    emit closeButtonClicked();
-            }
+            postRequestShowHelpUrl(Core::HelpManager::ExternalHelpAlways);
         });
     }
     openMenu->addSeparator();
@@ -555,7 +554,6 @@ void HelpWidget::setCurrentIndex(int index)
         LocalHelpManager::canOpenOnlineHelp(viewer->source()));
     if (m_style == ExternalWindow)
         updateWindowTitle();
-    emit sourceChanged(viewer->source());
     emit currentIndexChanged(index);
 }
 
@@ -576,7 +574,6 @@ HelpViewer *HelpWidget::insertViewer(int index, const QUrl &url, qreal zoom)
         if (currentViewer() == viewer) {
             m_addBookmarkAction->setEnabled(isBookmarkable(url));
             m_openOnlineDocumentationAction->setEnabled(LocalHelpManager::canOpenOnlineHelp(url));
-            emit sourceChanged(url);
         }
     });
     connect(viewer, &HelpViewer::forwardAvailable, this, [viewer, this](bool available) {
@@ -598,7 +595,9 @@ HelpViewer *HelpWidget::insertViewer(int index, const QUrl &url, qreal zoom)
 
     connect(viewer, &HelpViewer::loadFinished, this, &HelpWidget::highlightSearchTerms);
     connect(viewer, &HelpViewer::newPageRequested, this, &HelpWidget::openNewPage);
-    connect(viewer, &HelpViewer::externalPageRequested, this, &openUrlInWindow);
+    connect(viewer, &HelpViewer::externalPageRequested, this, [this](const QUrl &url) {
+        emit requestShowHelpUrl(url, Core::HelpManager::ExternalHelpAlways);
+    });
     updateCloseButton();
     m_model.endInsertRows();
     if (url.isValid())
@@ -731,12 +730,11 @@ void HelpWidget::updateWindowTitle()
         setWindowTitle(tr("Help - %1").arg(pageTitle));
 }
 
-void HelpWidget::helpModeButtonClicked()
+void HelpWidget::postRequestShowHelpUrl(Core::HelpManager::HelpViewerLocation location)
 {
     QTC_ASSERT(currentViewer(), return);
-    emit openHelpMode(currentViewer()->source());
-    if (m_style == ExternalWindow)
-        close();
+    emit requestShowHelpUrl(currentViewer()->source(), location);
+    closeWindow();
 }
 
 void HelpWidget::closeCurrentPage()
@@ -770,6 +768,14 @@ bool HelpWidget::supportsPages() const
     return m_style != SideBarWidget;
 }
 
+void HelpWidget::closeWindow()
+{
+    if (m_style == SideBarWidget)
+        emit closeButtonClicked();
+    else if (m_style == ExternalWindow)
+        close();
+}
+
 void HelpWidget::updateCloseButton()
 {
     if (supportsPages()) {

src/plugins/help/helpwidget.h

@@ -25,6 +25,7 @@
 
 #pragma once
 
+#include <coreplugin/helpmanager.h>
 #include <coreplugin/icontext.h>
 
 #include <QAbstractTableModel>
@@ -109,10 +110,9 @@ protected:
     void closeEvent(QCloseEvent *) override;
 
 signals:
-    void openHelpMode(const QUrl &url);
+    void requestShowHelpUrl(const QUrl &url, Core::HelpManager::HelpViewerLocation location);
     void closeButtonClicked();
     void aboutToClose();
-    void sourceChanged(const QUrl &url);
     void filterActivated(const QString &name);
     void currentIndexChanged(int index);
 
@@ -122,10 +122,11 @@ private:
     void updateBackMenu();
     void updateForwardMenu();
     void updateWindowTitle();
-    void helpModeButtonClicked();
+    void postRequestShowHelpUrl(Core::HelpManager::HelpViewerLocation location);
     void closeCurrentPage();
     void saveState() const;
     bool supportsPages() const;
+    void closeWindow();
 
     void goHome();
     void addBookmark();