Skip to content

Commit

Permalink
Partial fix #540: stop emitting TabBar::webActionEnabledChanged from …
Browse files Browse the repository at this point in the history
…ZimView

as it's bad practice according to QObject documentation.
  • Loading branch information
asashnov committed Dec 29, 2021
1 parent 07e121e commit 5496ba5
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 44 deletions.
21 changes: 8 additions & 13 deletions src/kiwixapp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,8 @@ void KiwixApp::init()

createAction();
mp_mainWindow = new MainWindow;
mp_tabWidget = mp_mainWindow->getTabBar();
mp_tabWidget->setContentManagerView(mp_manager->getView());
mp_tabWidget->setNewTabButton();
getTabWidget()->setContentManagerView(mp_manager->getView());
getTabWidget()->setNewTabButton();
postInit();
mp_errorDialog = new QErrorMessage(mp_mainWindow);
setActivationWindow(mp_mainWindow);
Expand Down Expand Up @@ -184,14 +183,14 @@ void KiwixApp::openZimFile(const QString &zimfile)

void KiwixApp::printPage()
{
if(!mp_tabWidget->currentZimView())
if(!getTabWidget()->currentZimView())
return;
QPrinter* printer = new QPrinter();
QPrintDialog printDialog(printer, mp_mainWindow);
printDialog.setStyle(nullptr);
printDialog.setStyleSheet("");
if (printDialog.exec() == QDialog::Accepted) {
auto webview = mp_tabWidget->currentWebView();
auto webview = getTabWidget()->currentWebView();
if(!webview)
return;
webview->page()->print(printer, [=](bool success) {
Expand All @@ -208,12 +207,12 @@ void KiwixApp::openUrl(const QString &url, bool newTab) {
}

void KiwixApp::openUrl(const QUrl &url, bool newTab) {
mp_tabWidget->openUrl(url, newTab);
getTabWidget()->openUrl(url, newTab);
}

void KiwixApp::openRandomUrl(bool newTab)
{
auto zimId = mp_tabWidget->currentZimId();
auto zimId = getTabWidget()->currentZimId();
if (zimId.isEmpty()) {
return;
}
Expand Down Expand Up @@ -346,7 +345,7 @@ void KiwixApp::createAction()
CREATE_ACTION(FindInPageAction, gt("find-in-page"));
mpa_actions[FindInPageAction]->setShortcuts({QKeySequence::Find, Qt::Key_F3});
connect(mpa_actions[FindInPageAction], &QAction::triggered,
this, [=]() { mp_tabWidget->openFindInPageBar(); });
this, [=]() { getTabWidget()->openFindInPageBar(); });

CREATE_ACTION_ICON_SHORTCUT(ToggleFullscreenAction, "full-screen-enter", gt("set-fullscreen"), QKeySequence::FullScreen);
connect(mpa_actions[ToggleFullscreenAction], &QAction::toggled,
Expand Down Expand Up @@ -391,11 +390,7 @@ void KiwixApp::createAction()
}

void KiwixApp::postInit() {
connect(mp_tabWidget, &TabBar::webActionEnabledChanged,
mp_mainWindow->getTopWidget(), &TopWidget::handleWebActionEnabledChanged);
connect(mp_tabWidget, &TabBar::currentTitleChanged, this,
[=](const QString& title) { emit currentTitleChanged(title); });
connect(mp_tabWidget, &TabBar::libraryPageDisplayed,
connect(getTabWidget(), &TabBar::libraryPageDisplayed,
this, &KiwixApp::disableItemsOnLibraryPage);
emit(m_library.booksChanged());
connect(&m_library, &Library::booksChanged, this, &KiwixApp::updateNameMapper);
Expand Down
7 changes: 1 addition & 6 deletions src/kiwixapp.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include "kiwix/downloader.h"
#include <kiwix/kiwixserve.h>
#include "kprofile.h"
#include "urlschemehandler.h"
#include "settingsmanager.h"
#include "translation.h"

Expand Down Expand Up @@ -73,7 +72,7 @@ class KiwixApp : public QtSingleApplication
MainWindow* getMainWindow() { return mp_mainWindow; }
ContentManager* getContentManager() { return mp_manager; }
kiwix::Downloader* getDownloader() { return mp_downloader; }
TabBar* getTabWidget() { return mp_tabWidget; }
TabBar* getTabWidget() { return getMainWindow()->getTabBar(); }
QAction* getAction(Actions action);
QString getLibraryDirectory() { return m_libraryDirectory; };
kiwix::Server* getLocalServer() { return &m_server; }
Expand All @@ -82,9 +81,6 @@ class KiwixApp : public QtSingleApplication

bool isCurrentArticleBookmarked();

signals:
void currentTitleChanged(const QString& title);

public slots:
void openZimFile(const QString& zimfile="");
void openUrl(const QString& url, bool newTab=true);
Expand All @@ -106,7 +102,6 @@ public slots:
kiwix::Downloader* mp_downloader;
ContentManager* mp_manager;
MainWindow* mp_mainWindow;
TabBar* mp_tabWidget;
QErrorMessage* mp_errorDialog;
kiwix::UpdatableNameMapper m_nameMapper;
kiwix::Server m_server;
Expand Down
14 changes: 12 additions & 2 deletions src/mainwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ MainWindow::MainWindow(QWidget *parent) :
this, [=]() { QDesktopServices::openUrl(QUrl("https://donate.kiwix.org")); });
connect(app->getAction(KiwixApp::KiwixServeAction), &QAction::triggered,
mp_localKiwixServer, &QDialog::show);

connect(app, &KiwixApp::currentTitleChanged, this, [=](const QString& title) {
connect(mp_ui->tabBar, &TabBar::currentTitleChanged, this, [=](const QString& title) {
if (!title.isEmpty() && !title.startsWith("zim://"))
setWindowTitle(title + " - Kiwix");
else
Expand All @@ -64,6 +63,17 @@ MainWindow::MainWindow(QWidget *parent) :
connect(mp_ui->tabBar, &TabBar::libraryPageDisplayed,
this, &MainWindow::when_libraryPageDisplayed);

connect(mp_ui->tabBar, &TabBar::currentTitleChanged,
&(mp_ui->mainToolBar->getSearchBar()), &SearchBar::on_currentTitleChanged);

// This signal emited more often than the history really updated
// but for now we have no better signal for it.
connect(mp_ui->tabBar, &TabBar::currentTitleChanged,
mp_ui->mainToolBar, &TopWidget::updateBackForwardButtons);

connect(mp_ui->tabBar, &TabBar::webActionEnabledChanged,
mp_ui->mainToolBar, &TopWidget::handleWebActionEnabledChanged);

mp_ui->contentmanagerside->setContentManager(app->getContentManager());
mp_ui->sideBar->setCurrentWidget(mp_ui->contentmanagerside);
}
Expand Down
4 changes: 2 additions & 2 deletions src/searchbar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "kiwixapp.h"
#include "suggestionlistworker.h"
#include "tabbar.h"

SearchButton::SearchButton(QWidget *parent) :
QPushButton(parent),
Expand Down Expand Up @@ -80,8 +81,7 @@ SearchBar::SearchBar(QWidget *parent) :

qRegisterMetaType<QVector<QUrl>>("QVector<QUrl>");
connect(mp_typingTimer, &QTimer::timeout, this, &SearchBar::updateCompletion);
connect(KiwixApp::instance(), &KiwixApp::currentTitleChanged,
this, &SearchBar::on_currentTitleChanged);

connect(this, &QLineEdit::textEdited, this,
[=](const QString &text) {
m_searchbarInput = text;
Expand Down
17 changes: 17 additions & 0 deletions src/tabbar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ ZimView* TabBar::createNewTab(bool setCurrent, bool adjacentToCurrentTab)
if (setCurrent) {
setCurrentIndex(index);
}

connect(tab, &ZimView::webActionEnabledChanged,
this, &TabBar::when_zimview_history_action_changed);

return tab;
}

Expand Down Expand Up @@ -292,17 +296,20 @@ void TabBar::onCurrentChanged(int index)
QWidget *w = mp_stackedWidget->widget(index);

if (qobject_cast<SettingsView*>(w)) {
// Settings tab became active
emit webActionEnabledChanged(QWebEnginePage::Back, false);
emit webActionEnabledChanged(QWebEnginePage::Forward, false);
emit libraryPageDisplayed(false);
QTimer::singleShot(0, [=](){emit currentTitleChanged("");});
} else if (auto zv = qobject_cast<ZimView*>(w)) {
// Some of zim files tab became active
auto view = zv->getWebView();
emit webActionEnabledChanged(QWebEnginePage::Back, view->isWebActionEnabled(QWebEnginePage::Back));
emit webActionEnabledChanged(QWebEnginePage::Forward, view->isWebActionEnabled(QWebEnginePage::Forward));
emit libraryPageDisplayed(false);
QTimer::singleShot(0, [=](){emit currentTitleChanged(view->title());});
} else if (qobject_cast<ContentManagerView*>(w)) {
// the Library tab became active
emit webActionEnabledChanged(QWebEnginePage::Back, false);
emit webActionEnabledChanged(QWebEnginePage::Forward, false);
emit libraryPageDisplayed(true);
Expand Down Expand Up @@ -342,6 +349,16 @@ void TabBar::on_webview_titleChanged(const QString& title)
emit currentTitleChanged(title);
}

void TabBar::when_zimview_history_action_changed(QWebEnginePage::WebAction action, bool enabled)
{
ZimView *zv = qobject_cast<ZimView*>(sender());

if (!zv || zv != this->currentZimView())
return;

emit webActionEnabledChanged(action, enabled);
}

void TabBar::mousePressEvent(QMouseEvent *event)
{
if (event->button() == Qt::MiddleButton) {
Expand Down
5 changes: 3 additions & 2 deletions src/tabbar.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ class TabBar : public QTabBar
void currentTitleChanged(const QString& title);

public slots:
void closeTab(int index);
void onCurrentChanged(int index);
void fullScreenRequested(QWebEngineFullScreenRequest request);
void on_webview_titleChanged(const QString& title);

Expand All @@ -65,9 +63,12 @@ public slots:
QScopedPointer<FullScreenWindow> m_fullScreenWindow;

void setSelectionBehaviorOnRemove(int index);
void closeTab(int index);

private slots:
void onTabMoved(int from, int to);
void onCurrentChanged(int index);
void when_zimview_history_action_changed(QWebEnginePage::WebAction action, bool enabled);
};

#endif // TABWIDGET_H
5 changes: 0 additions & 5 deletions src/topwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@ TopWidget::TopWidget(QWidget *parent) :
addAction(menuAction);
setContextMenuPolicy( Qt::PreventContextMenu );

// This signal emited more often than the history really updated
// but for now we have no better signal for it.
connect(KiwixApp::instance(), &KiwixApp::currentTitleChanged,
this, &TopWidget::updateBackForwardButtons);

#if !SYSTEMTITLEBAR
addAction(QIcon(":/icons/minimize.svg"), "minimize", parent, SLOT(showMinimized()));
#endif
Expand Down
4 changes: 1 addition & 3 deletions src/webview.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ class WebView : public QWebEngineView
QMenu* getHistoryBackMenu() const;
QMenu* getHistoryForwardMenu() const;

public slots:
void onUrlChanged(const QUrl& url);

signals:
void iconChanged(const QIcon& icon);
void zimIdChanged(const QString& zimId);
Expand All @@ -65,6 +62,7 @@ public slots:
QString m_linkHovered;

private slots:
void onUrlChanged(const QUrl& url);
void gotoTriggeredHistoryItemAction();

private:
Expand Down
14 changes: 4 additions & 10 deletions src/zimview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,12 @@ ZimView::ZimView(TabBar *tabBar, QWidget *parent)
[=](const QIcon& icon) { mp_tabBar->setIconOf(icon, this); });

connect(mp_webView->page()->action(QWebEnginePage::Back), &QAction::changed,
[=]() {
if (mp_tabBar->currentZimView() != this) {
return;
}
emit mp_tabBar->webActionEnabledChanged(QWebEnginePage::Back, mp_webView->isWebActionEnabled(QWebEnginePage::Back));
[this]() {
emit webActionEnabledChanged(QWebEnginePage::Back, this->mp_webView->isWebActionEnabled(QWebEnginePage::Back));
});
connect(mp_webView->page()->action(QWebEnginePage::Forward), &QAction::changed,
[=]() {
if (mp_tabBar->currentZimView() != this) {
return;
}
emit mp_tabBar->webActionEnabledChanged(QWebEnginePage::Forward, mp_webView->isWebActionEnabled(QWebEnginePage::Forward));
[this]() {
emit webActionEnabledChanged(QWebEnginePage::Forward, this->mp_webView->isWebActionEnabled(QWebEnginePage::Forward));
});
connect(mp_webView->page(), &QWebEnginePage::linkHovered, this,
[=](const QString& url) {
Expand Down
6 changes: 5 additions & 1 deletion src/zimview.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
#define ZIMVIEW_H

#include <QWidget>
#include "findinpagebar.h"
#include <QWebEnginePage>

class FindInPageBar;
class TabBar;
class WebView;

Expand All @@ -17,6 +18,9 @@ class ZimView : public QWidget
FindInPageBar *getFindInPageBar() { return mp_findInPageBar; }
void openFindInPageBar();

signals:
void webActionEnabledChanged(QWebEnginePage::WebAction action, bool enabled);

private:
WebView *mp_webView;
TabBar *mp_tabBar;
Expand Down

0 comments on commit 5496ba5

Please sign in to comment.