Find: Fix wrong target of (replace) next/previous actions.

They were always triggered on the find support that had the find tool bar,
even if the focus moved to a different one and even if the find tool bar was
not visible.

Task-number: QTCREATORBUG-11587
Change-Id: Ica7ef6275dc8a7e0e8b974c2796651c9ca7809a6
Reviewed-by: Daniel Teske <daniel.teske@digia.com>

src/plugins/coreplugin/actionmanager/command.cpp

@@ -421,18 +421,22 @@ bool Action::hasAttribute(Command::CommandAttribute attr) const
 }
 
 
+void Command::augmentActionWithShortcutToolTip(QAction *a) const
+{
+    a->setToolTip(stringWithAppendedShortcut(a->text()));
+    QObject::connect(this, &Core::Command::keySequenceChanged, a, [this, a]() {
+        a->setToolTip(stringWithAppendedShortcut(a->text()));
+    });
+    QObject::connect(a, &QAction::changed, this, [this, a]() {
+        a->setToolTip(stringWithAppendedShortcut(a->text()));
+    });
+}
+
 QToolButton *Command::toolButtonWithAppendedShortcut(QAction *action, Command *cmd)
 {
     QToolButton *button = new QToolButton;
     button->setDefaultAction(action);
-    if (cmd) {
-        action->setToolTip(cmd->stringWithAppendedShortcut(action->text()));
-        QObject::connect(cmd, &Core::Command::keySequenceChanged, action, [cmd, action]() {
-            action->setToolTip(cmd->stringWithAppendedShortcut(action->text()));
-        });
-        QObject::connect(action, &QAction::changed, cmd, [cmd, action]() {
-            action->setToolTip(cmd->stringWithAppendedShortcut(action->text()));
-        });
-    }
+    if (cmd)
+        cmd->augmentActionWithShortcutToolTip(action);
     return button;
 }

src/plugins/coreplugin/actionmanager/command.h

@@ -88,6 +88,7 @@ public:
 
     virtual void setKeySequence(const QKeySequence &key) = 0;
     virtual QString stringWithAppendedShortcut(const QString &str) const = 0;
+    void augmentActionWithShortcutToolTip(QAction *action) const;
     static QToolButton *toolButtonWithAppendedShortcut(QAction *action, Command *cmd);
 
     virtual bool isScriptable() const = 0;

src/plugins/coreplugin/find/currentdocumentfind.cpp

@@ -72,9 +72,9 @@ bool CurrentDocumentFind::isEnabled() const
     return m_currentFind && (!m_currentWidget || m_currentWidget->isVisible());
 }
 
-bool CurrentDocumentFind::candidateIsEnabled() const
+IFindSupport *CurrentDocumentFind::candidate() const
 {
-    return (m_candidateFind != 0);
+    return m_candidateFind;
 }
 
 bool CurrentDocumentFind::supportsReplace() const

src/plugins/coreplugin/find/currentdocumentfind.h

@@ -53,7 +53,7 @@ public:
     QString completedFindString() const;
 
     bool isEnabled() const;
-    bool candidateIsEnabled() const;
+    IFindSupport *candidate() const;
     void highlightAll(const QString &txt, FindFlags findFlags);
     IFindSupport::Result findIncremental(const QString &txt, FindFlags findFlags);
     IFindSupport::Result findStep(const QString &txt, FindFlags findFlags);

src/plugins/coreplugin/find/findplugin.cpp

