working on unifications of context menus in web viewers

localization/rssguard_en.ts

@@ -5859,6 +5859,21 @@ List of supported readers:</source>
         <source>not a Sitemap</source>
         <translation type="unfinished"></translation>
     </message>
+    <message>
+        <location filename="../src/librssguard/gui/webviewers/webviewer.cpp" line="34"/>
+        <source>Open with external tool</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <location filename="../src/librssguard/gui/webviewers/webviewer.cpp" line="95"/>
+        <source>Open in external browser</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <location filename="../src/librssguard/gui/webviewers/webviewer.cpp" line="98"/>
+        <source>Open link as audio/video</source>
+        <translation type="unfinished"></translation>
+    </message>
 </context>
 <context>
     <name>Readability</name>
@@ -8026,25 +8041,15 @@ Unread news: %2</translation>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="../src/librssguard/gui/webviewers/qtextbrowser/textbrowserviewer.cpp" line="370"/>
+        <location filename="../src/librssguard/gui/webviewers/qtextbrowser/textbrowserviewer.cpp" line="375"/>
         <source>Enable external resources</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="../src/librssguard/gui/webviewers/qtextbrowser/textbrowserviewer.cpp" line="373"/>
+        <location filename="../src/librssguard/gui/webviewers/qtextbrowser/textbrowserviewer.cpp" line="377"/>
-        <source>Open in external browser</source>
-        <translation type="unfinished"></translation>
-    </message>
-    <message>
-        <location filename="../src/librssguard/gui/webviewers/qtextbrowser/textbrowserviewer.cpp" line="375"/>
         <source>Download</source>
         <translation type="unfinished"></translation>
     </message>
-    <message>
-        <location filename="../src/librssguard/gui/webviewers/qtextbrowser/textbrowserviewer.cpp" line="400"/>
-        <source>Open with external tool</source>
-        <translation type="unfinished"></translation>
-    </message>
 </context>
 <context>
     <name>TimeSpinBox</name>
@@ -8529,29 +8534,6 @@ Last login on: %4</source>
         <translation type="unfinished"></translation>
     </message>
 </context>
-<context>
-    <name>WebEngineViewer</name>
-    <message>
-        <location filename="../src/librssguard/gui/webviewers/webengine/webengineviewer.cpp" line="92"/>
-        <source>Open link in external browser</source>
-        <translation type="unfinished"></translation>
-    </message>
-    <message>
-        <location filename="../src/librssguard/gui/webviewers/webengine/webengineviewer.cpp" line="105"/>
-        <source>Open link as audio/video</source>
-        <translation type="unfinished"></translation>
-    </message>
-    <message>
-        <location filename="../src/librssguard/gui/webviewers/webengine/webengineviewer.cpp" line="115"/>
-        <source>Open with external tool</source>
-        <translation type="unfinished"></translation>
-    </message>
-    <message>
-        <location filename="../src/librssguard/gui/webviewers/webengine/webengineviewer.cpp" line="134"/>
-        <source>No external tools activated</source>
-        <translation type="unfinished"></translation>
-    </message>
-</context>
 <context>
     <name>WebFactory</name>
     <message>

src/librssguard/CMakeLists.txt

