From c1ad1be5924eb28bf6c32b049098fe24bbbb9e4e Mon Sep 17 00:00:00 2001 From: Aqua-sama Date: Tue, 19 Dec 2017 10:14:22 +0100 Subject: Bug fixes Switching between tabs should work properly now. Refactored MainWindowTabBar (was WebViewTabBar) --- BUGS | 3 + docs/manual/Contributing.Style.md | 6 +- linux/makepkg/PKGBUILD | 4 +- smolbote.qbs | 4 +- src/browser.cpp | 6 +- src/browser.h | 6 +- src/mainwindow.cpp | 21 ++--- src/mainwindow.h | 7 +- src/widgets/mainwindowtabbar.cpp | 164 ++++++++++++++++++++++++++++++++++++++ src/widgets/mainwindowtabbar.h | 67 ++++++++++++++++ src/widgets/webviewtabbar.cpp | 159 ------------------------------------ src/widgets/webviewtabbar.h | 67 ---------------- 12 files changed, 264 insertions(+), 250 deletions(-) create mode 100644 BUGS create mode 100644 src/widgets/mainwindowtabbar.cpp create mode 100644 src/widgets/mainwindowtabbar.h delete mode 100644 src/widgets/webviewtabbar.cpp delete mode 100644 src/widgets/webviewtabbar.h diff --git a/BUGS b/BUGS new file mode 100644 index 0000000..1a75136 --- /dev/null +++ b/BUGS @@ -0,0 +1,3 @@ +Switching between tabs with keyboard shortcuts doesn't always work. +Closing sometimes causes the program to crash (much mystery). +MainWindow doesn't save size on close diff --git a/docs/manual/Contributing.Style.md b/docs/manual/Contributing.Style.md index 4d7ef1b..7f3a052 100644 --- a/docs/manual/Contributing.Style.md +++ b/docs/manual/Contributing.Style.md @@ -4,7 +4,6 @@ * Use generic formats * Where possible, use QVector over QList: http://lists.qt-project.org/pipermail/development/2017-March /029040.html -* Check pointers with Q_CHECK_PTR before returning them ## Versioning @@ -17,6 +16,11 @@ * next - next stable release preparation, mostly for bugfixes * development - development branch, anything goes there +## Qt +* Check pointers with ´´´Q_CHECK_PTR´´´ before returning them +* Avoid using connect SIGNAL and SLOT. Instead use &Class::method. This way, +connects are checked during the compile, not at runtime. + ## clazy You can use [clazy](https://github.com/KDE/clazy) to check Qt semantics. Requires clang. diff --git a/linux/makepkg/PKGBUILD b/linux/makepkg/PKGBUILD index b506347..9c47f91 100644 --- a/linux/makepkg/PKGBUILD +++ b/linux/makepkg/PKGBUILD @@ -41,11 +41,11 @@ prepare() { build() { cd smolbote - qbs build --settings-dir ../config -d ../build profile:qt release + qbs build --settings-dir ../config -d ../build -p poi profile:qt release } package() { cd smolbote - qbs install --settings-dir ../config -d ../build --install-root "${pkgdir}${_iroot}" profile:qt release + qbs install --settings-dir ../config -d ../build -p poi --install-root "${pkgdir}${_iroot}" profile:qt release } diff --git a/smolbote.qbs b/smolbote.qbs index d74dffe..c3af913 100644 --- a/smolbote.qbs +++ b/smolbote.qbs @@ -109,8 +109,8 @@ Project { "src/widgets/loadingbar.h", "src/widgets/mainwindowmenubar.cpp", "src/widgets/mainwindowmenubar.h", - "src/widgets/webviewtabbar.cpp", - "src/widgets/webviewtabbar.h", + "src/widgets/mainwindowtabbar.cpp", + "src/widgets/mainwindowtabbar.h", ] } diff --git a/src/browser.cpp b/src/browser.cpp index ab244fd..db535f3 100644 --- a/src/browser.cpp +++ b/src/browser.cpp @@ -70,13 +70,13 @@ void Browser::loadProfiles() const QStringList profileList = profileDir.entryList(QDir::Dirs | QDir::NoDotAndDotDot); for(const QString &name : profileList) { - WebEngineProfile *profile = new WebEngineProfile(name, profileDir.absoluteFilePath(name), this); + std::shared_ptr profile = std::make_shared(name, profileDir.absoluteFilePath(name), this); profile->setRequestInterceptor(m_urlRequestInterceptor.get()); m_profiles.insert(name, profile); } // Also add the Off-the-record profile - WebEngineProfile *otr = new WebEngineProfile(this); + std::shared_ptr otr = std::make_shared(this); otr->setRequestInterceptor(m_urlRequestInterceptor.get()); m_profiles.insert("", otr); @@ -142,7 +142,7 @@ MainWindow *Browser::createWindow() return window; } -WebEngineProfile* Browser::profile(const QString name) +std::shared_ptr Browser::profile(const QString name) { return m_profiles[name]; } diff --git a/src/browser.h b/src/browser.h index a1557b4..89931e8 100644 --- a/src/browser.h +++ b/src/browser.h @@ -42,7 +42,7 @@ public: void setConfiguration(std::shared_ptr &config); void loadProfiles(); - WebEngineProfile *profile(const QString name); + std::shared_ptr profile(const QString name); // QStringList profiles(); // std::shared_ptr& bookmarks(); @@ -60,8 +60,8 @@ private: std::shared_ptr m_config; QVector m_windows; - QHash m_profiles; - WebEngineProfile* m_defaultProfile; + QHash> m_profiles; + std::shared_ptr m_defaultProfile; std::shared_ptr m_urlRequestInterceptor; std::shared_ptr m_bookmarksManager; diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 3dbcbdf..4433948 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -44,7 +44,7 @@ MainWindow::MainWindow(std::shared_ptr config, QWidget *parent) : QMainWindow(parent), ui(new Ui::MainWindow), - tabBar(new WebViewTabBar(config, nullptr, this)), + tabBar(new MainWindowTabBar(config, this)), menuBar(new MainWindowMenuBar(config, this)), m_addressBar(new UrlLineEdit(this)), m_progressBar(new LoadingBar(this)) @@ -88,7 +88,7 @@ MainWindow::MainWindow(std::shared_ptr config, QWidget *parent) : QToolButton *homepageButton = new QToolButton(this); homepageButton->setIcon(style()->standardIcon(QStyle::SP_DirHomeIcon)); connect(homepageButton, &QToolButton::clicked, this, [&]() { - tabBar->currentView()->load(tabBar->profile()->homepage()); + tabBar->currentView()->load(m_profile->homepage()); }); ui->navigationToolBar->addWidget(m_backButton); @@ -181,14 +181,14 @@ void MainWindow::newTab(const QUrl &url) m_tabBarAdded = true; ui->mainToolBar->addWidget(tabBar); } - tabBar->addTab(url); + tabBar->addTab(createWebView(url, m_profile.get())); } void MainWindow::newWindow(const QUrl &url) { Browser *instance = static_cast(qApp->instance()); MainWindow *window = instance->createWindow(); - window->setProfile(tabBar->profile()); + window->setProfile(m_profile); window->newTab(url); } @@ -221,17 +221,18 @@ void MainWindow::showSettingsDialog() dlg->exec(); } -void MainWindow::setProfile(WebEngineProfile *profile) +void MainWindow::setProfile(std::shared_ptr profile) { - Q_CHECK_PTR(profile); - tabBar->setProfile(profile); + Q_ASSERT(profile); + m_profile = profile; + tabBar->setProfile(profile.get()); menuBar->setProfileName(profile->name()); } WebEngineProfile *MainWindow::profile() { - Q_CHECK_PTR(tabBar->profile()); - return tabBar->profile(); + Q_ASSERT(m_profile); + return m_profile.get(); } void MainWindow::setBookmarksWidget(std::shared_ptr &widget) @@ -307,7 +308,7 @@ void MainWindow::handleTitleUpdated(const QString &title) { QString t = QString::fromStdString(m_config->value("browser.window.title").value()); t.replace("title", title); - t.replace("profile", tabBar->profile()->name()); + t.replace("profile", m_profile->name()); setWindowTitle(t); //setWindowTitle(browser->settings()->value("window.title").toString().replace("title", title).replace("profile", tabBar->profile()->name())); } diff --git a/src/mainwindow.h b/src/mainwindow.h index 1062d64..1852053 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -24,7 +24,7 @@ #include #include "webengine/webengineprofile.h" #include -#include "widgets/webviewtabbar.h" +#include "widgets/mainwindowtabbar.h" #include "widgets/loadingbar.h" #include "navigation/navigationbutton.h" @@ -59,7 +59,7 @@ public slots: void newTab(const QUrl &url = QUrl("")); void newWindow(const QUrl &url = QUrl("")); - void setProfile(WebEngineProfile *profile); + void setProfile(std::shared_ptr profile); WebEngineProfile *profile(); void setBookmarksWidget(std::shared_ptr &widget); @@ -79,7 +79,7 @@ private: Q_DISABLE_COPY(MainWindow) Ui::MainWindow *ui; - WebViewTabBar *tabBar; + MainWindowTabBar *tabBar; WebView *m_currentView; MainWindowMenuBar *menuBar; @@ -90,6 +90,7 @@ private: LoadingBar *m_progressBar; bool m_tabBarAdded = false; + std::shared_ptr m_profile; std::shared_ptr m_config; std::shared_ptr m_bookmarksWidget; std::shared_ptr m_downloadsWidget; diff --git a/src/widgets/mainwindowtabbar.cpp b/src/widgets/mainwindowtabbar.cpp new file mode 100644 index 0000000..cae42df --- /dev/null +++ b/src/widgets/mainwindowtabbar.cpp @@ -0,0 +1,164 @@ +/******************************************************************************* + ** + ** smolbote: yet another qute browser + ** Copyright (C) 2017 Xian Nox + ** + ** This program is free software: you can redistribute it and/or modify + ** it under the terms of the GNU General Public License as published by + ** the Free Software Foundation, either version 3 of the License, or + ** (at your option) any later version. + ** + ** This program is distributed in the hope that it will be useful, + ** but WITHOUT ANY WARRANTY; without even the implied warranty of + ** MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + ** GNU General Public License for more details. + ** + ** You should have received a copy of the GNU General Public License + ** along with this program. If not, see . + ** + ******************************************************************************/ + +#include "mainwindowtabbar.h" +#include +#include +#include +#include +#include + +#include "mainwindow.h" + +MainWindowTabBar::MainWindowTabBar(const std::shared_ptr &config, MainWindow *parent) : + QTabBar(parent) +{ + Q_CHECK_PTR(parent); + m_parent = parent; + + setElideMode(Qt::ElideRight); + setTabsClosable(true); + setMovable(true); + setContextMenuPolicy(Qt::DefaultContextMenu); + + connect(this, &MainWindowTabBar::tabCloseRequested, this, &MainWindowTabBar::removeTab); + connect(this, &MainWindowTabBar::currentChanged, this, &MainWindowTabBar::handleCurrentChanged); + connect(this, &MainWindowTabBar::tabMoved, this, &MainWindowTabBar::updateVectorArrangement); + + QShortcut *tabCloseShortcut = new QShortcut(parent); + tabCloseShortcut->setKey(QKeySequence(QString::fromStdString(config->value("browser.shortcuts.tabClose").value()))); + connect(tabCloseShortcut, &QShortcut::activated, [this]() { + this->removeTab(currentIndex()); + }); + + QShortcut *tabLeftShortcut = new QShortcut(parent); + tabLeftShortcut->setKey(QKeySequence(QString::fromStdString(config->value("browser.shortcuts.tabLeft").value()))); + connect(tabLeftShortcut, &QShortcut::activated, [this]() { + this->setCurrentIndex(currentIndex()-1); + }); + + QShortcut *tabRightShortcut = new QShortcut(parent); + tabRightShortcut->setKey(QKeySequence(QString::fromStdString(config->value("browser.shortcuts.tabRight").value()))); + connect(tabRightShortcut, &QShortcut::activated, [this]() { + this->setCurrentIndex(currentIndex()+1); + }); +} + +MainWindowTabBar::~MainWindowTabBar() +{ + // cleanup + qDeleteAll(m_views); + m_views.clear(); +} + +int MainWindowTabBar::addTab(WebView *view) +{ + m_views.append(view); + + connect(view, &QWebEngineView::titleChanged, [this, view](const QString &title) { + int index = m_views.indexOf(view); + setTabText(index, title); + }); + connect(view, &QWebEngineView::iconChanged, [this, view](const QIcon &icon) { + int index = m_views.indexOf(view); + setTabIcon(index, icon); + }); + + return QTabBar::addTab("New Tab"); +} + +void MainWindowTabBar::setProfile(WebEngineProfile *profile) +{ + Q_CHECK_PTR(profile); + + for(auto view : qAsConst(m_views)) { + QWebEnginePage *page = new QWebEnginePage(profile); + page->load(view->url()); + view->setPage(page); + } +} + +WebView *MainWindowTabBar::currentView() +{ + return m_views.at(currentIndex()); +} + +void MainWindowTabBar::contextMenuEvent(QContextMenuEvent *event) +{ + int tabIndex = tabAt(event->pos()); + if(tabIndex < 0) { + return; + } + + QMenu menu(this); + QAction* closeAction = menu.addAction(tr("Close tab")); + connect(closeAction, &QAction::triggered, this, [tabIndex, this]() { + removeTab(tabIndex); + }); + menu.exec(event->globalPos()); +} + +QSize MainWindowTabBar::tabSizeHint(int index) const +{ + Q_UNUSED(index) + return QSize(200, this->height()); +} + +void MainWindowTabBar::handleCurrentChanged(int index) +{ + if(index < 0) { + addTab(createWebView(m_parent->profile()->homepage(), m_parent->profile())); + return; + } + + emit currentTabChanged(m_views.at(index)); +} + +void MainWindowTabBar::removeTab(int index) +{ + // remove the tab data from the index + m_views.at(index)->deleteLater(); + m_views.remove(index); + + // remove the tab from the QTabBar + // this emits the currentTabChanged signal, so it should be done after the view is removed from the index + QTabBar::removeTab(index); +} + +void MainWindowTabBar::updateTabText(WebView *view, const QString &text) +{ + int index = m_views.indexOf(view); + setTabText(index, text); +} + +void MainWindowTabBar::updateVectorArrangement(int from, int to) +{ + m_views.move(from, to); +} + +WebView *createWebView(const QUrl &url, WebEngineProfile *profile) +{ + WebView *view = new WebView(nullptr); + QWebEnginePage *page = new QWebEnginePage(profile); + view->setPage(page); + page->load(url); + + return view; +} diff --git a/src/widgets/mainwindowtabbar.h b/src/widgets/mainwindowtabbar.h new file mode 100644 index 0000000..1186695 --- /dev/null +++ b/src/widgets/mainwindowtabbar.h @@ -0,0 +1,67 @@ +/******************************************************************************* + ** + ** smolbote: yet another qute browser + ** Copyright (C) 2017 Xian Nox + ** + ** This program is free software: you can redistribute it and/or modify + ** it under the terms of the GNU General Public License as published by + ** the Free Software Foundation, either version 3 of the License, or + ** (at your option) any later version. + ** + ** This program is distributed in the hope that it will be useful, + ** but WITHOUT ANY WARRANTY; without even the implied warranty of + ** MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + ** GNU General Public License for more details. + ** + ** You should have received a copy of the GNU General Public License + ** along with this program. If not, see . + ** + ******************************************************************************/ + +#ifndef WEBVIEWTABBAR_H +#define WEBVIEWTABBAR_H + +#include +#include "webengine/webview.h" +#include "webengine/webengineprofile.h" +#include + +class Configuration; +class MainWindow; +class MainWindowTabBar : public QTabBar +{ + Q_OBJECT + +public: + explicit MainWindowTabBar(const std::shared_ptr &config, MainWindow *parent = nullptr); + ~MainWindowTabBar(); + + void setProfile(WebEngineProfile *profile); + WebView *currentView(); + +signals: + void currentTabChanged(WebView *view); + +public slots: + int addTab(WebView *view); + void removeTab(int index); + +protected: + void contextMenuEvent(QContextMenuEvent *event); + QSize tabSizeHint(int index) const; + +private slots: + void handleCurrentChanged(int index); + + void updateTabText(WebView *view, const QString &text); + void updateVectorArrangement(int from, int to); + +private: + // store all views in a vector since tabs can only store a QVariant, and that can't easily take a pointer + QVector m_views; + MainWindow *m_parent; +}; + +WebView *createWebView(const QUrl &url, WebEngineProfile *profile); + +#endif // WEBVIEWTABBAR_H diff --git a/src/widgets/webviewtabbar.cpp b/src/widgets/webviewtabbar.cpp deleted file mode 100644 index 81851f3..0000000 --- a/src/widgets/webviewtabbar.cpp +++ /dev/null @@ -1,159 +0,0 @@ -/******************************************************************************* - ** - ** smolbote: yet another qute browser - ** Copyright (C) 2017 Xian Nox - ** - ** This program is free software: you can redistribute it and/or modify - ** it under the terms of the GNU General Public License as published by - ** the Free Software Foundation, either version 3 of the License, or - ** (at your option) any later version. - ** - ** This program is distributed in the hope that it will be useful, - ** but WITHOUT ANY WARRANTY; without even the implied warranty of - ** MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - ** GNU General Public License for more details. - ** - ** You should have received a copy of the GNU General Public License - ** along with this program. If not, see . - ** - ******************************************************************************/ - -#include "webviewtabbar.h" -#include -#include -#include -#include -#include - -WebViewTabBar::WebViewTabBar(const std::shared_ptr &config, WebEngineProfile *profile, QWidget *parent) : - QTabBar(parent) -{ - m_profile = profile; - - setElideMode(Qt::ElideRight); - setTabsClosable(true); - setMovable(true); - setContextMenuPolicy(Qt::DefaultContextMenu); - connect(this, SIGNAL(tabCloseRequested(int)), this, SLOT(removeTab(int))); - connect(this, SIGNAL(currentChanged(int)), this, SLOT(handleCurrentChanged(int))); - connect(this, SIGNAL(tabMoved(int,int)), this, SLOT(updateVectorArrangement(int,int))); - - QShortcut *tabCloseShortcut = new QShortcut(this); - tabCloseShortcut->setKey(QKeySequence(QString::fromStdString(config->value("browser.shortcuts.tabClose").value()))); - connect(tabCloseShortcut, &QShortcut::activated, [this]() { - this->removeTab(currentIndex()); - }); - - QShortcut *tabLeftShortcut = new QShortcut(this); - tabLeftShortcut->setKey(QKeySequence(QString::fromStdString(config->value("browser.shortcuts.tabLeft").value()))); - connect(tabLeftShortcut, &QShortcut::activated, [this]() { - this->setCurrentIndex(currentIndex()-1); - }); - - QShortcut *tabRightShortcut = new QShortcut(this); - tabRightShortcut->setKey(QKeySequence(QString::fromStdString(config->value("browser.shortcuts.tabRight").value()))); - connect(tabRightShortcut, &QShortcut::activated, [this]() { - this->setCurrentIndex(currentIndex()+1); - }); -} - -WebViewTabBar::~WebViewTabBar() -{ - // cleanup - qDeleteAll(m_views); - m_views.clear(); -} - -int WebViewTabBar::addTab(const QUrl &url) -{ - WebView *view = new WebView(0); - QWebEnginePage *page = new QWebEnginePage(m_profile); - view->setPage(page); - page->load(url); - m_views.append(view); - - connect(view, &QWebEngineView::titleChanged, [this, view](const QString &title) { - int index = m_views.indexOf(view); - setTabText(index, title); - }); - connect(view, &QWebEngineView::iconChanged, [this, view](const QIcon &icon) { - int index = m_views.indexOf(view); - setTabIcon(index, icon); - }); - - return QTabBar::addTab("New Tab"); -} - -void WebViewTabBar::setProfile(WebEngineProfile *profile) -{ - Q_CHECK_PTR(profile); - - m_profile = profile; - for(auto view : qAsConst(m_views)) { - QWebEnginePage *page = new QWebEnginePage(profile); - page->load(view->url()); - view->setPage(page); - } -} - -WebEngineProfile *WebViewTabBar::profile() -{ - return m_profile; -} - -WebView *WebViewTabBar::currentView() -{ - return m_views.at(currentIndex()); -} - -void WebViewTabBar::contextMenuEvent(QContextMenuEvent *event) -{ - int tabIndex = tabAt(event->pos()); - if(tabIndex < 0) { - return; - } - - QMenu menu(this); - QAction* closeAction = menu.addAction(tr("Close tab")); - connect(closeAction, &QAction::triggered, this, [tabIndex, this]() { - removeTab(tabIndex); - }); - menu.exec(event->globalPos()); -} - -QSize WebViewTabBar::tabSizeHint(int index) const -{ - Q_UNUSED(index) - return QSize(200, this->height()); -} - -void WebViewTabBar::handleCurrentChanged(int index) -{ - if(index < 0) { - addTab(m_profile->newtab()); - return; - } - emit currentTabChanged(m_views.at(index)); -} - -void WebViewTabBar::removeTab(int index) -{ - // remove the tab data from the index - m_views.at(index)->deleteLater(); - m_views.remove(index); - - // remove the tab from the QTabBar - // this emits the currentTabChanged signal, so it should be done after the view is removed from the index - QTabBar::removeTab(index); -} - -void WebViewTabBar::updateTabText(WebView *view, const QString &text) -{ - int index = m_views.indexOf(view); - setTabText(index, text); -} - -void WebViewTabBar::updateVectorArrangement(int from, int to) -{ - m_views.move(from, to); -} diff --git a/src/widgets/webviewtabbar.h b/src/widgets/webviewtabbar.h deleted file mode 100644 index 64bb6a2..0000000 --- a/src/widgets/webviewtabbar.h +++ /dev/null @@ -1,67 +0,0 @@ -/******************************************************************************* - ** - ** smolbote: yet another qute browser - ** Copyright (C) 2017 Xian Nox - ** - ** This program is free software: you can redistribute it and/or modify - ** it under the terms of the GNU General Public License as published by - ** the Free Software Foundation, either version 3 of the License, or - ** (at your option) any later version. - ** - ** This program is distributed in the hope that it will be useful, - ** but WITHOUT ANY WARRANTY; without even the implied warranty of - ** MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - ** GNU General Public License for more details. - ** - ** You should have received a copy of the GNU General Public License - ** along with this program. If not, see . - ** - ******************************************************************************/ - -#ifndef WEBVIEWTABBAR_H -#define WEBVIEWTABBAR_H - -#include -#include "webengine/webview.h" -#include "webengine/webengineprofile.h" -#include - -class Configuration; -class WebViewTabBar : public QTabBar -{ - Q_OBJECT - -public: - explicit WebViewTabBar(const std::shared_ptr &config, WebEngineProfile *profile = nullptr, QWidget *parent = 0); - ~WebViewTabBar(); - - void setProfile(WebEngineProfile *profile); - WebEngineProfile *profile(); - - WebView *currentView(); - -signals: - void currentTabChanged(WebView *view); - -public slots: - int addTab(const QUrl &url); - void removeTab(int index); - -protected: - void contextMenuEvent(QContextMenuEvent *event); - QSize tabSizeHint(int index) const; - -private slots: - void handleCurrentChanged(int index); - - void updateTabText(WebView *view, const QString &text); - void updateVectorArrangement(int from, int to); - -private: - // store all views in a vector since tabs can only store a QVariant, and that can't easily take a pointer - QVector m_views; - - WebEngineProfile *m_profile = nullptr; -}; - -#endif // WEBVIEWTABBAR_H -- cgit v1.2.1