From ab83c930ef2698a47b0c36a79b5b5988a60a6ee3 Mon Sep 17 00:00:00 2001 From: Alexander Sashnov Date: Thu, 7 Jan 2021 03:32:44 +0700 Subject: [PATCH 1/4] gitignore: ignore .user (QtCreator local user settings, not portable) --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 259148f..c544c38 100644 --- a/.gitignore +++ b/.gitignore @@ -30,3 +30,6 @@ *.exe *.out *.app + +# QtCreator Environment Settings (not suitable for sharing) +kiwix-desktop.pro.user From f576df02fdcb9718b0821f2fdc8946381ce1dcf2 Mon Sep 17 00:00:00 2001 From: Alexander Sashnov Date: Sat, 13 Feb 2021 02:38:52 +0700 Subject: [PATCH 2/4] be clear about which pointer we get (instead of 'auto') --- src/searchbar.cpp | 4 ++-- src/suggestionlistworker.cpp | 6 +++--- src/tabbar.cpp | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/searchbar.cpp b/src/searchbar.cpp index e7d53dc..f7f76e3 100644 --- a/src/searchbar.cpp +++ b/src/searchbar.cpp @@ -155,8 +155,8 @@ void SearchBar::updateCompletion() { mp_typingTimer->stop(); clearSuggestions(); - auto currentWidget = KiwixApp::instance()->getTabWidget()->currentWebView(); - if (!currentWidget || currentWidget->url().isEmpty() || m_searchbarInput.isEmpty()) { + WebView* current = KiwixApp::instance()->getTabWidget()->currentWebView(); + if (!current || current->url().isEmpty() || m_searchbarInput.isEmpty()) { hideSuggestions(); return; } diff --git a/src/suggestionlistworker.cpp b/src/suggestionlistworker.cpp index 98aa846..770a29a 100644 --- a/src/suggestionlistworker.cpp +++ b/src/suggestionlistworker.cpp @@ -13,10 +13,10 @@ void SuggestionListWorker::run() QStringList suggestionList; QVector urlList; - auto currentWidget = KiwixApp::instance()->getTabWidget()->currentWebView(); - if(!currentWidget) + WebView *current = KiwixApp::instance()->getTabWidget()->currentWebView(); + if(!current) return; - auto qurl = currentWidget->url(); + auto qurl = current->url(); auto currentZimId = qurl.host().split(".")[0]; auto reader = KiwixApp::instance()->getLibrary()->getReader(currentZimId); QUrl url; diff --git a/src/tabbar.cpp b/src/tabbar.cpp index fde6280..8904522 100644 --- a/src/tabbar.cpp +++ b/src/tabbar.cpp @@ -55,9 +55,9 @@ TabBar::TabBar(QWidget *parent) : setCurrentIndex(m_settingsIndex); return; } - auto index = currentIndex() + 1; + int index = currentIndex() + 1; m_settingsIndex = index; - auto view = KiwixApp::instance()->getSettingsManager()->getView(); + SettingsManagerView* view = KiwixApp::instance()->getSettingsManager()->getView(); mp_stackedWidget->insertWidget(index, view); insertTab(index,QIcon(":/icons/settings.svg"), gt("settings")); QToolButton *tb = new QToolButton(this); From 639f8dd5f5bb97a91f9633fadab0fea71ef39c6e Mon Sep 17 00:00:00 2001 From: Alexander Sashnov Date: Sat, 13 Feb 2021 02:47:26 +0700 Subject: [PATCH 3/4] Illiminate Tabbar::widget method; Fix potential segfault when settings tab is open and closing by zim ID --- src/contentmanager.cpp | 10 +--------- src/tabbar.cpp | 17 +++++++++++++++-- src/tabbar.h | 2 +- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index 874cd76..9b9116f 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -250,15 +250,7 @@ void ContentManager::eraseBookFilesFromComputer(const QString dirPath, const QSt void ContentManager::eraseBook(const QString& id) { auto tabBar = KiwixApp::instance()->getTabWidget(); - int i = 1; - while (i < tabBar->count() - 1) { - WebView* webView = tabBar->widget(i)->getWebView(); - if (webView->zimId() == id) { - tabBar->closeTab(i); - } else { - i++; - } - } + tabBar->closeTabsByZimId(id); kiwix::Book book = mp_library->getBookById(id); QString dirPath = QString::fromStdString(removeLastPathElement(book.getPath())); QString filename = QString::fromStdString(getLastPathElement(book.getPath())) + "*"; diff --git a/src/tabbar.cpp b/src/tabbar.cpp index 8904522..c0d905c 100644 --- a/src/tabbar.cpp +++ b/src/tabbar.cpp @@ -200,6 +200,19 @@ void TabBar::triggerWebPageAction(QWebEnginePage::WebAction action, ZimView *wid widget->getWebView()->setFocus(); } +void TabBar::closeTabsByZimId(const QString &id) +{ + // the last tab is + button, skip it + for (int i = count() - 2 ; i >= 0 ; i--) { + auto *zv = qobject_cast(mp_stackedWidget->widget(i)); + if (!zv) + continue; + if (zv->getWebView()->zimId() == id) { + closeTab(i); + } + } +} + void TabBar::closeTab(int index) { setSelectionBehaviorOnRemove(index); @@ -211,7 +224,7 @@ void TabBar::closeTab(int index) if (index < m_settingsIndex) { m_settingsIndex--; } - auto webview = widget(index); + auto webview = mp_stackedWidget->widget(index); mp_stackedWidget->removeWidget(webview); webview->setParent(nullptr); removeTab(index); @@ -239,7 +252,7 @@ void TabBar::onCurrentChanged(int index) KiwixApp::instance()->setSideBar(KiwixApp::NONE); QTimer::singleShot(0, [=](){emit currentTitleChanged("");}); } else if (index) { - auto view = widget(index)->getWebView(); + auto view = static_cast(mp_stackedWidget->widget(index))->getWebView(); emit webActionEnabledChanged(QWebEnginePage::Back, view->isWebActionEnabled(QWebEnginePage::Back)); emit webActionEnabledChanged(QWebEnginePage::Forward, view->isWebActionEnabled(QWebEnginePage::Forward)); emit libraryPageDisplayed(false); diff --git a/src/tabbar.h b/src/tabbar.h index 6c8efa4..722a8eb 100644 --- a/src/tabbar.h +++ b/src/tabbar.h @@ -23,7 +23,6 @@ public: void setContentManagerView(ContentManagerView* view); void setNewTabButton(); ZimView* createNewTab(bool setCurrent); - ZimView* widget(int index) { return (index != 0) ? static_cast(mp_stackedWidget->widget(index)) : nullptr; } WebView* currentWebView() { auto current = mp_stackedWidget->currentWidget(); if (mp_stackedWidget->currentIndex() == 0 || mp_stackedWidget->currentIndex() == m_settingsIndex) return nullptr; @@ -46,6 +45,7 @@ public: QString currentArticleTitle(); virtual QSize tabSizeHint(int index) const; void openFindInPageBar(); + void closeTabsByZimId(const QString &id); protected: void mousePressEvent(QMouseEvent *event); From 6fe7e7487cf83a10febe35f823608bfb7282dd60 Mon Sep 17 00:00:00 2001 From: Alexander Sashnov Date: Mon, 15 Feb 2021 14:59:54 +0700 Subject: [PATCH 4/4] Fixes #454 - Allow to move tabs * the Library tab isn't necessary the first tab; * moving the Library tab is possible, but prohibited in TabBar::onTabMoved() * Tabbar uses qobject_cast<> to recognize tab types instead of previously used settings and library (always 0) tab indexes. --- src/contentmanagerview.h | 1 + src/kiwixapp.cpp | 2 +- src/settingsmanagerview.h | 1 + src/tabbar.cpp | 158 ++++++++++++++++++++++++++------------ src/tabbar.h | 24 +++--- src/zimview.cpp | 14 +++- 6 files changed, 132 insertions(+), 68 deletions(-) diff --git a/src/contentmanagerview.h b/src/contentmanagerview.h index d2241e3..b7e551c 100644 --- a/src/contentmanagerview.h +++ b/src/contentmanagerview.h @@ -6,6 +6,7 @@ class ContentManagerView : public QWebEngineView { + Q_OBJECT public: ContentManagerView(QWidget *parent = Q_NULLPTR); void registerObject(const QString &id, QObject *object); diff --git a/src/kiwixapp.cpp b/src/kiwixapp.cpp index 1558ed6..7838eac 100644 --- a/src/kiwixapp.cpp +++ b/src/kiwixapp.cpp @@ -180,7 +180,7 @@ void KiwixApp::openZimFile(const QString &zimfile) void KiwixApp::printPage() { - if(!mp_tabWidget->currentWidget()) + if(!mp_tabWidget->currentZimView()) return; QPrinter* printer = new QPrinter(); QPrintDialog printDialog(printer, mp_mainWindow); diff --git a/src/settingsmanagerview.h b/src/settingsmanagerview.h index aec1e00..de5dfc0 100644 --- a/src/settingsmanagerview.h +++ b/src/settingsmanagerview.h @@ -6,6 +6,7 @@ class SettingsManagerView : public QWebEngineView { + Q_OBJECT public: SettingsManagerView(QWidget *parent = nullptr); void registerObject(const QString &id, QObject *object); diff --git a/src/tabbar.cpp b/src/tabbar.cpp index c0d905c..f9af3c0 100644 --- a/src/tabbar.cpp +++ b/src/tabbar.cpp @@ -10,20 +10,19 @@ #include #define QUITIFNULL(VIEW) if (nullptr==(VIEW)) { return; } -#define QUITIFNOTCURRENT(VIEW) if((VIEW)!=currentWidget()) {return;} -#define CURRENTIFNULL(VIEW) if(nullptr==VIEW) { VIEW = currentWidget();} +#define CURRENTIFNULL(VIEW) if(nullptr==VIEW) { VIEW = currentZimView();} TabBar::TabBar(QWidget *parent) : - QTabBar(parent), - m_settingsIndex(-1) + QTabBar(parent) { QTabBar::setDrawBase(false); setTabsClosable(true); setElideMode(Qt::ElideNone); setDocumentMode(true); setFocusPolicy(Qt::NoFocus); + setMovable(true); setIconSize(QSize(30, 30)); - connect(this, &QTabBar::currentChanged, this, &TabBar::onCurrentChanged); + connect(this, &QTabBar::currentChanged, this, &TabBar::onCurrentChanged, Qt::QueuedConnection); auto app = KiwixApp::instance(); connect(app->getAction(KiwixApp::NewTabAction), &QAction::triggered, @@ -38,7 +37,12 @@ TabBar::TabBar(QWidget *parent) : connect(app->getAction(KiwixApp::CloseTabAction), &QAction::triggered, this, [=]() { auto index = this->tabAt(mapFromGlobal(QCursor::pos())); - if (index <= 0) { + if (index < 0) + return; + + // library tab cannot be closed + QWidget *w = mp_stackedWidget->widget(index); + if (qobject_cast(w)) { return; } this->closeTab(index); @@ -51,12 +55,13 @@ TabBar::TabBar(QWidget *parent) : }); connect(app->getAction(KiwixApp::SettingAction), &QAction::triggered, this, [=]() { - if (KiwixApp::instance()->getSettingsManager()->isSettingsViewdisplayed()) { - setCurrentIndex(m_settingsIndex); - return; + for (int i = 0 ; i < (mp_stackedWidget->count() - 1) ; i++) { + if (qobject_cast(mp_stackedWidget->widget(i))) { + setCurrentIndex(i); + return; + } } int index = currentIndex() + 1; - m_settingsIndex = index; SettingsManagerView* view = KiwixApp::instance()->getSettingsManager()->getView(); mp_stackedWidget->insertWidget(index, view); insertTab(index,QIcon(":/icons/settings.svg"), gt("settings")); @@ -65,6 +70,10 @@ TabBar::TabBar(QWidget *parent) : setTabButton(index, QTabBar::RightSide, tb); setCurrentIndex(index); }); + + // the slot relies the connection will be direct to reverting back the tab + connect(this, SIGNAL(tabMoved(int,int)), + this, SLOT(onTabMoved(int,int)), Qt::DirectConnection); } void TabBar::setStackedWidget(QStackedWidget *widget) { @@ -76,11 +85,10 @@ void TabBar::setStackedWidget(QStackedWidget *widget) { void TabBar::setContentManagerView(ContentManagerView* view) { qInfo() << "add widget"; - mp_contentManagerView = view; - mp_stackedWidget->addWidget(mp_contentManagerView); + mp_stackedWidget->addWidget(view); mp_stackedWidget->show(); - addTab(QIcon(":/icons/library-icon.svg"), ""); - setTabButton(0, RightSide, nullptr); + int idx = addTab(QIcon(":/icons/library-icon.svg"), ""); + setTabButton(idx, RightSide, nullptr); } void TabBar::setNewTabButton() @@ -88,19 +96,19 @@ void TabBar::setNewTabButton() QToolButton *tb = new QToolButton(); tb->setDefaultAction(KiwixApp::instance()->getAction(KiwixApp::NewTabAction)); tb->setIcon(QIcon(":/icons/new-tab-icon.svg")); - addTab(""); - setTabEnabled(1, false); - setTabButton(1, QTabBar::LeftSide, tb); - tabButton(1, QTabBar::RightSide)->deleteLater(); - setTabButton(1, QTabBar::RightSide, 0); + int idx = addTab(""); + setTabEnabled(idx, false); + setTabButton(idx, QTabBar::LeftSide, tb); + tabButton(idx, QTabBar::RightSide)->deleteLater(); + setTabButton(idx, QTabBar::RightSide, Q_NULLPTR); } ZimView* TabBar::createNewTab(bool setCurrent) { auto tab = new ZimView(this, this); - auto index = count() - 1; + int index = count() - 1; // the last tab is + button, insert before mp_stackedWidget->insertWidget(index, tab); - insertTab(index, ""); + index = insertTab(index, ""); QToolButton *tb = new QToolButton(this); tb->setDefaultAction(KiwixApp::instance()->getAction(KiwixApp::CloseTabAction)); setTabButton(index, QTabBar::RightSide, tb); @@ -162,34 +170,39 @@ void TabBar::setIconOf(const QIcon &icon, ZimView *tab) QString TabBar::currentZimId() { - if (!currentWidget()) - return ""; - return currentWebView()->zimId(); + if (WebView *w = currentWebView()) + return w->zimId(); + return ""; } QString TabBar::currentArticleUrl() { - if(!currentWidget()) - return ""; - return currentWebView()->url().path(); + if (WebView *w = currentWebView()) + return w->url().path(); + return ""; } QString TabBar::currentArticleTitle() { - if(!currentWidget()) - return ""; - return currentWebView()->title(); + if (WebView *w = currentWebView()) + return w->title(); + return ""; } -QSize TabBar::tabSizeHint(int index) const { - if (index) - return QSize(205, 40); - return QSize(40, 40); +QSize TabBar::tabSizeHint(int index) const +{ + QWidget *w = mp_stackedWidget->widget(index); + + if (w && qobject_cast(w)) + return QSize(40, 40); // the "Library" tab is only icon + + return QSize(205, 40); // "Settings" and content tabs have text } void TabBar::openFindInPageBar() { - currentWidget()->openFindInPageBar(); + if (ZimView *zv = currentZimView()) + zv->openFindInPageBar(); } void TabBar::triggerWebPageAction(QWebEnginePage::WebAction action, ZimView *widget) @@ -215,21 +228,24 @@ void TabBar::closeTabsByZimId(const QString &id) void TabBar::closeTab(int index) { - setSelectionBehaviorOnRemove(index); - if (index == 0 || index == this->count() - 1) + // the last tab is + button, cannot be closed + if (index == this->count() - 1) + return; + + setSelectionBehaviorOnRemove(index); + + QWidget *view = mp_stackedWidget->widget(index); + + // library tab cannot be closed + if (qobject_cast(view)) { return; - if (index == m_settingsIndex) { - m_settingsIndex = -1; } - if (index < m_settingsIndex) { - m_settingsIndex--; - } - auto webview = mp_stackedWidget->widget(index); - mp_stackedWidget->removeWidget(webview); - webview->setParent(nullptr); + + mp_stackedWidget->removeWidget(view); + view->setParent(nullptr); removeTab(index); - webview->close(); - delete webview; + view->close(); + view->deleteLater(); } void TabBar::setSelectionBehaviorOnRemove(int index) @@ -245,14 +261,23 @@ void TabBar::onCurrentChanged(int index) { if (index == -1) return; - if (index == m_settingsIndex) { + + // if somehow the last tab (+ button) became active, switch to the previous + if (index >= (count() - 1)) { + setCurrentIndex(count() - 2); + return; + } + + QWidget *w = mp_stackedWidget->widget(index); + + if (qobject_cast(w)) { emit webActionEnabledChanged(QWebEnginePage::Back, false); emit webActionEnabledChanged(QWebEnginePage::Forward, false); emit libraryPageDisplayed(false); KiwixApp::instance()->setSideBar(KiwixApp::NONE); QTimer::singleShot(0, [=](){emit currentTitleChanged("");}); - } else if (index) { - auto view = static_cast(mp_stackedWidget->widget(index))->getWebView(); + } else if (auto zv = qobject_cast(w)) { + auto view = zv->getWebView(); emit webActionEnabledChanged(QWebEnginePage::Back, view->isWebActionEnabled(QWebEnginePage::Back)); emit webActionEnabledChanged(QWebEnginePage::Forward, view->isWebActionEnabled(QWebEnginePage::Forward)); emit libraryPageDisplayed(false); @@ -260,13 +285,18 @@ void TabBar::onCurrentChanged(int index) KiwixApp::instance()->setSideBar(KiwixApp::NONE); } QTimer::singleShot(0, [=](){emit currentTitleChanged(view->title());}); - } else { + } else if (qobject_cast(w)) { emit webActionEnabledChanged(QWebEnginePage::Back, false); emit webActionEnabledChanged(QWebEnginePage::Forward, false); emit libraryPageDisplayed(true); KiwixApp::instance()->setSideBar(KiwixApp::CONTENTMANAGER_BAR); QTimer::singleShot(0, [=](){emit currentTitleChanged("");}); } + else { + Q_ASSERT(false); + // In the future, other types of tabs can be added. + // For example, About dialog, or Kiwix Server control panel. + } } void TabBar::fullScreenRequested(QWebEngineFullScreenRequest request) @@ -364,3 +394,29 @@ void TabBar::paintEvent(QPaintEvent *e) p.fillRect(tail, br); } } + +void TabBar::onTabMoved(int from, int to) +{ + // avoid infinitive recursion + static bool reverting = false; + if (reverting) + return; + + // on attempt to move the last tab (+ button) just move it back + // and the library should stick the first (zero) tab + int last = mp_stackedWidget->count(); + if ( + (from == last || to == last) || + (from == 0 || to == 0) + ) { + reverting = true; + moveTab(to, from); + reverting = false; + return; + } + + // swap widgets + QWidget *w_from = mp_stackedWidget->widget(from); + mp_stackedWidget->removeWidget(w_from); + mp_stackedWidget->insertWidget(to, w_from); +} diff --git a/src/tabbar.h b/src/tabbar.h index 722a8eb..e19d489 100644 --- a/src/tabbar.h +++ b/src/tabbar.h @@ -23,16 +23,16 @@ public: void setContentManagerView(ContentManagerView* view); void setNewTabButton(); ZimView* createNewTab(bool setCurrent); - WebView* currentWebView() { auto current = mp_stackedWidget->currentWidget(); - if (mp_stackedWidget->currentIndex() == 0 || - mp_stackedWidget->currentIndex() == m_settingsIndex) return nullptr; - return static_cast(current)->getWebView(); - } - ZimView* currentWidget() { auto current = mp_stackedWidget->currentWidget(); - if (mp_stackedWidget->currentIndex() == 0 || - mp_stackedWidget->currentIndex() == m_settingsIndex) return nullptr; - return static_cast(current); - } + + ZimView* currentZimView() { + return qobject_cast(mp_stackedWidget->currentWidget()); + } + + WebView* currentWebView() { + if (ZimView *zv = currentZimView()) + return zv->getWebView(); + return nullptr; + } void openUrl(const QUrl &url, bool newTab); // Redirect call to sub-webView @@ -63,13 +63,13 @@ public slots: void fullScreenRequested(QWebEngineFullScreenRequest request); private: - ContentManagerView* mp_contentManagerView; QStackedWidget* mp_stackedWidget; - int m_settingsIndex; QScopedPointer m_fullScreenWindow; void setSelectionBehaviorOnRemove(int index); +private slots: + void onTabMoved(int from, int to); }; #endif // TABWIDGET_H diff --git a/src/zimview.cpp b/src/zimview.cpp index 6103c02..ebe7b26 100644 --- a/src/zimview.cpp +++ b/src/zimview.cpp @@ -47,7 +47,7 @@ ZimView::ZimView(TabBar *tabBar, QWidget *parent) connect(mp_webView, &WebView::titleChanged, this, [=](const QString& str) { mp_tabBar->setTitleOf(str, this); - if (mp_tabBar->currentWidget() != this) { + if (mp_tabBar->currentZimView() != this) { return; } emit mp_tabBar->currentTitleChanged(str); @@ -56,21 +56,21 @@ ZimView::ZimView(TabBar *tabBar, QWidget *parent) [=](const QIcon& icon) { mp_tabBar->setIconOf(icon, this); }); connect(mp_webView, &WebView::zimIdChanged, this, [=](const QString& zimId) { - if (mp_tabBar->currentWidget() != this) { + if (mp_tabBar->currentZimView() != this) { return; } emit mp_tabBar->currentZimIdChanged(zimId); }); connect(mp_webView->page()->action(QWebEnginePage::Back), &QAction::changed, [=]() { - if (mp_tabBar->currentWidget() != this) { + if (mp_tabBar->currentZimView() != this) { return; } emit mp_tabBar->webActionEnabledChanged(QWebEnginePage::Back, mp_webView->isWebActionEnabled(QWebEnginePage::Back)); }); connect(mp_webView->page()->action(QWebEnginePage::Forward), &QAction::changed, [=]() { - if (mp_tabBar->currentWidget() != this) { + if (mp_tabBar->currentZimView() != this) { return; } emit mp_tabBar->webActionEnabledChanged(QWebEnginePage::Forward, mp_webView->isWebActionEnabled(QWebEnginePage::Forward)); @@ -87,7 +87,13 @@ ZimView::ZimView(TabBar *tabBar, QWidget *parent) if (url.startsWith("zim://")) { link = QUrl(url).path(); } + + /* because we use QWebEnginePage::linkHovered signal, + * we can be sure the current tab is a web page + * (mp_tabBar->currentWebView() is not nullptr) + */ auto height = mp_tabBar->currentWebView()->height() + 1; + auto pos = mp_tabBar->mapToGlobal(QPoint(-3, height)); QToolTip::showText(pos, link); }