Merge lp:~osomon/webbrowser-app/uriboni-tab-context-menu into lp:webbrowser-app

Proposed by Olivier Tilloy on 2015-09-23
Status: Merged
Approved by: Olivier Tilloy on 2015-09-28
Approved revision: 1154
Merged at revision: 1207
Proposed branch: lp:~osomon/webbrowser-app/uriboni-tab-context-menu
Merge into: lp:webbrowser-app
Diff against target: 1057 lines (+527/-130)
12 files modified
src/app/webbrowser/Browser.qml (+6/-5)
src/app/webbrowser/BrowserTab.qml (+8/-0)
src/app/webbrowser/Chrome.qml (+2/-2)
src/app/webbrowser/TabItem.qml (+28/-14)
src/app/webbrowser/TabsBar.qml (+62/-28)
src/app/webbrowser/tabs-model.cpp (+42/-12)
src/app/webbrowser/tabs-model.h (+2/-0)
tests/autopilot/webbrowser_app/emulators/browser.py (+3/-3)
tests/unittests/qml/qml_tree_helpers.js (+42/-0)
tests/unittests/qml/tst_BrowserTab.qml (+17/-0)
tests/unittests/qml/tst_TabsBar.qml (+146/-8)
tests/unittests/tabs-model/tst_TabsModelTests.cpp (+169/-58)
To merge this branch: bzr merge lp:~osomon/webbrowser-app/uriboni-tab-context-menu
Reviewer Review Type Date Requested Status
Ugo Riboni (community) 2015-09-23 Approve on 2015-09-28
Riccardo Padovani (community) Approve on 2015-09-28
PS Jenkins bot continuous-integration Needs Fixing on 2015-09-24
Review via email: mp+272148@code.launchpad.net

Commit message

Add a context menu to each tab in the tab bar, allowing to insert a new tab just after, close or reload the current tab.

To post a comment you must log in.
Olivier Tilloy (osomon) wrote :

Note to the reviewer: this is based on and supersedes a previous proposal by Ugo, see initial review at https://code.launchpad.net/~uriboni/webbrowser-app/tab-context-menu/+merge/268220.

Riccardo Padovani (rpadovani) wrote :

I tested it extensively and looks good: I also looked to code and I think it's ok.

Only thing to fix: the 'x' to close a tab isn't clickable anymore, the MouseArea that intercepts the click for context menu overalls it. Except from that, for me it's ready to go

review: Needs Fixing
Olivier Tilloy (osomon) wrote :

> Only thing to fix: the 'x' to close a tab isn't clickable anymore,
> the MouseArea that intercepts the click for context menu overalls it.

Wow, nice catch, thanks for testing. This was caused by merging the latest changes from trunk. The conflict was not as trivial as it initially appeared, I now fixed this properly.

Riccardo Padovani (rpadovani) wrote :

lgtm now, thanks!