@@ -181,7 +181,6 @@ void FindPlugin::openFindFilter()
 
 void FindPlugin::openFindDialog(IFindFilter *filter)
 {
-    if (d->m_currentDocumentFind->candidateIsEnabled())
     d->m_currentDocumentFind->acceptCandidate();
     const QString currentFindString =
         d->m_currentDocumentFind->isEnabled() ?

src/plugins/coreplugin/find/findtoolbar.cpp

@@ -183,15 +183,21 @@ FindToolBar::FindToolBar(FindPlugin *plugin, CurrentDocumentFind *currentDocumen
     cmd = Core::ActionManager::registerAction(m_findNextAction, Constants::FIND_NEXT, globalcontext);
     cmd->setDefaultKeySequence(QKeySequence::FindNext);
     mfind->addAction(cmd, Constants::G_FIND_ACTIONS);
-    connect(m_findNextAction, SIGNAL(triggered()), this, SLOT(invokeFindNext()));
-    m_ui.findNextButton->setDefaultAction(cmd->action());
+    connect(m_findNextAction, SIGNAL(triggered()), this, SLOT(invokeGlobalFindNext()));
+    auto localFindNextAction = new QAction(m_findNextAction->text(), this);
+    cmd->augmentActionWithShortcutToolTip(localFindNextAction);
+    connect(localFindNextAction, &QAction::triggered, this, &FindToolBar::invokeFindNext);
+    m_ui.findNextButton->setDefaultAction(localFindNextAction);
 
     m_findPreviousAction = new QAction(tr("Find Previous"), this);
     cmd = Core::ActionManager::registerAction(m_findPreviousAction, Constants::FIND_PREVIOUS, globalcontext);
     cmd->setDefaultKeySequence(QKeySequence::FindPrevious);
     mfind->addAction(cmd, Constants::G_FIND_ACTIONS);
-    connect(m_findPreviousAction, SIGNAL(triggered()), this, SLOT(invokeFindPrevious()));
-    m_ui.findPreviousButton->setDefaultAction(cmd->action());
+    connect(m_findPreviousAction, SIGNAL(triggered()), this, SLOT(invokeGlobalFindPrevious()));
+    auto localFindPreviousAction = new QAction(m_findPreviousAction->text(), this);
+    cmd->augmentActionWithShortcutToolTip(localFindPreviousAction);
+    connect(localFindPreviousAction, &QAction::triggered, this, &FindToolBar::invokeFindPrevious);
+    m_ui.findPreviousButton->setDefaultAction(localFindPreviousAction);
 
     m_findNextSelectedAction = new QAction(tr("Find Next (Selected)"), this);
     cmd = Core::ActionManager::registerAction(m_findNextSelectedAction, Constants::FIND_NEXT_SELECTED, globalcontext);
@@ -209,29 +215,38 @@ FindToolBar::FindToolBar(FindPlugin *plugin, CurrentDocumentFind *currentDocumen
     cmd = Core::ActionManager::registerAction(m_replaceAction, Constants::REPLACE, globalcontext);
     cmd->setDefaultKeySequence(QKeySequence());
     mfind->addAction(cmd, Constants::G_FIND_ACTIONS);
-    connect(m_replaceAction, SIGNAL(triggered()), this, SLOT(invokeReplace()));
-    m_ui.replaceButton->setDefaultAction(cmd->action());
+    connect(m_replaceAction, SIGNAL(triggered()), this, SLOT(invokeGlobalReplace()));
+    auto localReplaceAction = new QAction(m_replaceAction->text(), this);
+    cmd->augmentActionWithShortcutToolTip(localReplaceAction);
+    connect(localReplaceAction, &QAction::triggered, this, &FindToolBar::invokeReplace);
+    m_ui.replaceButton->setDefaultAction(localReplaceAction);
 
     m_replaceNextAction = new QAction(tr("Replace && Find"), this);
     m_replaceNextAction->setIconText(tr("Replace && Find")); // work around bug in Qt that kills ampersands in tool button
     cmd = Core::ActionManager::registerAction(m_replaceNextAction, Constants::REPLACE_NEXT, globalcontext);
     cmd->setDefaultKeySequence(QKeySequence(tr("Ctrl+=")));
     mfind->addAction(cmd, Constants::G_FIND_ACTIONS);
-    connect(m_replaceNextAction, SIGNAL(triggered()), this, SLOT(invokeReplaceNext()));
-    m_ui.replaceNextButton->setDefaultAction(cmd->action());
+    connect(m_replaceNextAction, SIGNAL(triggered()), this, SLOT(invokeGlobalReplaceNext()));
+    auto localReplaceNextAction = new QAction(m_replaceNextAction->text(), this);
+    cmd->augmentActionWithShortcutToolTip(localReplaceNextAction);
+    connect(localReplaceNextAction, &QAction::triggered, this, &FindToolBar::invokeReplaceNext);
+    m_ui.replaceNextButton->setDefaultAction(localReplaceNextAction);
 
     m_replacePreviousAction = new QAction(tr("Replace && Find Previous"), this);
     cmd = Core::ActionManager::registerAction(m_replacePreviousAction, Constants::REPLACE_PREVIOUS, globalcontext);
     // shortcut removed, clashes with Ctrl++ on many keyboard layouts
     //cmd->setDefaultKeySequence(QKeySequence(tr("Ctrl+Shift+=")));
     mfind->addAction(cmd, Constants::G_FIND_ACTIONS);
-    connect(m_replacePreviousAction, SIGNAL(triggered()), this, SLOT(invokeReplacePrevious()));
+    connect(m_replacePreviousAction, SIGNAL(triggered()), this, SLOT(invokeGlobalReplacePrevious()));
 
     m_replaceAllAction = new QAction(tr("Replace All"), this);
     cmd = Core::ActionManager::registerAction(m_replaceAllAction, Constants::REPLACE_ALL, globalcontext);
     mfind->addAction(cmd, Constants::G_FIND_ACTIONS);
-    connect(m_replaceAllAction, SIGNAL(triggered()), this, SLOT(invokeReplaceAll()));
-    m_ui.replaceAllButton->setDefaultAction(cmd->action());
+    connect(m_replaceAllAction, SIGNAL(triggered()), this, SLOT(invokeGlobalReplaceAll()));
+    auto localReplaceAllAction = new QAction(m_replaceAllAction->text(), this);
+    cmd->augmentActionWithShortcutToolTip(localReplaceAllAction);
+    connect(localReplaceAllAction, &QAction::triggered, this, &FindToolBar::invokeReplaceAll);
+    m_ui.replaceAllButton->setDefaultAction(localReplaceAllAction);
 
     m_caseSensitiveAction = new QAction(tr("Case Sensitive"), this);
     m_caseSensitiveAction->setIcon(QIcon(QLatin1String(":/find/images/casesensitively.png")));
@@ -265,8 +280,13 @@ FindToolBar::FindToolBar(FindPlugin *plugin, CurrentDocumentFind *currentDocumen
     mfind->addAction(cmd, Constants::G_FIND_FLAGS);
     connect(m_preserveCaseAction, SIGNAL(toggled(bool)), this, SLOT(setPreserveCase(bool)));
 
-    connect(m_currentDocumentFind, SIGNAL(candidateChanged()), this, SLOT(adaptToCandidate()));
-    connect(m_currentDocumentFind, SIGNAL(changed()), this, SLOT(updateToolBar()));
+    connect(m_currentDocumentFind, &CurrentDocumentFind::candidateChanged,
+            this, &FindToolBar::adaptToCandidate);
+    connect(m_currentDocumentFind, &CurrentDocumentFind::changed,
+            this, &FindToolBar::updateGlobalActions);
+    connect(m_currentDocumentFind, &CurrentDocumentFind::changed,
+            this, &FindToolBar::updateToolBar);
+    updateGlobalActions();
     updateToolBar();
 
     m_findIncrementalTimer.setSingleShot(true);
@@ -335,7 +355,7 @@ bool FindToolBar::eventFilter(QObject *obj, QEvent *event)
 
 void FindToolBar::adaptToCandidate()
 {
-    updateFindAction();
+    updateGlobalActions();
     if (findToolBarPlaceHolder() == Core::FindToolBarPlaceHolder::getCurrent()) {
         m_currentDocumentFind->acceptCandidate();
         if (isVisible())
@@ -343,14 +363,22 @@ void FindToolBar::adaptToCandidate()
     }
 }
 
-void FindToolBar::updateFindAction()
+void FindToolBar::updateGlobalActions()
 {
-    bool enabled = m_currentDocumentFind->candidateIsEnabled();
-    m_findInDocumentAction->setEnabled((toolBarHasFocus() && isEnabled()) || enabled);
+    IFindSupport *candidate = m_currentDocumentFind->candidate();
+    bool enabled = (candidate != 0);
+    bool replaceEnabled = enabled && candidate->supportsReplace();
+    m_findInDocumentAction->setEnabled(enabled || (toolBarHasFocus() && isEnabled()));
     m_findNextSelectedAction->setEnabled(enabled);
     m_findPreviousSelectedAction->setEnabled(enabled);
     if (QApplication::clipboard()->supportsFindBuffer())
         m_enterFindStringAction->setEnabled(enabled);
+    m_findNextAction->setEnabled(enabled);
+    m_findPreviousAction->setEnabled(enabled);
+    m_replaceAction->setEnabled(replaceEnabled);
+    m_replaceNextAction->setEnabled(replaceEnabled);
+    m_replacePreviousAction->setEnabled(replaceEnabled);
+    m_replaceAllAction->setEnabled(replaceEnabled);
 }
 
 void FindToolBar::updateToolBar()
@@ -359,13 +387,12 @@ void FindToolBar::updateToolBar()
     bool replaceEnabled = enabled && m_currentDocumentFind->supportsReplace();
     bool showAllControls = canShowAllControls(replaceEnabled);
 
-    m_findNextAction->setEnabled(enabled);
-    m_findPreviousAction->setEnabled(enabled);
+    m_ui.findNextButton->setEnabled(enabled);
+    m_ui.findPreviousButton->setEnabled(enabled);
 
-    m_replaceAction->setEnabled(replaceEnabled);
-    m_replaceNextAction->setEnabled(replaceEnabled);
-    m_replacePreviousAction->setEnabled(replaceEnabled);
-    m_replaceAllAction->setEnabled(replaceEnabled);
+    m_ui.replaceButton->setEnabled(replaceEnabled);
+    m_ui.replaceNextButton->setEnabled(replaceEnabled);
+    m_ui.replaceAllButton->setEnabled(replaceEnabled);
 
     m_caseSensitiveAction->setEnabled(enabled);
     m_wholeWordAction->setEnabled(enabled);
@@ -428,12 +455,24 @@ void FindToolBar::invokeFindNext()
     invokeFindStep();
 }
 
+void FindToolBar::invokeGlobalFindNext()
+{
+    acceptCandidateAndMoveToolBar();
+    invokeFindNext();
+}
+
 void FindToolBar::invokeFindPrevious()
 {
     setFindFlag(FindBackward, true);
     invokeFindStep();
 }
 
+void FindToolBar::invokeGlobalFindPrevious()
+{
+    acceptCandidateAndMoveToolBar();
+    invokeFindPrevious();
+}
+
 QString FindToolBar::getFindText()
 {
     return m_ui.findEdit->text();
@@ -497,18 +536,36 @@ void FindToolBar::invokeReplace()
     }
 }
 
+void FindToolBar::invokeGlobalReplace()
+{
+    acceptCandidateAndMoveToolBar();
+    invokeReplace();
+}
+
 void FindToolBar::invokeReplaceNext()
 {
     setFindFlag(FindBackward, false);
     invokeReplaceStep();
 }
 
+void FindToolBar::invokeGlobalReplaceNext()
+{
+    acceptCandidateAndMoveToolBar();
+    invokeReplaceNext();
+}
+
 void FindToolBar::invokeReplacePrevious()
 {
     setFindFlag(FindBackward, true);
     invokeReplaceStep();
 }
 
+void FindToolBar::invokeGlobalReplacePrevious()
+{
+    acceptCandidateAndMoveToolBar();
+    invokeReplacePrevious();
+}
+
 void FindToolBar::invokeReplaceStep()
 {
     if (m_currentDocumentFind->isEnabled() && m_currentDocumentFind->supportsReplace()) {
@@ -526,6 +583,12 @@ void FindToolBar::invokeReplaceAll()
         m_currentDocumentFind->replaceAll(getFindText(), getReplaceText(), effectiveFindFlags());
 }
 
+void FindToolBar::invokeGlobalReplaceAll()
+{
+    acceptCandidateAndMoveToolBar();
+    invokeReplaceAll();
+}
+
 void FindToolBar::invokeResetIncrementalSearch()
 {
     m_findIncrementalTimer.stop();
@@ -674,6 +737,27 @@ bool FindToolBar::canShowAllControls(bool replaceIsVisible) const
     return fullWidth - replaceFixedWidth >= MINIMUM_WIDTH_FOR_COMPLEX_LAYOUT;
 }
 
+/*!
+    Accepts the candidate find of the current focus widget (if any), and moves the tool bar
+    there, if it was visible before.
+*/
+void FindToolBar::acceptCandidateAndMoveToolBar()
+{
+    if (!m_currentDocumentFind->candidate())
+        return;
+    if (isVisible()) {
+        openFindToolBar(UpdateHighlight);
+    } else {
+        // Make sure we are really hidden, and not just because our parent was hidden.
+        // Otherwise when the tool bar gets visible again, it will be in a different widget than
+        // the current document find it acts on.
+        // Test case: Open find in navigation side bar, hide side bar, click into editor,
+        // trigger find next, show side bar
+        hide();
+        m_currentDocumentFind->acceptCandidate();
+    }
+}
+
 void FindToolBar::openFind(bool focus)
 {
     setBackward(false);

src/plugins/coreplugin/find/findtoolbar.h

@@ -99,13 +99,19 @@ public slots:
 
 private slots:
     void invokeFindNext();
+    void invokeGlobalFindNext();
     void invokeFindPrevious();
+    void invokeGlobalFindPrevious();
     void invokeFindStep();
     void invokeReplace();
+    void invokeGlobalReplace();
     void invokeReplaceNext();
+    void invokeGlobalReplaceNext();
     void invokeReplacePrevious();
+    void invokeGlobalReplacePrevious();
     void invokeReplaceStep();
     void invokeReplaceAll();
+    void invokeGlobalReplaceAll();
     void invokeResetIncrementalSearch();
 
     void invokeFindIncremental();
@@ -118,7 +124,7 @@ private slots:
     void openFind(bool focus = true);
     void findNextSelected();
     void findPreviousSelected();
-    void updateFindAction();
+    void updateGlobalActions();
     void updateToolBar();
     void findFlagsChanged();
     void findEditButtonClicked();
@@ -145,6 +151,7 @@ private:
     Core::FindToolBarPlaceHolder *findToolBarPlaceHolder() const;
     bool toolBarHasFocus() const;
     bool canShowAllControls(bool replaceIsVisible) const;
+    void acceptCandidateAndMoveToolBar();
 
     bool eventFilter(QObject *obj, QEvent *event);
     void setFindText(const QString &text);