@@ -205,6 +205,7 @@ set(SOURCES
   gui/toolbars/toolbareditor.h
   gui/webbrowser.cpp
   gui/webbrowser.h
+  gui/webviewers/webviewer.cpp
   gui/webviewers/webviewer.h
   miscellaneous/application.cpp
   miscellaneous/application.h

src/librssguard/gui/webviewers/qtextbrowser/textbrowserviewer.cpp

@@ -365,69 +365,33 @@ void TextBrowserViewer::contextMenuEvent(QContextMenuEvent* event) {
     return;
   }
 
+  /*
+  connect(menu, &QMenu::aboutToHide, this, [menu] {
+    menu->deleteLater();
+  });*/
+
   if (m_actionEnableResources.isNull()) {
     m_actionEnableResources.reset(new QAction(qApp->icons()->fromTheme(QSL("viewimage"), QSL("image-x-generic")),
                                               tr("Enable external resources"),
                                               this));
-    m_actionOpenExternalBrowser.reset(new QAction(qApp->icons()->fromTheme(QSL("document-open")),
-                                                  tr("Open in external browser"),
-                                                  this));
     m_actionDownloadLink.reset(new QAction(qApp->icons()->fromTheme(QSL("download")), tr("Download"), this));
 
     m_actionEnableResources.data()->setCheckable(true);
     m_actionEnableResources.data()->setChecked(resourcesEnabled());
 
-    connect(m_actionOpenExternalBrowser.data(),
-            &QAction::triggered,
-            this,
-            &TextBrowserViewer::openLinkInExternalBrowser);
     connect(m_actionDownloadLink.data(), &QAction::triggered, this, &TextBrowserViewer::downloadLink);
     connect(m_actionEnableResources.data(), &QAction::toggled, this, &TextBrowserViewer::enableResources);
   }
 
   menu->addAction(m_actionEnableResources.data());
-  menu->addAction(m_actionOpenExternalBrowser.data());
   menu->addAction(m_actionDownloadLink.data());
 
   auto anchor = anchorAt(event->pos());
 
   m_lastContextMenuPos = event->pos();
-  m_actionOpenExternalBrowser.data()->setEnabled(!anchor.isEmpty());
   m_actionDownloadLink.data()->setEnabled(!anchor.isEmpty());
 
-  if (!anchor.isEmpty()) {
+  processContextMenu(menu, event);
-    QFileIconProvider icon_provider;
-    QMenu* menu_ext_tools = new QMenu(tr("Open with external tool"), menu);
-    auto tools = ExternalTool::toolsFromSettings();
-
-    menu_ext_tools->setIcon(qApp->icons()->fromTheme(QSL("document-open")));
-
-    for (const ExternalTool& tool : std::as_const(tools)) {
-      QAction* act_tool = new QAction(QFileInfo(tool.executable()).fileName(), menu_ext_tools);
-
-      act_tool->setIcon(icon_provider.icon(QFileInfo(tool.executable())));
-      act_tool->setToolTip(tool.executable());
-      act_tool->setData(QVariant::fromValue(tool));
-      menu_ext_tools->addAction(act_tool);
-
-      connect(act_tool, &QAction::triggered, this, [act_tool, anchor]() {
-        act_tool->data().value<ExternalTool>().run(anchor);
-      });
-    }
-
-    if (menu_ext_tools->actions().isEmpty()) {
-      QAction* act_not_tools = new QAction("No external tools activated");
-
-      act_not_tools->setEnabled(false);
-      menu_ext_tools->addAction(act_not_tools);
-    }
-
-    menu->addMenu(menu_ext_tools);
-  }
-
-  connect(menu, &QMenu::aboutToHide, this, [menu] {
-    menu->deleteLater();
-  });
 
   menu->popup(event->globalPos());
 }
@@ -449,24 +413,6 @@ void TextBrowserViewer::enableResources(bool enable) {
   setResourcesEnabled(enable);
 }
 
-void TextBrowserViewer::openLinkInExternalBrowser() {
-  auto url = QUrl(anchorAt(m_lastContextMenuPos));
-
-  if (url.isValid()) {
-    const QUrl resolved_url = (m_currentUrl.isValid() && url.isRelative()) ? m_currentUrl.resolved(url) : url;
-
-    qApp->web()->openUrlInExternalBrowser(resolved_url.toString());
-
-    if (qApp->settings()
-          ->value(GROUP(Messages), SETTING(Messages::BringAppToFrontAfterMessageOpenedExternally))
-          .toBool()) {
-      QTimer::singleShot(1000, qApp, []() {
-        qApp->mainForm()->display();
-      });
-    }
-  }
-}
-
 void TextBrowserViewer::downloadLink() {
   auto url = QUrl(anchorAt(m_lastContextMenuPos));
 
@@ -644,3 +590,15 @@ bool TextBrowserViewer::resourcesEnabled() const {
 void TextBrowserViewer::setResourcesEnabled(bool enabled) {
   m_resourcesEnabled = enabled;
 }
+
+ContextMenuData TextBrowserViewer::provideContextMenuData(QContextMenuEvent* event) const {
+  ContextMenuData c;
+
+  QString anchor = anchorAt(event->pos());
+
+  if (!anchor.isEmpty()) {
+    c.m_linkUrl = anchor;
+  }
+
+  return c;
+}

src/librssguard/gui/webviewers/qtextbrowser/textbrowserviewer.h

@@ -63,13 +63,14 @@ class TextBrowserViewer : public QTextBrowser, public WebViewer {
     void setResourcesEnabled(bool enabled);
 
   protected:
+    virtual ContextMenuData provideContextMenuData(QContextMenuEvent* event) const;
+
     virtual void contextMenuEvent(QContextMenuEvent* event);
     virtual void resizeEvent(QResizeEvent* event);
     virtual void wheelEvent(QWheelEvent* event);
 
   private slots:
     void enableResources(bool enable);
-    void openLinkInExternalBrowser();
     void downloadLink();
     void onAnchorClicked(const QUrl& url);
     void reloadHtmlDelayed();
@@ -112,7 +113,6 @@ class TextBrowserViewer : public QTextBrowser, public WebViewer {
     QFont m_baseFont;
     qreal m_zoomFactor = 1.0;
     QScopedPointer<QAction> m_actionEnableResources;
-    QScopedPointer<QAction> m_actionOpenExternalBrowser;
     QScopedPointer<QAction> m_actionDownloadLink;
     QScopedPointer<TextBrowserDocument> m_document;
     QPoint m_lastContextMenuPos;

src/librssguard/gui/webviewers/webengine/webengineviewer.cpp

@@ -78,74 +78,18 @@ void WebEngineViewer::contextMenuEvent(QContextMenuEvent* event) {
 
 #if QT_VERSION_MAJOR == 6
   QMenu* menu = createStandardContextMenu();
-  auto* menu_pointer = lastContextMenuRequest();
-  QWebEngineContextMenuRequest& menu_data = *menu_pointer;
 #else
   QMenu* menu = page()->createStandardContextMenu();
-  QWebEngineContextMenuData menu_data = page()->contextMenuData();
 #endif
 
-  if (menu_data.linkUrl().isValid()) {
-    QString link_url = menu_data.linkUrl().toString();
-
-    // Add option to open link in external viewe
-    menu->addAction(qApp->icons()->fromTheme(QSL("document-open")), tr("Open link in external browser"), [link_url]() {
-      qApp->web()->openUrlInExternalBrowser(link_url);
-
-      if (qApp->settings()
-            ->value(GROUP(Messages), SETTING(Messages::BringAppToFrontAfterMessageOpenedExternally))
-            .toBool()) {
-        QTimer::singleShot(1000, qApp, []() {
-          qApp->mainForm()->display();
-        });
-      }
-    });
-
-    menu->addAction(qApp->icons()->fromTheme(QSL("player_play"), QSL("media-playback-start")),
-                    tr("Open link as audio/video"),
-                    [link_url]() {
-                      qApp->mainForm()->tabWidget()->addMediaPlayer(link_url, true);
-                    });
-  }
-
-  if (menu_data.mediaUrl().isValid() || menu_data.linkUrl().isValid()) {
-    QString media_link =
-      menu_data.mediaUrl().isValid() ? menu_data.mediaUrl().toString() : menu_data.linkUrl().toString();
-    QFileIconProvider icon_provider;
-    QMenu* menu_ext_tools = new QMenu(tr("Open with external tool"), menu);
-    auto tools = ExternalTool::toolsFromSettings();
-
-    menu_ext_tools->setIcon(qApp->icons()->fromTheme(QSL("document-open")));
-
-    for (const ExternalTool& tool : std::as_const(tools)) {
-      QAction* act_tool = new QAction(QFileInfo(tool.executable()).fileName(), menu_ext_tools);
-
-      act_tool->setIcon(icon_provider.icon(QFileInfo(tool.executable())));
-      act_tool->setToolTip(tool.executable());
-      act_tool->setData(QVariant::fromValue(tool));
-      menu_ext_tools->addAction(act_tool);
-
-      connect(act_tool, &QAction::triggered, this, [this, act_tool, media_link]() {
-        openUrlWithExternalTool(act_tool->data().value<ExternalTool>(), media_link);
-      });
-    }
-
-    if (menu_ext_tools->actions().isEmpty()) {
-      QAction* act_not_tools = new QAction(tr("No external tools activated"));
-
-      act_not_tools->setEnabled(false);
-      menu_ext_tools->addAction(act_not_tools);
-    }
-
-    menu->addMenu(menu_ext_tools);
-  }
-
   menu->addAction(qApp->web()->adBlock()->adBlockIcon());
   menu->addAction(qApp->web()->engineSettingsAction());
 
   const QPoint pos = event->globalPos();
   QPoint p(pos.x(), pos.y() + 1);
 
+  processContextMenu(menu, event);
+
   menu->popup(p);
 }
 
@@ -160,10 +104,6 @@ void WebEngineViewer::openUrlWithExternalTool(ExternalTool tool, const QString&
   tool.run(target_url);
 }
 
-RootItem* WebEngineViewer::root() const {
-  return m_root;
-}
-
 void WebEngineViewer::bindToBrowser(WebBrowser* browser) {
   m_browser = browser;
 
@@ -258,3 +198,24 @@ QString WebEngineViewer::html() const {
 QUrl WebEngineViewer::url() const {
   return QWebEngineView::url();
 }
+
+ContextMenuData WebEngineViewer::provideContextMenuData(QContextMenuEvent* event) const {
+#if QT_VERSION_MAJOR == 6
+  auto* menu_pointer = lastContextMenuRequest();
+  QWebEngineContextMenuRequest& menu_data = *menu_pointer;
+#else
+  QWebEngineContextMenuData menu_data = page()->contextMenuData();
+#endif
+
+  ContextMenuData c;
+
+  if (menu_data.mediaUrl().isValid()) {
+    c.m_mediaUrl = menu_data.linkUrl();
+  }
+
+  if (menu_data.linkUrl().isValid()) {
+    c.m_linkUrl = menu_data.linkUrl();
+  }
+
+  return c;
+}

src/librssguard/gui/webviewers/webengine/webengineviewer.h

@@ -21,8 +21,6 @@ class WebEngineViewer : public QWebEngineView, public WebViewer {
   public:
     explicit WebEngineViewer(QWidget* parent = nullptr);
 
-    RootItem* root() const;
-
   public:
     virtual void loadMessages(const QList<Message>& messages, RootItem* root);
     virtual void bindToBrowser(WebBrowser* browser);
@@ -51,6 +49,8 @@ class WebEngineViewer : public QWebEngineView, public WebViewer {
     void closeWindowRequested();
 
   protected:
+    virtual ContextMenuData provideContextMenuData(QContextMenuEvent* event) const;
+
     virtual QWebEngineView* createWindow(QWebEnginePage::WebWindowType type);
     virtual void contextMenuEvent(QContextMenuEvent* event);
     virtual bool event(QEvent* event);

src/librssguard/gui/webviewers/webviewer.cpp

@@ -0,0 +1,110 @@
+// For license of this file, see <project-root-folder>/LICENSE.md.
+
+#include "gui/webviewers/webviewer.h"
+
+#include "gui/dialogs/formmain.h"
+#include "miscellaneous/externaltool.h"
+#include "miscellaneous/iconfactory.h"
+#include "miscellaneous/settings.h"
+#include "network-web/webfactory.h"
+
+#include <QFileIconProvider>
+#include <QTimer>
+
+WebViewer::WebViewer() {}
+
+WebViewer::~WebViewer() {}
+
+void WebViewer::processContextMenu(QMenu* specific_menu, QContextMenuEvent* event) {
+  // Setup the menu.
+  m_contextMenuData = provideContextMenuData(event);
+  specific_menu->setAttribute(Qt::WidgetAttribute::WA_DeleteOnClose, true);
+  initializeCommonMenuItems();
+
+  // Add common items.
+  specific_menu->addSeparator();
+  specific_menu->addAction(m_actionOpenExternalBrowser.data());
+  specific_menu->addAction(m_actionPlayLink.data());
+
+  m_actionOpenExternalBrowser.data()->setEnabled(m_contextMenuData.m_linkUrl.isValid());
+  m_actionPlayLink.data()->setEnabled(m_contextMenuData.m_linkUrl.isValid());
+
+  if (m_contextMenuData.m_linkUrl.isValid()) {
+    QFileIconProvider icon_provider;
+    QMenu* menu_ext_tools = new QMenu(QObject::tr("Open with external tool"), specific_menu);
+    auto tools = ExternalTool::toolsFromSettings();
+
+    menu_ext_tools->setIcon(qApp->icons()->fromTheme(QSL("document-open")));
+
+    for (const ExternalTool& tool : std::as_const(tools)) {
+      QAction* act_tool = new QAction(QFileInfo(tool.executable()).fileName(), menu_ext_tools);
+
+      act_tool->setIcon(icon_provider.icon(QFileInfo(tool.executable())));
+      act_tool->setToolTip(tool.executable());
+      act_tool->setData(QVariant::fromValue(tool));
+      menu_ext_tools->addAction(act_tool);
+
+      QObject::connect(act_tool, &QAction::triggered, act_tool, [this, act_tool]() {
+        act_tool->data().value<ExternalTool>().run(m_contextMenuData.m_linkUrl.toString());
+      });
+    }
+
+    if (menu_ext_tools->actions().isEmpty()) {
+      QAction* act_not_tools = new QAction("No external tools activated");
+
+      act_not_tools->setEnabled(false);
+      menu_ext_tools->addAction(act_not_tools);
+    }
+
+    specific_menu->addMenu(menu_ext_tools);
+  }
+}
+
+void WebViewer::playClickedLinkAsMedia() {
+  auto context_url = m_contextMenuData.m_linkUrl;
+
+  if (context_url.isValid()) {
+    qApp->mainForm()->tabWidget()->addMediaPlayer(context_url.toString(), true);
+  }
+}
+
+void WebViewer::openClickedLinkInExternalBrowser() {
+  auto context_url = m_contextMenuData.m_linkUrl;
+
+  if (context_url.isValid()) {
+    const QUrl resolved_url = (url().isValid() && context_url.isRelative()) ? url().resolved(context_url) : context_url;
+
+    qApp->web()->openUrlInExternalBrowser(resolved_url.toString());
+
+    if (qApp->settings()
+          ->value(GROUP(Messages), SETTING(Messages::BringAppToFrontAfterMessageOpenedExternally))
+          .toBool()) {
+      QTimer::singleShot(1000, qApp, []() {
+        qApp->mainForm()->display();
+      });
+    }
+  }
+}
+
+void WebViewer::initializeCommonMenuItems() {
+  if (!m_actionOpenExternalBrowser.isNull()) {
+    return;
+  }
+
+  m_actionOpenExternalBrowser.reset(new QAction(qApp->icons()->fromTheme(QSL("document-open")),
+                                                QObject::tr("Open in external browser")));
+
+  m_actionPlayLink.reset(new QAction(qApp->icons()->fromTheme(QSL("player_play"), QSL("media-playback-start")),
+                                     QObject::tr("Open link as audio/video")));
+
+  QObject::connect(m_actionOpenExternalBrowser.data(),
+                   &QAction::triggered,
+                   m_actionOpenExternalBrowser.data(),
+                   [this]() {
+                     openClickedLinkInExternalBrowser();
+                   });
+
+  QObject::connect(m_actionPlayLink.data(), &QAction::triggered, m_actionPlayLink.data(), [this]() {
+    playClickedLinkAsMedia();
+  });
+}

src/librssguard/gui/webviewers/webviewer.h

@@ -1,3 +1,5 @@
+// For license of this file, see <project-root-folder>/LICENSE.md.
+
 #ifndef WEBVIEWER_H
 #define WEBVIEWER_H
 
@@ -5,8 +7,12 @@
 
 #include "definitions/definitions.h"
 
+#include <QAction>
 #include <QUrl>
 
+class QMenu;
+class QContextMenuEvent;
+
 class WebBrowser;
 class RootItem;
 
@@ -15,9 +21,15 @@ struct PreparedHtml {
     QUrl m_baseUrl;
 };
 
+struct ContextMenuData {
+    QUrl m_linkUrl;
+    QUrl m_mediaUrl;
+};
+
 // Interface for web/article viewers.
 class WebViewer {
   public:
+    WebViewer();
     virtual ~WebViewer();
 
     // Performs necessary steps to make viewer work with browser.
@@ -68,6 +80,11 @@ class WebViewer {
     virtual qreal zoomFactor() const = 0;
     virtual void setZoomFactor(qreal zoom_factor) = 0;
 
+  protected:
+    void processContextMenu(QMenu* specific_menu, QContextMenuEvent* event);
+
+    virtual ContextMenuData provideContextMenuData(QContextMenuEvent* event) const = 0;
+
   signals:
     virtual void pageTitleChanged(const QString& new_title) = 0;
     virtual void pageUrlChanged(const QUrl& url) = 0;
@@ -78,12 +95,20 @@ class WebViewer {
     virtual void loadingFinished(bool success) = 0;
     virtual void newWindowRequested(WebViewer* viewer) = 0;
     virtual void closeWindowRequested() = 0;
+
+  private:
+    void playClickedLinkAsMedia();
+    void openClickedLinkInExternalBrowser();
+    void initializeCommonMenuItems();
+
+  private:
+    QScopedPointer<QAction> m_actionOpenExternalBrowser;
+    QScopedPointer<QAction> m_actionPlayLink;
+    ContextMenuData m_contextMenuData;
 };
 
 Q_DECLARE_INTERFACE(WebViewer, "WebViewer")
 
-inline WebViewer::~WebViewer() {}
-
 inline void WebViewer::zoomIn() {
   setZoomFactor(zoomFactor() + double(ZOOM_FACTOR_STEP));
 }