just a very little thing (tbh, I don't think it's fixable): you disabled the x dragging while right clicking (and that's perfect) but if I press right click and before releasing it I middle click the tab is closed (I expect it is ignored)

review: Approve
Riccardo Padovani (rpadovani) wrote :

Now is perfect, thanks so much!

review: Approve
Ugo Riboni (uriboni) wrote :

All looks good

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/app/webbrowser/Browser.qml'
--- src/app/webbrowser/Browser.qml 2015-09-22 01:27:19 +0000
+++ src/app/webbrowser/Browser.qml 2015-09-28 08:10:23 +0000
@@ -353,7 +353,7 @@
353 onRemoved: if (chrome.bookmarked && (url === chrome.webview.url)) chrome.bookmarked = false353 onRemoved: if (chrome.bookmarked && (url === chrome.webview.url)) chrome.bookmarked = false
354 }354 }
355355
356 onRequestNewTab: browser.openUrlInNewTab("", true)356 onRequestNewTab: browser.openUrlInNewTab("", makeCurrent, true, index)
357357
358 onFindInPageModeChanged: if (!chrome.findInPageMode) internal.resetFocus()358 onFindInPageModeChanged: if (!chrome.findInPageMode) internal.resetFocus()
359359
@@ -1247,8 +1247,9 @@
1247 if (share) share.shareText(text)1247 if (share) share.shareText(text)
1248 }1248 }
12491249
1250 function addTab(tab, setCurrent) {1250 function addTab(tab, setCurrent, index) {
1251 var index = tabsModel.add(tab)1251 if (index === undefined) index = tabsModel.add(tab)
1252 else index = tabsModel.insert(tab, index)
1252 if (setCurrent) {1253 if (setCurrent) {
1253 tabsModel.currentIndex = index1254 tabsModel.currentIndex = index
1254 chrome.requestedUrl = tab.initialUrl1255 chrome.requestedUrl = tab.initialUrl
@@ -1351,10 +1352,10 @@
1351 }1352 }
1352 }1353 }
13531354
1354 function openUrlInNewTab(url, setCurrent, load) {1355 function openUrlInNewTab(url, setCurrent, load, index) {
1355 load = typeof load !== 'undefined' ? load : true1356 load = typeof load !== 'undefined' ? load : true
1356 var tab = tabComponent.createObject(tabContainer, {"initialUrl": url, 'incognito': browser.incognito})1357 var tab = tabComponent.createObject(tabContainer, {"initialUrl": url, 'incognito': browser.incognito})
1357 internal.addTab(tab, setCurrent)1358 internal.addTab(tab, setCurrent, index)
1358 if (load) {1359 if (load) {
1359 tabsModel.currentTab.load()1360 tabsModel.currentTab.load()
1360 }1361 }
13611362
=== modified file 'src/app/webbrowser/BrowserTab.qml'
--- src/app/webbrowser/BrowserTab.qml 2015-09-02 10:35:36 +0000
+++ src/app/webbrowser/BrowserTab.qml 2015-09-28 08:10:23 +0000
@@ -71,6 +71,14 @@
71 }71 }
72 }72 }
7373
74 function reload() {
75 if (webview) {
76 webview.reload()
77 } else {
78 load()
79 }
80 }
81
74 function close() {82 function close() {
75 unload()83 unload()
76 if (preview && preview.toString()) {84 if (preview && preview.toString()) {
7785
=== modified file 'src/app/webbrowser/Chrome.qml'
--- src/app/webbrowser/Chrome.qml 2015-09-17 09:15:51 +0000
+++ src/app/webbrowser/Chrome.qml 2015-09-28 08:10:23 +0000
@@ -39,7 +39,7 @@
39 property alias showFaviconInAddressBar: navigationBar.showFaviconInAddressBar39 property alias showFaviconInAddressBar: navigationBar.showFaviconInAddressBar
40 readonly property alias bookmarkTogglePlaceHolder: navigationBar.bookmarkTogglePlaceHolder40 readonly property alias bookmarkTogglePlaceHolder: navigationBar.bookmarkTogglePlaceHolder
4141
42 signal requestNewTab()42 signal requestNewTab(int index, bool makeCurrent)
4343
44 backgroundColor: incognito ? UbuntuColors.darkGrey : "#bcbcbc"44 backgroundColor: incognito ? UbuntuColors.darkGrey : "#bcbcbc"
4545
@@ -67,7 +67,7 @@
67 sourceComponent: TabsBar {67 sourceComponent: TabsBar {
68 model: tabsModel68 model: tabsModel
69 incognito: chrome.incognito69 incognito: chrome.incognito
70 onRequestNewTab: chrome.requestNewTab()70 onRequestNewTab: chrome.requestNewTab(index, makeCurrent)
71 }71 }
7272
73 anchors {73 anchors {
7474
=== modified file 'src/app/webbrowser/TabItem.qml'
--- src/app/webbrowser/TabItem.qml 2015-09-17 18:32:59 +0000
+++ src/app/webbrowser/TabItem.qml 2015-09-28 08:10:23 +0000
@@ -26,13 +26,18 @@
26 property bool incognito: false26 property bool incognito: false
27 property bool active: false27 property bool active: false
28 property bool hoverable: true28 property bool hoverable: true
29 property int rightMargin: tabImage.anchors.rightMargin29 property real rightMargin: 0
3030
31 property alias title: label.text31 property alias title: label.text
32 property alias icon: favicon.source32 property alias icon: favicon.source
3333
34 property real dragMin: 0
35 property real dragMax: 0
36 readonly property bool dragging: mouseArea.drag.active
37
34 signal selected()38 signal selected()
35 signal closed()39 signal closed()
40 signal contextMenu()
3641
37 BorderImage {42 BorderImage {
38 id: tabImage43 id: tabImage
@@ -83,14 +88,33 @@
83 }88 }
8489
85 MouseArea {90 MouseArea {
91 id: hoverArea
92 anchors.fill: parent
93 hoverEnabled: !tabItem.active && tabItem.hoverable
94 }
95
96 MouseArea {
97 id: mouseArea
86 anchors.left: parent.left98 anchors.left: parent.left
87 anchors.top: parent.top99 anchors.top: parent.top
88 anchors.bottom: parent.bottom100 anchors.bottom: parent.bottom
89 anchors.right: closeButton.left101 anchors.right: closeButton.left
90 onClicked: tabItem.selected()102 acceptedButtons: Qt.AllButtons
103 onPressed: {
104 if (mouse.button === Qt.LeftButton) {
105 tabItem.selected()
106 } else if (mouse.button === Qt.RightButton) {
107 tabItem.contextMenu()
108 }
109 }
110 onClicked: {
111 if ((mouse.buttons === 0) && (mouse.button === Qt.MiddleButton)) {
112 tabItem.closed()
113 }
114 }
91 }115 }
92116
93 AbstractButton {117 MouseArea {
94 id: closeButton118 id: closeButton
95 objectName: "closeButton"119 objectName: "closeButton"
96120
@@ -108,17 +132,7 @@
108 name: "close"132 name: "close"
109 }133 }
110134
111 onTriggered: closed()135 onClicked: closed()
112 }
113
114 MouseArea {
115 id: hoverArea
116 anchors.fill: parent
117 hoverEnabled: !tabItem.active && tabItem.hoverable
118 propagateComposedEvents: true
119 acceptedButtons: Qt.MiddleButton
120 onClicked: tabItem.closed()
121 }136 }
122 }137 }
123}138}
124
125139
=== modified file 'src/app/webbrowser/TabsBar.qml'
--- src/app/webbrowser/TabsBar.qml 2015-09-04 17:48:37 +0000
+++ src/app/webbrowser/TabsBar.qml 2015-09-28 08:10:23 +0000
@@ -18,6 +18,7 @@
1818
19import QtQuick 2.419import QtQuick 2.4
20import Ubuntu.Components 1.320import Ubuntu.Components 1.3
21import Ubuntu.Components.Popups 1.3
21import ".."22import ".."
2223
23Item {24Item {
@@ -31,7 +32,7 @@
3132
32 property bool incognito: false33 property bool incognito: false
3334
34 signal requestNewTab()35 signal requestNewTab(int index, bool makeCurrent)
3536
36 MouseArea {37 MouseArea {
37 anchors.fill: parent38 anchors.fill: parent
@@ -67,7 +68,36 @@
67 color: incognito ? "white" : UbuntuColors.darkGrey68 color: incognito ? "white" : UbuntuColors.darkGrey
68 }69 }
6970
70 onClicked: root.requestNewTab()71 onClicked: root.requestNewTab(root.model.count, true)
72 }
73
74 Component {
75 id: contextualOptionsComponent
76 ActionSelectionPopover {
77 id: menu
78 objectName: "tabContextualActions"
79 property int targetIndex
80 readonly property var tab: root.model.get(targetIndex)
81
82 actions: ActionList {
83 Action {
84 objectName: "tab_action_new_tab"
85 text: i18n.tr("New Tab")
86 onTriggered: root.requestNewTab(menu.targetIndex + 1, false)
87 }
88 Action {
89 objectName: "tab_action_reload"
90 text: i18n.tr("Reload")
91 enabled: menu.tab.url.toString().length > 0
92 onTriggered: menu.tab.reload()
93 }
94 Action {
95 objectName: "tab_action_close_tab"
96 text: i18n.tr("Close Tab")
97 onTriggered: internal.closeTab(menu.targetIndex)
98 }
99 }
100 }
71 }101 }
72102
73 Item {103 Item {
@@ -88,54 +118,58 @@
88118
89 property bool reordering: false119 property bool reordering: false
90120
91 delegate: TabItem {121 delegate: MouseArea {
92 id: tabDelegate122 id: tabDelegate
93 objectName: "tabDelegate"123 objectName: "tabDelegate"
124
94 readonly property int tabIndex: index125 readonly property int tabIndex: index
95126
96 active: index === root.model.currentIndex
97 hoverable: true
98 incognito: root.incognito
99 title: model.title ? model.title : (model.url.toString() ? model.url : i18n.tr("New tab"))
100 icon: model.icon
101
102 anchors.top: tabsContainer.top127 anchors.top: tabsContainer.top
128 property real rightMargin: units.dp(1)
103 width: tabWidth + rightMargin129 width: tabWidth + rightMargin
104 height: tabsContainer.height130 height: tabsContainer.height
105 rightMargin: units.dp(1)131
106132 acceptedButtons: Qt.LeftButton | Qt.MiddleButton | Qt.RightButton
107 onClosed: internal.closeTab(index)133 readonly property bool dragging: drag.active
108 onSelected: root.model.currentIndex = index134 drag {
109135 target: (pressedButtons === Qt.LeftButton) ? tabDelegate : null
110 MouseArea {136 axis: Drag.XAxis
111 id: mouseArea137 minimumX: 0
138 maximumX: root.width - tabDelegate.width
139 filterChildren: true
140 }
141
142 TabItem {
112 anchors.fill: parent143 anchors.fill: parent
113 // XXX: should not start a drag when middle button was pressed144
114 drag {145 active: tabIndex === root.model.currentIndex
115 target: tabDelegate146 hoverable: true
116 axis: Drag.XAxis147 incognito: root.incognito
117 minimumX: 0148 title: model.title ? model.title : (model.url.toString() ? model.url : i18n.tr("New tab"))
118 maximumX: root.width - tabDelegate.width149 icon: model.icon
119 }150
120 onReleased: root.model.currentIndex = index151 rightMargin: tabDelegate.rightMargin
121 propagateComposedEvents: true152
153 onClosed: internal.closeTab(index)
154 onSelected: root.model.currentIndex = index
155 onContextMenu: PopupUtils.open(contextualOptionsComponent, tabDelegate, {"targetIndex": index})
122 }156 }
123157
124 Binding {158 Binding {
125 target: repeater159 target: repeater
126 property: "reordering"160 property: "reordering"
127 value: mouseArea.drag.active161 value: dragging
128 }162 }
129163
130 Binding on x {164 Binding on x {
131 when: !mouseArea.drag.active165 when: !dragging
132 value: index * width166 value: index * width
133 }167 }
134168
135 Behavior on x { NumberAnimation { duration: 250 } }169 Behavior on x { NumberAnimation { duration: 250 } }
136170
137 onXChanged: {171 onXChanged: {
138 if (!mouseArea.drag.active) return172 if (!dragging) return
139 if (x < (index * width - width / 2)) {173 if (x < (index * width - width / 2)) {
140 root.model.move(index, index - 1)174 root.model.move(index, index - 1)
141 } else if ((x > (index * width + width / 2)) && (index < (root.model.count - 1))) {175 } else if ((x > (index * width + width / 2)) && (index < (root.model.count - 1))) {
142176
=== modified file 'src/app/webbrowser/tabs-model.cpp'
--- src/app/webbrowser/tabs-model.cpp 2015-09-02 16:48:01 +0000
+++ src/app/webbrowser/tabs-model.cpp 2015-09-28 08:10:23 +0000
@@ -21,6 +21,7 @@
21// Qt21// Qt
22#include <QtCore/QDebug>22#include <QtCore/QDebug>
23#include <QtCore/QObject>23#include <QtCore/QObject>
24#include <QtCore/QtGlobal>
2425
25/*!26/*!
26 \class TabsModel27 \class TabsModel
@@ -101,35 +102,58 @@
101102
102QObject* TabsModel::currentTab() const103QObject* TabsModel::currentTab() const
103{104{
104 if (m_tabs.isEmpty()) {105 if (m_tabs.isEmpty() || !checkValidTabIndex(m_currentIndex)) {
105 return nullptr;106 return nullptr;
106 }107 }
107 return m_tabs.at(m_currentIndex);108 return m_tabs.at(m_currentIndex);
108}109}
109110
110/*!111/*!
111 Add a tab to the model and return the corresponding index in the model.112 Append a tab to the model and return the corresponding index in the model.
112113
113 It is the responsibility of the caller to instantiate the corresponding114 It is the responsibility of the caller to instantiate the corresponding
114 Tab beforehand.115 Tab beforehand.
115*/116*/
116int TabsModel::add(QObject* tab)117int TabsModel::add(QObject* tab)
117{118{
119 return insert(tab, m_tabs.count());
120}
121
122/*!
123 Add a tab to the model at the specified index, and return the index itself,
124 or -1 if the operation failed.
125
126 It is the responsibility of the caller to instantiate the corresponding
127 Tab beforehand.
128*/
129int TabsModel::insert(QObject* tab, int index)
130{
118 if (tab == nullptr) {131 if (tab == nullptr) {
119 qWarning() << "Invalid Tab";132 qWarning() << "Invalid Tab";
120 return -1;133 return -1;
121 }134 }
122 int index = m_tabs.count();135 index = qMax(qMin(index, m_tabs.count()), 0);
123 beginInsertRows(QModelIndex(), index, index);136 beginInsertRows(QModelIndex(), index, index);
124 m_tabs.append(tab);137 m_tabs.insert(index, tab);
125 connect(tab, SIGNAL(urlChanged()), SLOT(onUrlChanged()));138 connect(tab, SIGNAL(urlChanged()), SLOT(onUrlChanged()));
126 connect(tab, SIGNAL(titleChanged()), SLOT(onTitleChanged()));139 connect(tab, SIGNAL(titleChanged()), SLOT(onTitleChanged()));
127 connect(tab, SIGNAL(iconChanged()), SLOT(onIconChanged()));140 connect(tab, SIGNAL(iconChanged()), SLOT(onIconChanged()));
128 endInsertRows();141 endInsertRows();
129 Q_EMIT countChanged();142 Q_EMIT countChanged();
130 if (index == 0) {143
131 setCurrentIndex(0);144 if (m_currentIndex == -1) {
145 // Set the index to zero if this is the first item that gets added to the
146 // model, as it should not be possible to have items in the model but no
147 // current tab.
148 m_currentIndex = 0;
149 Q_EMIT currentIndexChanged();
150 Q_EMIT currentTabChanged();
151 } else if (index <= m_currentIndex) {
152 // Increment the index if we are inserting items before the current index.
153 m_currentIndex++;
154 Q_EMIT currentIndexChanged();
132 }155 }
156
133 return index;157 return index;
134}158}
135159
@@ -150,15 +174,21 @@
150 tab->disconnect(this);174 tab->disconnect(this);
151 endRemoveRows();175 endRemoveRows();
152 Q_EMIT countChanged();176 Q_EMIT countChanged();
153 if (index == m_currentIndex) {177
154 if (!checkValidTabIndex(index)) {178 if (index < m_currentIndex) {
155 m_currentIndex = m_tabs.count() - 1;179 // If we removed any tab before the current one, decrease the
180 // current index to match.
181 m_currentIndex--;
182 Q_EMIT currentIndexChanged();
183 } else if (index == m_currentIndex) {
184 // If the current tab was removed, the following one (if any) is made
185 // current. If it was the last tab in the model, the current index needs
186 // to be decreased.
187 if (m_currentIndex == m_tabs.count()) {
188 m_currentIndex--;
156 Q_EMIT currentIndexChanged();189 Q_EMIT currentIndexChanged();
157 }190 }
158 Q_EMIT currentTabChanged();191 Q_EMIT currentTabChanged();
159 } else if (m_currentIndex > index) {
160 m_currentIndex -= 1;
161 Q_EMIT currentIndexChanged();
162 }192 }
163 return tab;193 return tab;
164}194}
165195
=== modified file 'src/app/webbrowser/tabs-model.h'
--- src/app/webbrowser/tabs-model.h 2015-06-18 14:28:11 +0000
+++ src/app/webbrowser/tabs-model.h 2015-09-28 08:10:23 +0000
@@ -57,6 +57,7 @@
57 QObject* currentTab() const;57 QObject* currentTab() const;
5858
59 Q_INVOKABLE int add(QObject* tab);59 Q_INVOKABLE int add(QObject* tab);
60 Q_INVOKABLE int insert(QObject* tab, int index);
60 Q_INVOKABLE QObject* remove(int index);61 Q_INVOKABLE QObject* remove(int index);
61 Q_INVOKABLE QObject* get(int index) const;62 Q_INVOKABLE QObject* get(int index) const;
62 Q_INVOKABLE void move(int from, int to);63 Q_INVOKABLE void move(int from, int to);
@@ -76,6 +77,7 @@
76 int m_currentIndex;77 int m_currentIndex;
7778
78 bool checkValidTabIndex(int index) const;79 bool checkValidTabIndex(int index) const;
80 void setCurrentIndexNoCheck(int index);
79 void onDataChanged(QObject* tab, int role);81 void onDataChanged(QObject* tab, int role);
80};82};
8183
8284
=== modified file 'tests/autopilot/webbrowser_app/emulators/browser.py'
--- tests/autopilot/webbrowser_app/emulators/browser.py 2015-09-13 21:24:35 +0000
+++ tests/autopilot/webbrowser_app/emulators/browser.py 2015-09-28 08:10:23 +0000
@@ -329,10 +329,10 @@
329 self.pointing_device.click_object(button)329 self.pointing_device.click_object(button)
330330
331 def get_tabs(self):331 def get_tabs(self):
332 return self.select_many("QQuickItem", objectName="tabDelegate")332 return self.select_many("QQuickMouseArea", objectName="tabDelegate")
333333
334 def get_tab(self, index):334 def get_tab(self, index):
335 return self.select_single("QQuickItem", objectName="tabDelegate",335 return self.select_single("QQuickMouseArea", objectName="tabDelegate",
336 tabIndex=index)336 tabIndex=index)
337337
338 @autopilot.logging.log_action(logger.info)338 @autopilot.logging.log_action(logger.info)
@@ -342,7 +342,7 @@
342 @autopilot.logging.log_action(logger.info)342 @autopilot.logging.log_action(logger.info)
343 def close_tab(self, index):343 def close_tab(self, index):
344 tab = self.get_tab(index)344 tab = self.get_tab(index)
345 close_button = tab.select_single("Icon", objectName="closeButton")345 close_button = tab.select_single(objectName="closeButton")
346 self.pointing_device.click_object(close_button)346 self.pointing_device.click_object(close_button)
347347
348348
349349
=== added file 'tests/unittests/qml/qml_tree_helpers.js'
--- tests/unittests/qml/qml_tree_helpers.js 1970-01-01 00:00:00 +0000
+++ tests/unittests/qml/qml_tree_helpers.js 2015-09-28 08:10:23 +0000
@@ -0,0 +1,42 @@
1/*
2 * Copyright 2015 Canonical Ltd.
3 *
4 * This file is part of webbrowser-app.
5 *
6 * webbrowser-app is free software; you can redistribute it and/or modify
7 * it under the terms of the GNU General Public License as published by
8 * the Free Software Foundation; version 3.
9 *
10 * webbrowser-app is distributed in the hope that it will be useful,
11 * but WITHOUT ANY WARRANTY; without even the implied warranty of
12 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13 * GNU General Public License for more details.
14 *
15 * You should have received a copy of the GNU General Public License
16 * along with this program. If not, see <http://www.gnu.org/licenses/>.
17 */
18
19// Given the naming convention in QML for class names, we should
20// never have a class name with underscores in it, so the following
21// should be a safe way to remove the rest of the extra metatype
22// information produced by converting QML objects to strings.
23function qmlType(item) {
24 return String(item).split("_")[0]
25}
26
27function findDescendantsByType(item, type, list) {
28 list = list || []
29 if (qmlType(item) === type) list.push(item)
30 for (var i in item.children) {
31 findDescendantsByType(item.children[i], type, list)
32 }
33 return list
34}
35
36function findAncestorByType(item, type) {
37 while (item.parent) {
38 if (qmlType(item.parent) === type) return item.parent
39 item = item.parent
40 }
41 return null
42}
043
=== modified file 'tests/unittests/qml/tst_BrowserTab.qml'
--- tests/unittests/qml/tst_BrowserTab.qml 2015-08-10 15:22:00 +0000
+++ tests/unittests/qml/tst_BrowserTab.qml 2015-09-28 08:10:23 +0000
@@ -36,6 +36,9 @@
36 property url icon36 property url icon
37 property var request37 property var request
38 property string currentState38 property string currentState
39
40 property int reloaded: 0
41 function reload() { reloaded++ }
39 }42 }
40 readonly property bool webviewPresent: webview43 readonly property bool webviewPresent: webview
41 }44 }
@@ -76,6 +79,20 @@
76 tab.destroy()79 tab.destroy()
77 }80 }
7881
82 function test_reload() {
83 var tab = tabComponent.createObject(root)
84 verify(!tab.webviewPresent)
85
86 tab.initialUrl = "http://example.org"
87 tab.reload()
88 tryCompare(tab, 'webviewPresent', true)
89 compare(tab.webview.reloaded, 0)
90
91 tab.reload()
92 verify(tab.webviewPresent)
93 compare(tab.webview.reloaded, 1)
94 }
95
79 function test_create_with_request() {96 function test_create_with_request() {
80 var tab = tabComponent.createObject(root, {'request': "foobar"})97 var tab = tabComponent.createObject(root, {'request': "foobar"})
81 tryCompare(tab, 'webviewPresent', true)98 tryCompare(tab, 'webviewPresent', true)
8299
=== modified file 'tests/unittests/qml/tst_TabsBar.qml'
--- tests/unittests/qml/tst_TabsBar.qml 2015-08-10 15:22:00 +0000
+++ tests/unittests/qml/tst_TabsBar.qml 2015-09-28 08:10:23 +0000
@@ -21,12 +21,14 @@
21import Ubuntu.Test 1.021import Ubuntu.Test 1.0
22import "../../../src/app/webbrowser"22import "../../../src/app/webbrowser"
23import webbrowserapp.private 0.123import webbrowserapp.private 0.1
24import "qml_tree_helpers.js" as Tree
2425
25Item {26Item {
26 id: root27 id: root
2728
28 width: 60029 width: 600
29 height: 5030 height: 200
31 signal reload(string url)
3032
31 TabsModel {33 TabsModel {
32 id: tabsModel34 id: tabsModel
@@ -35,22 +37,35 @@
35 Component {37 Component {
36 id: tabComponent38 id: tabComponent
37 QtObject {39 QtObject {
40 id: tab
38 property url url41 property url url
39 property string title42 property string title
40 property url icon43 property url icon
41 function close() { destroy() }44 function close() { destroy() }
45 function reload() { root.reload(tab.url) }
42 }46 }
43 }47 }
4448
45 TabsBar {49 TabsBar {
46 id: tabs50 id: tabs
47 anchors.fill: parent51
52 // Make the tabs bar smaller than the window and aligned in the middle
53 // to leave room for the context menu to pop up and have all its items
54 // visible within the screen.
55 anchors.left: parent.left
56 anchors.right: parent.right
57 anchors.verticalCenter: parent.verticalCenter
58 height: 50
59
48 model: tabsModel60 model: tabsModel
49 onRequestNewTab: appendTab("", "", "")61 onRequestNewTab: insertTab("", "", "", index)
50 function appendTab(url, title, icon) {62 function appendTab(url, title, icon) {
63 insertTab(url, title, icon, model.count)
64 model.currentIndex = model.count - 1
65 }
66 function insertTab(url, title, icon, index) {
51 var tab = tabComponent.createObject(root, {"url": url, "title": title, "icon": icon})67 var tab = tabComponent.createObject(root, {"url": url, "title": title, "icon": icon})
52 model.add(tab)68 model.insert(tab, index)
53 model.currentIndex = model.count - 1
54 }69 }
55 }70 }
5671
@@ -60,13 +75,46 @@
60 signalName: "requestNewTab"75 signalName: "requestNewTab"
61 }76 }
6277
78 SignalSpy {
79 id: reloadSpy
80 target: root
81 signalName: "reload"
82 }
83
84 // Ideally we would get menu items by their objectName, however they are
85 // created dynamically by the ActionSelectionPopover and the objectName
86 // of the source action is not copied to the generated item.
87 // So we first select the source Action by objectName then look for an item
88 // with the same text as the action. This is not ideal but it will work
89 // since we don't have items with the same text.
90 // https://launchpad.net/bugs/1205144 tracks the issue, and as of 2015-09-23
91 // is fixed in the vivid overlay PPA but not in wily yet.
92 function getMenuItemForAction(menu, actionName) {
93 actionName = "tab_action_" + actionName
94 var text = ""
95 var actions = menu.actions.actions
96 for (var i = 0; i < actions.length; i++) {
97 if (actions[i].objectName === actionName) {
98 text = actions[i].text
99 break
100 }
101 }
102 if (text === "") return null
103
104 var menuItems = Tree.findDescendantsByType(menu, "Label")
105 var matching = menuItems.filter(function(item) { return item.text === text })
106 if (matching.length === 0) return null
107 else return Tree.findAncestorByType(matching[0], "Empty")
108 }
109
63 UbuntuTestCase {110 UbuntuTestCase {
64 name: "TabsBar"111 name: "TabsBar"
65 when: windowShown112 when: windowShown
66113
67 function clickItem(item) {114 function clickItem(item, button) {
115 if (button === undefined) button = Qt.LeftButton
68 var center = centerOf(item)116 var center = centerOf(item)
69 mouseClick(item, center.x, center.y)117 mouseClick(item, center.x, center.y, button)
70 }118 }
71119
72 function getTabDelegate(index) {120 function getTabDelegate(index) {
@@ -80,11 +128,22 @@
80 return null128 return null
81 }129 }
82130
131 function popupMenuOnTab(index) {
132 var tab = getTabDelegate(index)
133 if (tab) {
134 clickItem(tab, Qt.RightButton)
135 var menu = findChild(root, "tabContextualActions")
136 waitForRendering(menu)
137 return menu
138 } else return null
139 }
140
83 function cleanup() {141 function cleanup() {
84 while (tabsModel.count > 0) {142 while (tabsModel.count > 0) {
85 tabsModel.remove(0).destroy()143 tabsModel.remove(0).destroy()
86 }144 }
87 newTabRequestSpy.clear()145 newTabRequestSpy.clear()
146 reloadSpy.clear()
88 }147 }
89148
90 function populateTabs() {149 function populateTabs() {
@@ -101,6 +160,9 @@
101 clickItem(newTabButton)160 clickItem(newTabButton)
102 }161 }
103 compare(newTabRequestSpy.count, 3)162 compare(newTabRequestSpy.count, 3)
163 compare(newTabRequestSpy.signalArguments[0][0], 0)
164 compare(newTabRequestSpy.signalArguments[1][0], 1)
165 compare(newTabRequestSpy.signalArguments[2][0], 2)
104 }166 }
105167
106 function test_mouse_left_click() {168 function test_mouse_left_click() {
@@ -117,11 +179,19 @@
117 populateTabs()179 populateTabs()
118 for (var i = 2; i >= 0; --i) {180 for (var i = 2; i >= 0; --i) {
119 var tab0 = getTabDelegate(0)181 var tab0 = getTabDelegate(0)
120 mouseClick(tab0, centerOf(tab0).x, centerOf(tab0).y, Qt.MiddleButton)182 clickItem(tab0, Qt.MiddleButton)
121 compare(tabsModel.count, i)183 compare(tabsModel.count, i)
122 }184 }
123 }185 }
124186
187 function test_mouse_right_click() {
188 // Right click pops up the contextual actions menu
189 populateTabs()
190 var menu = popupMenuOnTab(0)
191 verify(menu)
192 verify(menu.visible)
193 }
194
125 function test_mouse_wheel() {195 function test_mouse_wheel() {
126 // Wheel events cycle through open tabs196 // Wheel events cycle through open tabs
127 populateTabs()197 populateTabs()
@@ -141,6 +211,16 @@
141 compare(tabsModel.currentIndex, 1)211 compare(tabsModel.currentIndex, 1)
142 }212 }
143213
214 function test_close_tabs() {
215 populateTabs()
216 for (var i = 2; i >= 0; --i) {
217 var tab0 = getTabDelegate(0)
218 var closeButton = findChild(tab0, "closeButton")
219 clickItem(closeButton, Qt.LeftButton)
220 compare(tabsModel.count, i)
221 }
222 }
223
144 function test_drag_tab() {224 function test_drag_tab() {
145 populateTabs()225 populateTabs()
146226
@@ -169,5 +249,63 @@
169 tab = getTabDelegate(1)249 tab = getTabDelegate(1)
170 dragTab(tab, -tab.width * 2, 0)250 dragTab(tab, -tab.width * 2, 0)
171 }251 }
252
253 function test_menu_states_on_new_tab() {
254 populateTabs()
255 var menu = popupMenuOnTab(0)
256 var item = getMenuItemForAction(menu, "new_tab")
257 verify(item.enabled)
258 item = getMenuItemForAction(menu, "reload")
259 verify(!item.enabled)
260 item = getMenuItemForAction(menu, "close_tab")
261 verify(item.enabled)
262 }
263
264 function test_menu_states_on_page() {
265 tabs.appendTab("http://localhost/", "tab", "")
266 var menu = popupMenuOnTab(0)
267 var item = getMenuItemForAction(menu, "new_tab")
268 verify(item.enabled)
269 item = getMenuItemForAction(menu, "reload")
270 verify(item.enabled)
271 item = getMenuItemForAction(menu, "close_tab")
272 verify(item.enabled)
273 }
274
275 function test_context_menu_close() {
276 populateTabs()
277 var menu = popupMenuOnTab(1)
278 var item = getMenuItemForAction(menu, "close_tab")
279 clickItem(item)
280 compare(tabsModel.count, 2)
281 compare(tabsModel.get(0).title, "tab 0")
282 compare(tabsModel.get(1).title, "tab 2")
283 }
284
285 function test_context_menu_reload() {
286 var baseUrl = "http://localhost/"
287 tabs.appendTab(baseUrl + "1", "tab 1", "")
288 tabs.appendTab(baseUrl + "2", "tab 2", "")
289 var menu = popupMenuOnTab(1)
290 var item = getMenuItemForAction(menu, "reload")
291 clickItem(item)
292 compare(reloadSpy.count, 1)
293 compare(reloadSpy.signalArguments[0][0], baseUrl + "2")
294 }
295
296 function test_context_menu_new_tab() {
297 var baseUrl = "http://localhost/"
298 tabs.appendTab(baseUrl + "1", "tab 1", "")
299 tabs.appendTab(baseUrl + "2", "tab 2", "")
300 var menu = popupMenuOnTab(0)
301 var item = getMenuItemForAction(menu, "new_tab")
302 clickItem(item)
303 compare(newTabRequestSpy.count, 1)
304 compare(newTabRequestSpy.signalArguments[0][0], 1)
305 compare(tabsModel.count, 3)
306 compare(tabsModel.get(0).url, baseUrl + "1")
307 compare(tabsModel.get(1).url, "")
308 compare(tabsModel.get(2).url, baseUrl + "2")
309 }
172 }310 }
173}311}
174312
=== modified file 'tests/unittests/tabs-model/tst_TabsModelTests.cpp'
--- tests/unittests/tabs-model/tst_TabsModelTests.cpp 2015-09-02 16:48:01 +0000
+++ tests/unittests/tabs-model/tst_TabsModelTests.cpp 2015-09-28 08:10:23 +0000
@@ -17,6 +17,7 @@
17 */17 */
1818
19// Qt19// Qt
20#include <QtCore/QStringList>
20#include <QtQml/QQmlComponent>21#include <QtQml/QQmlComponent>
21#include <QtQml/QQmlEngine>22#include <QtQml/QQmlEngine>
22#include <QtQml/QQmlProperty>23#include <QtQml/QQmlProperty>
@@ -47,6 +48,20 @@
47 return item;48 return item;
48 }49 }
4950
51 QQuickItem* createTabWithTitle(const QString& title) {
52 QQuickItem* tab = createTab();
53 tab->setProperty("title", title);
54 return tab;
55 }
56
57 void verifyTabsOrder(QStringList orderedTitles) {
58 QCOMPARE(model->rowCount(), orderedTitles.count());
59 int i = 0;
60 Q_FOREACH(QString title, orderedTitles) {
61 QCOMPARE(model->get(i++)->property("title").toString(), title);
62 }
63 }
64
50private Q_SLOTS:65private Q_SLOTS:
51 void init()66 void init()
52 {67 {
@@ -113,6 +128,51 @@
113 QCOMPARE(model->rowCount(), 1);128 QCOMPARE(model->rowCount(), 1);
114 }129 }
115130
131 void shouldNotInsertNullTab()
132 {
133 QCOMPARE(model->insert(0, 0), -1);
134 QCOMPARE(model->rowCount(), 0);
135 }
136
137 void shouldReturnIndexWhenInsertingTab()
138 {
139 for(int i = 0; i < 3; ++i) {
140 model->add(createTab());
141 }
142 for(int i = 2; i >= 0; --i) {
143 QCOMPARE(model->insert(createTab(), i), i);
144 }
145 }
146
147 void shouldUpdateCountWhenInsertingTab()
148 {
149 QSignalSpy spy(model, SIGNAL(countChanged()));
150 model->insert(createTab(), 0);
151 QCOMPARE(spy.count(), 1);
152 QCOMPARE(model->rowCount(), 1);
153 }
154
155 void shouldInsertAtCorrectIndex()
156 {
157 model->insert(createTabWithTitle("B"), 0);
158 model->insert(createTabWithTitle("A"), 0);
159 verifyTabsOrder(QStringList({"A", "B"}));
160 model->insert(createTabWithTitle("X"), 1);
161 verifyTabsOrder(QStringList({"A", "X", "B"}));
162 model->insert(createTabWithTitle("C"), 3);
163 verifyTabsOrder(QStringList({"A", "X", "B", "C"}));
164 }
165
166 void shouldClampIndexWhenInsertingTabOutOfBounds()
167 {
168 model->add(createTabWithTitle("A"));
169 model->add(createTabWithTitle("B"));
170 model->insert(createTabWithTitle("C"), 3);
171 verifyTabsOrder(QStringList({"A", "B", "C"}));
172 model->insert(createTabWithTitle("X"), -1);
173 verifyTabsOrder(QStringList({"X", "A", "B", "C"}));
174 }
175
116 void shouldUpdateCountWhenRemovingTab()176 void shouldUpdateCountWhenRemovingTab()
117 {177 {
118 model->add(createTab());178 model->add(createTab());
@@ -138,19 +198,6 @@
138 delete removed;198 delete removed;
139 }199 }
140200
141 void shouldNotChangeCurrentTabWhenAddingUnlessModelWasEmpty()
142 {
143 QSignalSpy spy(model, SIGNAL(currentTabChanged()));
144 QQuickItem* tab = createTab();
145 model->add(tab);
146 QCOMPARE(spy.count(), 1);
147 QCOMPARE(model->currentTab(), tab);
148 spy.clear();
149 model->add(createTab());
150 QVERIFY(spy.isEmpty());
151 QCOMPARE(model->currentTab(), tab);
152 }
153
154 void shouldNotDeleteTabWhenRemoving()201 void shouldNotDeleteTabWhenRemoving()
155 {202 {
156 QQuickItem* tab = createTab();203 QQuickItem* tab = createTab();
@@ -244,64 +291,128 @@
244 QCOMPARE(model->currentTab(), tab2);291 QCOMPARE(model->currentTab(), tab2);
245 }292 }
246293
247 void shouldUpdateCurrentTabWhenRemoving()294 void shouldSetCurrentTabWhenAddingFirstTab()
248 {295 {
249 QSignalSpy tabSpy(model, SIGNAL(currentTabChanged()));296 // Adding a tab to an empty model should update the current tab
250 QSignalSpy indexSpy(model, SIGNAL(currentIndexChanged()));297 // to that tab
251298 QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
252 // Adding a tab to an empty model should update the current tab.299 QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
253 // Removing the last tab from the model should update it too.300 QCOMPARE(model->currentIndex(), -1);
254 model->add(createTab());301 QCOMPARE(model->currentTab(), (QObject*) nullptr);
255 tabSpy.clear();302
256 indexSpy.clear();303 QQuickItem* tab1 = createTab();
304 model->add(tab1);
305
306 QCOMPARE(spytab.count(), 1);
307 QCOMPARE(spyindex.count(), 1);
308 QCOMPARE(model->currentIndex(), 0);
309 QCOMPARE(model->currentTab(), tab1);
310
311 // But adding further items should keep the index where it was
312 model->add(createTab());
313 model->add(createTab());
314
315 QCOMPARE(spytab.count(), 1);
316 QCOMPARE(spyindex.count(), 1);
317 QCOMPARE(model->currentIndex(), 0);
318 QCOMPARE(model->currentTab(), tab1);
319 }
320
321 void shouldSetCurrentTabWhenInsertingFirstTab()
322 {
323 // Inserting a tab to an empty model should update the current tab
324 // to that tab
325 QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
326 QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
327 QCOMPARE(model->currentIndex(), -1);
328 QCOMPARE(model->currentTab(), (QObject*) nullptr);
329
330 QQuickItem* tab1 = createTab();
331 model->insert(tab1, 0);
332
333 QCOMPARE(spytab.count(), 1);
334 QCOMPARE(spyindex.count(), 1);
335 QCOMPARE(model->currentIndex(), 0);
336 QCOMPARE(model->currentTab(), tab1);
337 }
338
339 void shouldSetInvalidIndexWhenRemovingLastTab()
340 {
341 // Removing the last item should also set the current index to -1
342 // and the current tab to null
343 model->add(createTab());
344
345 QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
346 QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
257 delete model->remove(0);347 delete model->remove(0);
258 QCOMPARE(tabSpy.count(), 1);348 QCOMPARE(spytab.count(), 1);
259 QCOMPARE(indexSpy.count(), 1);349 QCOMPARE(spyindex.count(), 1);
350 QCOMPARE(model->currentIndex(), -1);
351 QCOMPARE(model->currentTab(), (QObject*) nullptr);
352 }
260353
261 // When removing a tab after the current one, neither the354 void shouldNotChangeIndexWhenRemovingAfterCurrent()
262 // current tab nor the current index should change.355 {
356 // When removing a tab after the current one,
357 // the current tab shouldn’t change.
263 QQuickItem* tab1 = createTab();358 QQuickItem* tab1 = createTab();
264 model->add(tab1);359 model->add(tab1);
265 model->add(createTab());360 model->add(createTab());
266 tabSpy.clear();361
267 indexSpy.clear();362 QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
363 QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
268 delete model->remove(1);364 delete model->remove(1);
269 QCOMPARE(model->currentTab(), tab1);365 QCOMPARE(model->currentTab(), tab1);
270 QVERIFY(tabSpy.isEmpty());366 QVERIFY(spytab.isEmpty());
271 QVERIFY(indexSpy.isEmpty());367 QVERIFY(spyindex.isEmpty());
368 }
272369
273 // When removing the current tab, if there is a tab after it, it370 void shouldUpdateIndexWhenRemovingCurrent()
274 // becomes the current one, and thus the current index doesn’t change.371 {
372 // When removing the current tab, if there is a tab after it,
373 // it becomes the current one.
374 QQuickItem* tab1 = createTab();
275 QQuickItem* tab2 = createTab();375 QQuickItem* tab2 = createTab();
276 model->add(tab2);
277 tabSpy.clear();
278 indexSpy.clear();
279 delete model->remove(0);
280 QCOMPARE(tabSpy.count(), 1);
281 QVERIFY(indexSpy.isEmpty());
282 QCOMPARE(model->currentTab(), tab2);
283
284 // When removing a tab before the current one, the current
285 // tab doesn’t change but the current index is updated.
286 QQuickItem* tab3 = createTab();376 QQuickItem* tab3 = createTab();
377 model->add(tab1);
378 model->add(tab2);
287 model->add(tab3);379 model->add(tab3);
288 model->setCurrentIndex(1);380 model->setCurrentIndex(1);
289 tabSpy.clear();381 QCOMPARE(model->currentIndex(), 1);
290 indexSpy.clear();382 QCOMPARE(model->currentTab(), tab2);
291 delete model->remove(0);383
292 QVERIFY(tabSpy.isEmpty());384 QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
293 QCOMPARE(indexSpy.count(), 1);385 QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
386 delete model->remove(1);
387 QCOMPARE(spyindex.count(), 0);
388 QCOMPARE(spytab.count(), 1);
389 QCOMPARE(model->currentTab(), tab3);
390
391 // If there is no tab after it but one before, that one becomes current
392 delete model->remove(1);
393 QCOMPARE(spyindex.count(), 1);
394 QCOMPARE(spytab.count(), 2);
294 QCOMPARE(model->currentIndex(), 0);395 QCOMPARE(model->currentIndex(), 0);
295396 QCOMPARE(model->currentTab(), tab1);
296 // When removing the current tab, if it was the last one, the397 }
297 // current tab should be reset to 0.398
298 tabSpy.clear();399 void shouldDecreaseIndexWhenRemovingBeforeCurrent()
299 indexSpy.clear();400 {
300 delete model->remove(0);401 // When removing a tab before the current tab, the current index
301 QCOMPARE(tabSpy.count(), 1);402 // should decrease to match.
302 QCOMPARE(indexSpy.count(), 1);403 model->add(createTab());
303 QCOMPARE(model->currentTab(), (QObject*) nullptr);404 model->add(createTab());
304 QCOMPARE(model->currentIndex(), -1);405 QQuickItem* tab = createTab();
406 model->add(tab);
407 model->setCurrentIndex(2);
408
409 QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
410 QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
411 delete model->remove(1);
412 QVERIFY(spytab.isEmpty());
413 QCOMPARE(spyindex.count(), 1);
414 QCOMPARE(model->currentIndex(), 1);
415 QCOMPARE(model->currentTab(), tab);
305 }416 }
306417
307 void shouldReturnData()418 void shouldReturnData()

Subscribers

People subscribed via source and target branches

to status/vote changes: