Verified Commit 30db8cee authored by Jonah Brüchert's avatar Jonah Brüchert 🌳

Add tabsmodel as central place to control the gui

Advantages:

* Model information is not duplicated between the qml part (QtQml.Models) and the browserManager::m_tabs
* Changes do not need to be synced with browser manager anymore
* properly testable (and test implemented)
* A lot less logic in the ui (qml part)
* No json inside the ini file

Drawbacks:
* isMobile is not saved to disk anymore to be able to use a simple ini list (doesn't matter much to me, please comment if it does to you, ideally with idea how to store it)

General Changed:
* BrowserManager is now a singleton with a static instance getter, to share one QSettings instance throughout Angelfish

 #This is the commit message #3:
parent 9bf37022
......@@ -16,5 +16,10 @@ ecm_add_test(useragenttest.cpp ../src/useragent.cpp
ecm_add_test(browsermanagertest.cpp ../src/browsermanager.cpp ../src/urlmodel.cpp ../src/urlutils.cpp
TEST_NAME browsermanagertest
LINK_LIBRARIES Qt5::Test Qt5::Qml
LINK_LIBRARIES Qt5::Test
)
ecm_add_test(tabsmodeltest.cpp ../src/tabsmodel.cpp ../src/browsermanager.cpp ../src/urlmodel.cpp
TEST_NAME tabsmodeltest
LINK_LIBRARIES Qt5::Test
)
/*
* Copyright 2020 Jonah Brüchert <jbb@kaidan.im>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
* License version 2 as published by the Free Software Foundation;
*
* This library 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
* Library General Public License for more details.
*
* You should have received a copy of the GNU Library General Public License
* along with this library; see the file COPYING.LIB. If not, write to
* the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
* Boston, MA 02110-1301, USA.
*/
#include <QtTest/QTest>
#include "tabsmodel.h"
class TabsModelTest : public QObject
{
Q_OBJECT
private Q_SLOTS:
void initTestCase()
{
m_tabsModel = new TabsModel();
}
void testInitialTabExists()
{
QCOMPARE(m_tabsModel->tabs().count(), 1);
// Current tab should be initial tab
QCOMPARE(m_tabsModel->currentTab(), 0);
QCOMPARE(m_tabsModel->tabs().at(0), "about:blank");
}
void testNewTab()
{
m_tabsModel->newTab("https://kde.org");
QCOMPARE(m_tabsModel->tabs().count(), 2);
QCOMPARE(m_tabsModel->tabs().at(1), "https://kde.org");
// newly created tab should be current tab now
QCOMPARE(m_tabsModel->currentTab(), 1);
}
void testCurrentTab()
{
QCOMPARE(m_tabsModel->tabs().at(m_tabsModel->currentTab()), "https://kde.org");
}
void testCloseTab() {
// Close initial tab, keep kde.org one
m_tabsModel->closeTab(0);
QCOMPARE(m_tabsModel->tabs().count(), 1);
// Check tabs moved properly
QCOMPARE(m_tabsModel->tabs().at(0), "https://kde.org");
}
void testLoad() {
m_tabsModel->load("https://debian.org");
// Number of tabs must not change
QCOMPARE(m_tabsModel->tabs().count(), 1);
QCOMPARE(m_tabsModel->tabs().at(0), "https://debian.org");
}
void testRowCountMatches() {
QCOMPARE(m_tabsModel->tabs().count(), m_tabsModel->rowCount());
}
void testCloseCurrentTab() {
//
// Case 1: There is only one tab, a new one should be created
//
QCOMPARE(m_tabsModel->tabs().count(), 1);
m_tabsModel->setCurrentTab(0);
m_tabsModel->closeTab(0);
// Check whether a new empty tab was created (count must not be less than one)
QCOMPARE(m_tabsModel->tabs().count(), 1);
QCOMPARE(m_tabsModel->tabs().at(0), "about:blank");
//
// Case 2: There are multiple tabs
//
m_tabsModel->newTab("second");
m_tabsModel->newTab("third");
QCOMPARE(m_tabsModel->tabs(), QStringList({"about:blank", "second", "third"}));
// current tab is 2
// close tab "second"
m_tabsModel->closeTab(1);
// current tab should now be 0, since we reset to first tab if the current tab is closed
QCOMPARE(m_tabsModel->currentTab(), 0);
// "second" is indeed gone
QCOMPARE(m_tabsModel->tabs(), QStringList({"about:blank", "third"}));
}
void testSetTab() {
m_tabsModel->setTabUrl(0, "https://debian.org");
QCOMPARE(m_tabsModel->tabs(), QStringList({"https://debian.org", "third"}));
}
void testPrivateMode() {
// private mode is off by default
QCOMPARE(m_tabsModel->privateMode(), false);
m_tabsModel->setPrivateMode(true);
QCOMPARE(m_tabsModel->privateMode(), true);
m_tabsModel->setPrivateMode(false);
QCOMPARE(m_tabsModel->privateMode(), false);
}
private:
TabsModel *m_tabsModel;
};
QTEST_MAIN(TabsModelTest);
#include "tabsmodeltest.moc"
......@@ -5,6 +5,7 @@ set(angelfish_SRCS
urlfilterproxymodel.cpp
urlutils.cpp
useragent.cpp
tabsmodel.cpp
)
qt5_add_resources(RESOURCES resources.qrc)
......
/***************************************************************************
/***************************************************************************
* Copyright 2014 Sebastian Kügler <sebas@kde.org> *
* *
* This program is free software; you can redistribute it and/or modify *
......@@ -28,9 +28,11 @@
using namespace AngelFish;
BrowserManager *BrowserManager::s_instance = nullptr;
BrowserManager::BrowserManager(QObject *parent) : QObject(parent), m_settings(new QSettings(this))
{
loadTabs();
BrowserManager::s_instance = this;
}
BrowserManager::~BrowserManager()
......@@ -107,75 +109,21 @@ void BrowserManager::setSearchBaseUrl(const QString &searchBaseUrl)
emit searchBaseUrlChanged();
}
QString BrowserManager::searchBaseUrl()
QSettings *BrowserManager::settings() const
{
return m_settings->value("browser/searchBaseUrl", "https://start.duckduckgo.com/?q=")
.toString();
return m_settings;
}
int BrowserManager::currentTab() const
{
return m_current_tab;
}
QList<QString> BrowserManager::tabs() const
{
return m_tabs;
}
void BrowserManager::loadTabs()
{
m_tabs = m_settings->value("browser/tabs").toStringList();
m_current_tab = m_settings->value("browser/current_tab", 0).toInt();
}
void BrowserManager::saveTabs()
{
qDebug() << "saveTabs called" << m_tabs;
m_settings->setValue("browser/tabs", QVariant(m_tabs));
}
void BrowserManager::setCurrentTab(int index)
{
if (m_tabsReadonly) return;
m_current_tab = index;
m_settings->setValue("browser/current_tab", m_current_tab);
}
void BrowserManager::setTab(int index, QString url, bool isMobile)
{
if (m_tabsReadonly)
return;
while (m_tabs.length() <= index) {
m_tabs.append(QString());
}
m_tabs[index] = url;
saveTabs();
tabsChanged();
}
void BrowserManager::setTabUrl(int index, QString url)
QString BrowserManager::searchBaseUrl()
{
while (m_tabs.length() <= index) {
m_tabs.append(QString());
}
m_tabs[index] = url;
saveTabs();
tabsChanged();
return m_settings->value("browser/searchBaseUrl", "https://start.duckduckgo.com/?q=")
.toString();
}
void BrowserManager::setTabsWritable()
BrowserManager *BrowserManager::instance()
{
m_tabsReadonly = false;
}
if (!s_instance)
s_instance = new BrowserManager();
void BrowserManager::rmTab(int index)
{
if (index >= 0 && index < m_tabs.size()) {
m_tabs.removeAt(index);
saveTabs();
}
return s_instance;
}
......@@ -23,7 +23,6 @@
#define BOOKMARKSMANAGER_H
#include <QObject>
#include <QQmlPropertyMap>
#include "urlmodel.h"
......@@ -47,26 +46,19 @@ class BrowserManager : public QObject
Q_PROPERTY(QString searchBaseUrl READ searchBaseUrl WRITE setSearchBaseUrl NOTIFY
searchBaseUrlChanged)
Q_PROPERTY(int currentTab READ currentTab NOTIFY currentTabChanged)
Q_PROPERTY(QList<QString> tabs READ tabs NOTIFY tabsChanged)
public:
BrowserManager(QObject *parent = nullptr);
~BrowserManager() override;
static BrowserManager *instance();
UrlModel *bookmarks();
UrlModel *history();
QString homepage();
QString searchBaseUrl();
Q_INVOKABLE int currentTab() const;
Q_INVOKABLE QList<QString> tabs() const;
Q_INVOKABLE void setCurrentTab(int index);
Q_INVOKABLE void setTab(int index, QString url, bool isMobile);
Q_INVOKABLE void setTabUrl(int index, QString url);
Q_INVOKABLE void setTabsWritable();
Q_INVOKABLE void rmTab(int index);
QSettings* settings() const;
signals:
void updated();
......@@ -76,11 +68,6 @@ signals:
void homepageChanged();
void searchBaseUrlChanged();
void loadUrlRequested(const QString &url);
void currentTabChanged();
void tabsChanged();
public slots:
void reload();
......@@ -93,18 +80,12 @@ public slots:
void setHomepage(const QString &homepage);
void setSearchBaseUrl(const QString &searchBaseUrl);
protected:
void loadTabs();
void saveTabs();
private:
UrlModel *m_bookmarks = nullptr;
UrlModel *m_history = nullptr;
QSettings *m_settings;
int m_current_tab = 0;
QList<QString> m_tabs;
bool m_tabsReadonly = true;
static BrowserManager *s_instance;
};
} // namespace
......
......@@ -24,6 +24,8 @@ import QtQuick.Layouts 1.0
import org.kde.kirigami 2.7 as Kirigami
import org.kde.mobile.angelfish 1.0
Kirigami.ScrollablePage {
// id: options
......@@ -35,7 +37,7 @@ Kirigami.ScrollablePage {
UrlDelegate {
onClicked: pageStack.pop()
onRemoved: browserManager.removeBookmark(url);
onRemoved: BrowserManager.removeBookmark(url);
}
}
......@@ -45,7 +47,7 @@ Kirigami.ScrollablePage {
interactive: height < contentHeight
clip: true
model: browserManager.bookmarks
model: BrowserManager.bookmarks
delegate: Kirigami.DelegateRecycler {
width: parent.width
......
......@@ -35,7 +35,7 @@ Kirigami.ScrollablePage {
UrlDelegate {
onClicked: pageStack.pop()
onRemoved: browserManager.removeFromHistory(url);
onRemoved: BrowserManager.removeFromHistory(url);
}
}
......@@ -46,7 +46,7 @@ Kirigami.ScrollablePage {
clip: true
model: UrlFilterProxyModel {
sourceModel: browserManager.history
sourceModel: BrowserManager.history
}
delegate: Kirigami.DelegateRecycler {
......
......@@ -21,10 +21,10 @@
import QtQuick 2.3
import QtQuick.Controls 2.0
import QtQuick.Controls.Styles 1.0
import QtQml.Models 2.1
import QtWebEngine 1.6
import org.kde.mobile.angelfish 1.0
Repeater {
......@@ -34,23 +34,24 @@ Repeater {
property bool activeTabs: true
property bool privateTabsMode: false
property int currentIndex: -1
property alias currentIndex: tabsModel.currentTab
property var currentItem
property alias count: tabsModel.count
property alias tabsModel: tabsModel
model: ListModel {
model: TabsModel {
id: tabsModel
privateMode: privateTabsMode
}
delegate: WebView {
id: wv
id: webView
anchors {
bottom: tabs.bottom
top: tabs.top
}
privateMode: tabs.privateTabsMode
url: pageurl;
url: model.pageurl
width: tabs.width
property bool showView: index === tabs.currentIndex
......@@ -58,59 +59,23 @@ Repeater {
visible: showView && tabs.activeTabs
x: 0
onShowViewChanged: if (showView) tabs.currentItem = wv
onUrlChanged: if (!privateTabsMode) browserManager.setTabUrl(index, url)
}
function createEmptyTab(front) {
newTab("about:blank", front);
}
function newTab(url, front) {
var p = {pageurl: url};
if (front) {
tabsModel.insert(0, p);
tabs.currentIndex = 0;
onShowViewChanged: {
if (showView) {
tabs.currentItem = webView
}
}
else {
tabsModel.append(p);
tabs.currentIndex = tabs.count - 1;
onUrlChanged: {
tabs.tabsModel.setTabUrl(index, url)
}
}
function closeTab(index) {
if (index<0 && index >= tabs.count)
return; // index out of bounds
if (tabs.count <= 1) {
// create new tab before removing the last one
// to avoid linking all signals to null object
createEmptyTab(true);
tabs.currentItem = tabs.itemAt(0);
index = 1;
}
if (tabs.currentIndex === index) {
// handle the removal of current tab
tabs.currentIndex = 0;
tabs.currentItem = tabs.itemAt(0);
}
if (!privateTabsMode) browserManager.rmTab(index);
tabsModel.remove(index);
}
Component.onCompleted: {
if (privateTabsMode)
createEmptyTab();
else
newTab(browserManager.homepage);
if (initialUrl) {
load(initialUrl)
if (!privateTabsMode && !initialUrl && tabsModel.rowCount() === 1 && tabsModel.tabs[0] === "about:blank")
tabsModel.load(BrowserManager.homepage);
else if (initialUrl) {
tabsModel.newTab(initialUrl)
} else {
console.log("Using homepage")
load(browserManager.homepage)
console.log("in private mode, not loading homepage")
}
}
onCurrentIndexChanged: browserManager.setCurrentTab(tabs.currentIndex)
}
......@@ -27,6 +27,8 @@ import QtQuick.Controls 2.0 as Controls
import org.kde.kirigami 2.5 as Kirigami
import org.kde.mobile.angelfish 1.0
Item {
id: navigation
......
......@@ -89,9 +89,9 @@ Controls.Drawer {
function applyUrl() {
if (text.match(RegexWebUrl.re_weburl)) {
load(UrlUtils.urlFromUserInput(text))
tabs.tabsModel.load(UrlUtils.urlFromUserInput(text))
} else {
load(UrlUtils.urlFromUserInput(browserManager.searchBaseUrl + text))
tabs.tabsModel.load(UrlUtils.urlFromUserInput(BrowserManager.searchBaseUrl + text))
}
overlay.close();
}
......@@ -131,7 +131,7 @@ Controls.Drawer {
model: UrlFilterProxyModel {
id: urlFilter
sourceModel: browserManager.history
sourceModel: BrowserManager.history
}
Component.onCompleted: overlay.listView = listView
......
......@@ -15,7 +15,7 @@ Kirigami.InlineMessage {
icon.name: "tab-new"
text: i18n("Open")
onTriggered: {
tabs.newTab(newTabQuestion.url.toString())
tabs.tabsModel.newTab(newTabQuestion.url.toString())
newTabQuestion.visible = false
}
}
......
......@@ -26,6 +26,8 @@ import QtQuick.Layouts 1.11
import org.kde.kirigami 2.7 as Kirigami
import org.kde.mobile.angelfish 1.0
import org.kde.mobile.angelfish 1.0
Kirigami.ScrollablePage {
title: i18n("Settings")
......@@ -82,10 +84,10 @@ Kirigami.ScrollablePage {
id: homePagePopup
title: i18n("Homepage")
description: i18n("Website that should be loaded on startup")
placeholderText: browserManager.homepage
placeholderText: BrowserManager.homepage
onAccepted: {
if (homePagePopup.text !== "")
browserManager.homepage = UrlUtils.urlFromUserInput(homePagePopup.text)
BrowserManager.homepage = UrlUtils.urlFromUserInput(homePagePopup.text)
}
}
......@@ -93,10 +95,10 @@ Kirigami.ScrollablePage {
id: searchEnginePopup
title: i18n("Search Engine")
description: i18n("Base URL of your preferred search engine")
placeholderText: browserManager.searchBaseUrl
placeholderText: BrowserManager.searchBaseUrl
onAccepted: {
if (searchEnginePopup.text !== "")
browserManager.searchBaseUrl = UrlUtils.urlFromUserInput(searchEnginePopup.text);
BrowserManager.searchBaseUrl = UrlUtils.urlFromUserInput(searchEnginePopup.text);
}
}
......
......@@ -28,9 +28,8 @@ import QtGraphicalEffects 1.0
import QtQuick.Layouts 1.0
import org.kde.kirigami 2.7 as Kirigami
// import org.kde.plasma.components 2.0 as PlasmaComponents
// import org.kde.plasma.extras 2.0 as PlasmaExtras
import org.kde.mobile.angelfish 1.0
Kirigami.ScrollablePage {
id: tabsRoot
......@@ -46,7 +45,8 @@ Kirigami.ScrollablePage {
icon.name: "list-add"
text: i18n("New")
onTriggered: {
tabs.newTab(rootPage.privateMode ? "about:blank" : browserManager.homepage)
// Somewhat weird behaviour, consider always just opening "about:blank"
tabs.tabsModel.newTab(rootPage.privateMode ? "about:blank" : BrowserManager.homepage)
tabs.currentIndex = tabs.count - 1;
pageStack.pop()
}
......@@ -193,7 +193,7 @@ Kirigami.ScrollablePage {
anchors.rightMargin: Kirigami.Units.smallSpacing + Kirigami.Units.largeSpacing + (tabsRoot.landscapeMode ? 0 : tabsRoot.width-grid.width)
anchors.top: parent.top
anchors.topMargin: Kirigami.Units.smallSpacing
onClicked: tabs.closeTab(index)
onClicked: tabs.tabsModel.closeTab(index)
}
Column {
......
......@@ -39,7 +39,7 @@ Kirigami.SwipeListItem {
Kirigami.Theme.colorSet: Kirigami.Theme.View
onClicked: {
load(url)
tabs.tabsModel.load(url)
//tabs.newTab(url)
//contentView.state = "hidden"
}
......
......@@ -121,20 +121,20 @@ WebEngineView {
onTriggered: webEngineView.triggerWebAction(WebEngineView.Paste)
}
Controls.MenuItem {
enabled: contextMenu.request != null && contextMenu.request.linkUrl !== ""
enabled: contextMenu.request !== null && contextMenu.request.linkUrl !== ""
text: i18n("Copy Url")
onTriggered: webEngineView.triggerWebAction(WebEngineView.CopyLinkToClipboard)
}
Controls.MenuItem {
text: i18n("View source")
onTriggered: newTab("view-source:" + webEngineView.url)
onTriggered: tabsModel.newTab("view-source:" + webEngineView.url)
}
Controls.MenuItem {
text: i18n("Download")
onTriggered: webEngineView.triggerWebAction(WebEngineView.DownloadLinkToDisk)
}
Controls.MenuItem {
enabled: contextMenu.request != null && contextMenu.request.linkUrl !== ""
enabled: contextMenu.request !== null && contextMenu.request.linkUrl !== ""
text: i18n("Open in new Tab")
onTriggered: webEngineView.triggerWebAction(WebEngineView.OpenLinkInNewTab)
}
......@@ -182,12 +182,12 @@ WebEngineView {
onIconChanged: {
if (icon)
browserManager.history.updateIcon(url, icon)
BrowserManager.history.updateIcon(url, icon)
}
onNewViewRequested: {
if (request.userInitiated) {
newTab(request.requestedUrl.toString())
tabsModel.newTab(request.requestedUrl.toString())
showPassiveNotification(i18n("Website was opened in a new tab"))
} else {
questionLoader.setSource("NewTabQuestion.qml")
......
......@@ -44,7 +44,7 @@ Kirigami.ApplicationWindow {
//
// As there are private and normal tabs, switch between
// them according to the current mode.
property Item tabs: rootPage.privateMode ? privateTabs : regularTabs
property ListWebView tabs: rootPage.privateMode ? privateTabs : regularTabs