webbrowser-app

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

  1. uriboni-tab-context-menu
  2. Merge into trunk
Proposed by Olivier Tilloy
Status: Merged
Approved by: Olivier Tilloy
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) Approve
Riccardo Padovani (community) Approve
PS Jenkins bot continuous-integration Needs Fixing
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.
Revision history for this message
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.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1149
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2274/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4331
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1028/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1028
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1028/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1028/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3531
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4328
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4328/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23617

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2274/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
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
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1150
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2276/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4334
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1030/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1030
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1030/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1030/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3533
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4331
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4331/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23621

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2276/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
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.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1152
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2280/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4347
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1034/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1034
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1034/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1034/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3543
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4344
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4344/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23645

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2280/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
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
Revision history for this message
Riccardo Padovani (rpadovani) wrote :

Now is perfect, thanks so much!

review: Approve
Revision history for this message
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
1=== modified file 'src/app/webbrowser/Browser.qml'
2--- src/app/webbrowser/Browser.qml 2015-09-22 01:27:19 +0000
3+++ src/app/webbrowser/Browser.qml 2015-09-28 08:10:23 +0000
4@@ -353,7 +353,7 @@
5 onRemoved: if (chrome.bookmarked && (url === chrome.webview.url)) chrome.bookmarked = false
6 }
7
8- onRequestNewTab: browser.openUrlInNewTab("", true)
9+ onRequestNewTab: browser.openUrlInNewTab("", makeCurrent, true, index)
10
11 onFindInPageModeChanged: if (!chrome.findInPageMode) internal.resetFocus()
12
13@@ -1247,8 +1247,9 @@
14 if (share) share.shareText(text)
15 }
16
17- function addTab(tab, setCurrent) {
18- var index = tabsModel.add(tab)
19+ function addTab(tab, setCurrent, index) {
20+ if (index === undefined) index = tabsModel.add(tab)
21+ else index = tabsModel.insert(tab, index)
22 if (setCurrent) {
23 tabsModel.currentIndex = index
24 chrome.requestedUrl = tab.initialUrl
25@@ -1351,10 +1352,10 @@
26 }
27 }
28
29- function openUrlInNewTab(url, setCurrent, load) {
30+ function openUrlInNewTab(url, setCurrent, load, index) {
31 load = typeof load !== 'undefined' ? load : true
32 var tab = tabComponent.createObject(tabContainer, {"initialUrl": url, 'incognito': browser.incognito})
33- internal.addTab(tab, setCurrent)
34+ internal.addTab(tab, setCurrent, index)
35 if (load) {
36 tabsModel.currentTab.load()
37 }
38
39=== modified file 'src/app/webbrowser/BrowserTab.qml'
40--- src/app/webbrowser/BrowserTab.qml 2015-09-02 10:35:36 +0000
41+++ src/app/webbrowser/BrowserTab.qml 2015-09-28 08:10:23 +0000
42@@ -71,6 +71,14 @@
43 }
44 }
45
46+ function reload() {
47+ if (webview) {
48+ webview.reload()
49+ } else {
50+ load()
51+ }
52+ }
53+
54 function close() {
55 unload()
56 if (preview && preview.toString()) {
57
58=== modified file 'src/app/webbrowser/Chrome.qml'
59--- src/app/webbrowser/Chrome.qml 2015-09-17 09:15:51 +0000
60+++ src/app/webbrowser/Chrome.qml 2015-09-28 08:10:23 +0000
61@@ -39,7 +39,7 @@
62 property alias showFaviconInAddressBar: navigationBar.showFaviconInAddressBar
63 readonly property alias bookmarkTogglePlaceHolder: navigationBar.bookmarkTogglePlaceHolder
64
65- signal requestNewTab()
66+ signal requestNewTab(int index, bool makeCurrent)
67
68 backgroundColor: incognito ? UbuntuColors.darkGrey : "#bcbcbc"
69
70@@ -67,7 +67,7 @@
71 sourceComponent: TabsBar {
72 model: tabsModel
73 incognito: chrome.incognito
74- onRequestNewTab: chrome.requestNewTab()
75+ onRequestNewTab: chrome.requestNewTab(index, makeCurrent)
76 }
77
78 anchors {
79
80=== modified file 'src/app/webbrowser/TabItem.qml'
81--- src/app/webbrowser/TabItem.qml 2015-09-17 18:32:59 +0000
82+++ src/app/webbrowser/TabItem.qml 2015-09-28 08:10:23 +0000
83@@ -26,13 +26,18 @@
84 property bool incognito: false
85 property bool active: false
86 property bool hoverable: true
87- property int rightMargin: tabImage.anchors.rightMargin
88+ property real rightMargin: 0
89
90 property alias title: label.text
91 property alias icon: favicon.source
92
93+ property real dragMin: 0
94+ property real dragMax: 0
95+ readonly property bool dragging: mouseArea.drag.active
96+
97 signal selected()
98 signal closed()
99+ signal contextMenu()
100
101 BorderImage {
102 id: tabImage
103@@ -83,14 +88,33 @@
104 }
105
106 MouseArea {
107+ id: hoverArea
108+ anchors.fill: parent
109+ hoverEnabled: !tabItem.active && tabItem.hoverable
110+ }
111+
112+ MouseArea {
113+ id: mouseArea
114 anchors.left: parent.left
115 anchors.top: parent.top
116 anchors.bottom: parent.bottom
117 anchors.right: closeButton.left
118- onClicked: tabItem.selected()
119+ acceptedButtons: Qt.AllButtons
120+ onPressed: {
121+ if (mouse.button === Qt.LeftButton) {
122+ tabItem.selected()
123+ } else if (mouse.button === Qt.RightButton) {
124+ tabItem.contextMenu()
125+ }
126+ }
127+ onClicked: {
128+ if ((mouse.buttons === 0) && (mouse.button === Qt.MiddleButton)) {
129+ tabItem.closed()
130+ }
131+ }
132 }
133
134- AbstractButton {
135+ MouseArea {
136 id: closeButton
137 objectName: "closeButton"
138
139@@ -108,17 +132,7 @@
140 name: "close"
141 }
142
143- onTriggered: closed()
144- }
145-
146- MouseArea {
147- id: hoverArea
148- anchors.fill: parent
149- hoverEnabled: !tabItem.active && tabItem.hoverable
150- propagateComposedEvents: true
151- acceptedButtons: Qt.MiddleButton
152- onClicked: tabItem.closed()
153+ onClicked: closed()
154 }
155 }
156 }
157-
158
159=== modified file 'src/app/webbrowser/TabsBar.qml'
160--- src/app/webbrowser/TabsBar.qml 2015-09-04 17:48:37 +0000
161+++ src/app/webbrowser/TabsBar.qml 2015-09-28 08:10:23 +0000
162@@ -18,6 +18,7 @@
163
164 import QtQuick 2.4
165 import Ubuntu.Components 1.3
166+import Ubuntu.Components.Popups 1.3
167 import ".."
168
169 Item {
170@@ -31,7 +32,7 @@
171
172 property bool incognito: false
173
174- signal requestNewTab()
175+ signal requestNewTab(int index, bool makeCurrent)
176
177 MouseArea {
178 anchors.fill: parent
179@@ -67,7 +68,36 @@
180 color: incognito ? "white" : UbuntuColors.darkGrey
181 }
182
183- onClicked: root.requestNewTab()
184+ onClicked: root.requestNewTab(root.model.count, true)
185+ }
186+
187+ Component {
188+ id: contextualOptionsComponent
189+ ActionSelectionPopover {
190+ id: menu
191+ objectName: "tabContextualActions"
192+ property int targetIndex
193+ readonly property var tab: root.model.get(targetIndex)
194+
195+ actions: ActionList {
196+ Action {
197+ objectName: "tab_action_new_tab"
198+ text: i18n.tr("New Tab")
199+ onTriggered: root.requestNewTab(menu.targetIndex + 1, false)
200+ }
201+ Action {
202+ objectName: "tab_action_reload"
203+ text: i18n.tr("Reload")
204+ enabled: menu.tab.url.toString().length > 0
205+ onTriggered: menu.tab.reload()
206+ }
207+ Action {
208+ objectName: "tab_action_close_tab"
209+ text: i18n.tr("Close Tab")
210+ onTriggered: internal.closeTab(menu.targetIndex)
211+ }
212+ }
213+ }
214 }
215
216 Item {
217@@ -88,54 +118,58 @@
218
219 property bool reordering: false
220
221- delegate: TabItem {
222+ delegate: MouseArea {
223 id: tabDelegate
224 objectName: "tabDelegate"
225+
226 readonly property int tabIndex: index
227
228- active: index === root.model.currentIndex
229- hoverable: true
230- incognito: root.incognito
231- title: model.title ? model.title : (model.url.toString() ? model.url : i18n.tr("New tab"))
232- icon: model.icon
233-
234 anchors.top: tabsContainer.top
235+ property real rightMargin: units.dp(1)
236 width: tabWidth + rightMargin
237 height: tabsContainer.height
238- rightMargin: units.dp(1)
239-
240- onClosed: internal.closeTab(index)
241- onSelected: root.model.currentIndex = index
242-
243- MouseArea {
244- id: mouseArea
245+
246+ acceptedButtons: Qt.LeftButton | Qt.MiddleButton | Qt.RightButton
247+ readonly property bool dragging: drag.active
248+ drag {
249+ target: (pressedButtons === Qt.LeftButton) ? tabDelegate : null
250+ axis: Drag.XAxis
251+ minimumX: 0
252+ maximumX: root.width - tabDelegate.width
253+ filterChildren: true
254+ }
255+
256+ TabItem {
257 anchors.fill: parent
258- // XXX: should not start a drag when middle button was pressed
259- drag {
260- target: tabDelegate
261- axis: Drag.XAxis
262- minimumX: 0
263- maximumX: root.width - tabDelegate.width
264- }
265- onReleased: root.model.currentIndex = index
266- propagateComposedEvents: true
267+
268+ active: tabIndex === root.model.currentIndex
269+ hoverable: true
270+ incognito: root.incognito
271+ title: model.title ? model.title : (model.url.toString() ? model.url : i18n.tr("New tab"))
272+ icon: model.icon
273+
274+ rightMargin: tabDelegate.rightMargin
275+
276+ onClosed: internal.closeTab(index)
277+ onSelected: root.model.currentIndex = index
278+ onContextMenu: PopupUtils.open(contextualOptionsComponent, tabDelegate, {"targetIndex": index})
279 }
280
281 Binding {
282 target: repeater
283 property: "reordering"
284- value: mouseArea.drag.active
285+ value: dragging
286 }
287
288 Binding on x {
289- when: !mouseArea.drag.active
290+ when: !dragging
291 value: index * width
292 }
293
294 Behavior on x { NumberAnimation { duration: 250 } }
295
296 onXChanged: {
297- if (!mouseArea.drag.active) return
298+ if (!dragging) return
299 if (x < (index * width - width / 2)) {
300 root.model.move(index, index - 1)
301 } else if ((x > (index * width + width / 2)) && (index < (root.model.count - 1))) {
302
303=== modified file 'src/app/webbrowser/tabs-model.cpp'
304--- src/app/webbrowser/tabs-model.cpp 2015-09-02 16:48:01 +0000
305+++ src/app/webbrowser/tabs-model.cpp 2015-09-28 08:10:23 +0000
306@@ -21,6 +21,7 @@
307 // Qt
308 #include <QtCore/QDebug>
309 #include <QtCore/QObject>
310+#include <QtCore/QtGlobal>
311
312 /*!
313 \class TabsModel
314@@ -101,35 +102,58 @@
315
316 QObject* TabsModel::currentTab() const
317 {
318- if (m_tabs.isEmpty()) {
319+ if (m_tabs.isEmpty() || !checkValidTabIndex(m_currentIndex)) {
320 return nullptr;
321 }
322 return m_tabs.at(m_currentIndex);
323 }
324
325 /*!
326- Add a tab to the model and return the corresponding index in the model.
327+ Append a tab to the model and return the corresponding index in the model.
328
329 It is the responsibility of the caller to instantiate the corresponding
330 Tab beforehand.
331 */
332 int TabsModel::add(QObject* tab)
333 {
334+ return insert(tab, m_tabs.count());
335+}
336+
337+/*!
338+ Add a tab to the model at the specified index, and return the index itself,
339+ or -1 if the operation failed.
340+
341+ It is the responsibility of the caller to instantiate the corresponding
342+ Tab beforehand.
343+*/
344+int TabsModel::insert(QObject* tab, int index)
345+{
346 if (tab == nullptr) {
347 qWarning() << "Invalid Tab";
348 return -1;
349 }
350- int index = m_tabs.count();
351+ index = qMax(qMin(index, m_tabs.count()), 0);
352 beginInsertRows(QModelIndex(), index, index);
353- m_tabs.append(tab);
354+ m_tabs.insert(index, tab);
355 connect(tab, SIGNAL(urlChanged()), SLOT(onUrlChanged()));
356 connect(tab, SIGNAL(titleChanged()), SLOT(onTitleChanged()));
357 connect(tab, SIGNAL(iconChanged()), SLOT(onIconChanged()));
358 endInsertRows();
359 Q_EMIT countChanged();
360- if (index == 0) {
361- setCurrentIndex(0);
362+
363+ if (m_currentIndex == -1) {
364+ // Set the index to zero if this is the first item that gets added to the
365+ // model, as it should not be possible to have items in the model but no
366+ // current tab.
367+ m_currentIndex = 0;
368+ Q_EMIT currentIndexChanged();
369+ Q_EMIT currentTabChanged();
370+ } else if (index <= m_currentIndex) {
371+ // Increment the index if we are inserting items before the current index.
372+ m_currentIndex++;
373+ Q_EMIT currentIndexChanged();
374 }
375+
376 return index;
377 }
378
379@@ -150,15 +174,21 @@
380 tab->disconnect(this);
381 endRemoveRows();
382 Q_EMIT countChanged();
383- if (index == m_currentIndex) {
384- if (!checkValidTabIndex(index)) {
385- m_currentIndex = m_tabs.count() - 1;
386+
387+ if (index < m_currentIndex) {
388+ // If we removed any tab before the current one, decrease the
389+ // current index to match.
390+ m_currentIndex--;
391+ Q_EMIT currentIndexChanged();
392+ } else if (index == m_currentIndex) {
393+ // If the current tab was removed, the following one (if any) is made
394+ // current. If it was the last tab in the model, the current index needs
395+ // to be decreased.
396+ if (m_currentIndex == m_tabs.count()) {
397+ m_currentIndex--;
398 Q_EMIT currentIndexChanged();
399 }
400 Q_EMIT currentTabChanged();
401- } else if (m_currentIndex > index) {
402- m_currentIndex -= 1;
403- Q_EMIT currentIndexChanged();
404 }
405 return tab;
406 }
407
408=== modified file 'src/app/webbrowser/tabs-model.h'
409--- src/app/webbrowser/tabs-model.h 2015-06-18 14:28:11 +0000
410+++ src/app/webbrowser/tabs-model.h 2015-09-28 08:10:23 +0000
411@@ -57,6 +57,7 @@
412 QObject* currentTab() const;
413
414 Q_INVOKABLE int add(QObject* tab);
415+ Q_INVOKABLE int insert(QObject* tab, int index);
416 Q_INVOKABLE QObject* remove(int index);
417 Q_INVOKABLE QObject* get(int index) const;
418 Q_INVOKABLE void move(int from, int to);
419@@ -76,6 +77,7 @@
420 int m_currentIndex;
421
422 bool checkValidTabIndex(int index) const;
423+ void setCurrentIndexNoCheck(int index);
424 void onDataChanged(QObject* tab, int role);
425 };
426
427
428=== modified file 'tests/autopilot/webbrowser_app/emulators/browser.py'
429--- tests/autopilot/webbrowser_app/emulators/browser.py 2015-09-13 21:24:35 +0000
430+++ tests/autopilot/webbrowser_app/emulators/browser.py 2015-09-28 08:10:23 +0000
431@@ -329,10 +329,10 @@
432 self.pointing_device.click_object(button)
433
434 def get_tabs(self):
435- return self.select_many("QQuickItem", objectName="tabDelegate")
436+ return self.select_many("QQuickMouseArea", objectName="tabDelegate")
437
438 def get_tab(self, index):
439- return self.select_single("QQuickItem", objectName="tabDelegate",
440+ return self.select_single("QQuickMouseArea", objectName="tabDelegate",
441 tabIndex=index)
442
443 @autopilot.logging.log_action(logger.info)
444@@ -342,7 +342,7 @@
445 @autopilot.logging.log_action(logger.info)
446 def close_tab(self, index):
447 tab = self.get_tab(index)
448- close_button = tab.select_single("Icon", objectName="closeButton")
449+ close_button = tab.select_single(objectName="closeButton")
450 self.pointing_device.click_object(close_button)
451
452
453
454=== added file 'tests/unittests/qml/qml_tree_helpers.js'
455--- tests/unittests/qml/qml_tree_helpers.js 1970-01-01 00:00:00 +0000
456+++ tests/unittests/qml/qml_tree_helpers.js 2015-09-28 08:10:23 +0000
457@@ -0,0 +1,42 @@
458+/*
459+ * Copyright 2015 Canonical Ltd.
460+ *
461+ * This file is part of webbrowser-app.
462+ *
463+ * webbrowser-app is free software; you can redistribute it and/or modify
464+ * it under the terms of the GNU General Public License as published by
465+ * the Free Software Foundation; version 3.
466+ *
467+ * webbrowser-app is distributed in the hope that it will be useful,
468+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
469+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
470+ * GNU General Public License for more details.
471+ *
472+ * You should have received a copy of the GNU General Public License
473+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
474+ */
475+
476+// Given the naming convention in QML for class names, we should
477+// never have a class name with underscores in it, so the following
478+// should be a safe way to remove the rest of the extra metatype
479+// information produced by converting QML objects to strings.
480+function qmlType(item) {
481+ return String(item).split("_")[0]
482+}
483+
484+function findDescendantsByType(item, type, list) {
485+ list = list || []
486+ if (qmlType(item) === type) list.push(item)
487+ for (var i in item.children) {
488+ findDescendantsByType(item.children[i], type, list)
489+ }
490+ return list
491+}
492+
493+function findAncestorByType(item, type) {
494+ while (item.parent) {
495+ if (qmlType(item.parent) === type) return item.parent
496+ item = item.parent
497+ }
498+ return null
499+}
500
501=== modified file 'tests/unittests/qml/tst_BrowserTab.qml'
502--- tests/unittests/qml/tst_BrowserTab.qml 2015-08-10 15:22:00 +0000
503+++ tests/unittests/qml/tst_BrowserTab.qml 2015-09-28 08:10:23 +0000
504@@ -36,6 +36,9 @@
505 property url icon
506 property var request
507 property string currentState
508+
509+ property int reloaded: 0
510+ function reload() { reloaded++ }
511 }
512 readonly property bool webviewPresent: webview
513 }
514@@ -76,6 +79,20 @@
515 tab.destroy()
516 }
517
518+ function test_reload() {
519+ var tab = tabComponent.createObject(root)
520+ verify(!tab.webviewPresent)
521+
522+ tab.initialUrl = "http://example.org"
523+ tab.reload()
524+ tryCompare(tab, 'webviewPresent', true)
525+ compare(tab.webview.reloaded, 0)
526+
527+ tab.reload()
528+ verify(tab.webviewPresent)
529+ compare(tab.webview.reloaded, 1)
530+ }
531+
532 function test_create_with_request() {
533 var tab = tabComponent.createObject(root, {'request': "foobar"})
534 tryCompare(tab, 'webviewPresent', true)
535
536=== modified file 'tests/unittests/qml/tst_TabsBar.qml'
537--- tests/unittests/qml/tst_TabsBar.qml 2015-08-10 15:22:00 +0000
538+++ tests/unittests/qml/tst_TabsBar.qml 2015-09-28 08:10:23 +0000
539@@ -21,12 +21,14 @@
540 import Ubuntu.Test 1.0
541 import "../../../src/app/webbrowser"
542 import webbrowserapp.private 0.1
543+import "qml_tree_helpers.js" as Tree
544
545 Item {
546 id: root
547
548 width: 600
549- height: 50
550+ height: 200
551+ signal reload(string url)
552
553 TabsModel {
554 id: tabsModel
555@@ -35,22 +37,35 @@
556 Component {
557 id: tabComponent
558 QtObject {
559+ id: tab
560 property url url
561 property string title
562 property url icon
563 function close() { destroy() }
564+ function reload() { root.reload(tab.url) }
565 }
566 }
567
568 TabsBar {
569 id: tabs
570- anchors.fill: parent
571+
572+ // Make the tabs bar smaller than the window and aligned in the middle
573+ // to leave room for the context menu to pop up and have all its items
574+ // visible within the screen.
575+ anchors.left: parent.left
576+ anchors.right: parent.right
577+ anchors.verticalCenter: parent.verticalCenter
578+ height: 50
579+
580 model: tabsModel
581- onRequestNewTab: appendTab("", "", "")
582+ onRequestNewTab: insertTab("", "", "", index)
583 function appendTab(url, title, icon) {
584+ insertTab(url, title, icon, model.count)
585+ model.currentIndex = model.count - 1
586+ }
587+ function insertTab(url, title, icon, index) {
588 var tab = tabComponent.createObject(root, {"url": url, "title": title, "icon": icon})
589- model.add(tab)
590- model.currentIndex = model.count - 1
591+ model.insert(tab, index)
592 }
593 }
594
595@@ -60,13 +75,46 @@
596 signalName: "requestNewTab"
597 }
598
599+ SignalSpy {
600+ id: reloadSpy
601+ target: root
602+ signalName: "reload"
603+ }
604+
605+ // Ideally we would get menu items by their objectName, however they are
606+ // created dynamically by the ActionSelectionPopover and the objectName
607+ // of the source action is not copied to the generated item.
608+ // So we first select the source Action by objectName then look for an item
609+ // with the same text as the action. This is not ideal but it will work
610+ // since we don't have items with the same text.
611+ // https://launchpad.net/bugs/1205144 tracks the issue, and as of 2015-09-23
612+ // is fixed in the vivid overlay PPA but not in wily yet.
613+ function getMenuItemForAction(menu, actionName) {
614+ actionName = "tab_action_" + actionName
615+ var text = ""
616+ var actions = menu.actions.actions
617+ for (var i = 0; i < actions.length; i++) {
618+ if (actions[i].objectName === actionName) {
619+ text = actions[i].text
620+ break
621+ }
622+ }
623+ if (text === "") return null
624+
625+ var menuItems = Tree.findDescendantsByType(menu, "Label")
626+ var matching = menuItems.filter(function(item) { return item.text === text })
627+ if (matching.length === 0) return null
628+ else return Tree.findAncestorByType(matching[0], "Empty")
629+ }
630+
631 UbuntuTestCase {
632 name: "TabsBar"
633 when: windowShown
634
635- function clickItem(item) {
636+ function clickItem(item, button) {
637+ if (button === undefined) button = Qt.LeftButton
638 var center = centerOf(item)
639- mouseClick(item, center.x, center.y)
640+ mouseClick(item, center.x, center.y, button)
641 }
642
643 function getTabDelegate(index) {
644@@ -80,11 +128,22 @@
645 return null
646 }
647
648+ function popupMenuOnTab(index) {
649+ var tab = getTabDelegate(index)
650+ if (tab) {
651+ clickItem(tab, Qt.RightButton)
652+ var menu = findChild(root, "tabContextualActions")
653+ waitForRendering(menu)
654+ return menu
655+ } else return null
656+ }
657+
658 function cleanup() {
659 while (tabsModel.count > 0) {
660 tabsModel.remove(0).destroy()
661 }
662 newTabRequestSpy.clear()
663+ reloadSpy.clear()
664 }
665
666 function populateTabs() {
667@@ -101,6 +160,9 @@
668 clickItem(newTabButton)
669 }
670 compare(newTabRequestSpy.count, 3)
671+ compare(newTabRequestSpy.signalArguments[0][0], 0)
672+ compare(newTabRequestSpy.signalArguments[1][0], 1)
673+ compare(newTabRequestSpy.signalArguments[2][0], 2)
674 }
675
676 function test_mouse_left_click() {
677@@ -117,11 +179,19 @@
678 populateTabs()
679 for (var i = 2; i >= 0; --i) {
680 var tab0 = getTabDelegate(0)
681- mouseClick(tab0, centerOf(tab0).x, centerOf(tab0).y, Qt.MiddleButton)
682+ clickItem(tab0, Qt.MiddleButton)
683 compare(tabsModel.count, i)
684 }
685 }
686
687+ function test_mouse_right_click() {
688+ // Right click pops up the contextual actions menu
689+ populateTabs()
690+ var menu = popupMenuOnTab(0)
691+ verify(menu)
692+ verify(menu.visible)
693+ }
694+
695 function test_mouse_wheel() {
696 // Wheel events cycle through open tabs
697 populateTabs()
698@@ -141,6 +211,16 @@
699 compare(tabsModel.currentIndex, 1)
700 }
701
702+ function test_close_tabs() {
703+ populateTabs()
704+ for (var i = 2; i >= 0; --i) {
705+ var tab0 = getTabDelegate(0)
706+ var closeButton = findChild(tab0, "closeButton")
707+ clickItem(closeButton, Qt.LeftButton)
708+ compare(tabsModel.count, i)
709+ }
710+ }
711+
712 function test_drag_tab() {
713 populateTabs()
714
715@@ -169,5 +249,63 @@
716 tab = getTabDelegate(1)
717 dragTab(tab, -tab.width * 2, 0)
718 }
719+
720+ function test_menu_states_on_new_tab() {
721+ populateTabs()
722+ var menu = popupMenuOnTab(0)
723+ var item = getMenuItemForAction(menu, "new_tab")
724+ verify(item.enabled)
725+ item = getMenuItemForAction(menu, "reload")
726+ verify(!item.enabled)
727+ item = getMenuItemForAction(menu, "close_tab")
728+ verify(item.enabled)
729+ }
730+
731+ function test_menu_states_on_page() {
732+ tabs.appendTab("http://localhost/", "tab", "")
733+ var menu = popupMenuOnTab(0)
734+ var item = getMenuItemForAction(menu, "new_tab")
735+ verify(item.enabled)
736+ item = getMenuItemForAction(menu, "reload")
737+ verify(item.enabled)
738+ item = getMenuItemForAction(menu, "close_tab")
739+ verify(item.enabled)
740+ }
741+
742+ function test_context_menu_close() {
743+ populateTabs()
744+ var menu = popupMenuOnTab(1)
745+ var item = getMenuItemForAction(menu, "close_tab")
746+ clickItem(item)
747+ compare(tabsModel.count, 2)
748+ compare(tabsModel.get(0).title, "tab 0")
749+ compare(tabsModel.get(1).title, "tab 2")
750+ }
751+
752+ function test_context_menu_reload() {
753+ var baseUrl = "http://localhost/"
754+ tabs.appendTab(baseUrl + "1", "tab 1", "")
755+ tabs.appendTab(baseUrl + "2", "tab 2", "")
756+ var menu = popupMenuOnTab(1)
757+ var item = getMenuItemForAction(menu, "reload")
758+ clickItem(item)
759+ compare(reloadSpy.count, 1)
760+ compare(reloadSpy.signalArguments[0][0], baseUrl + "2")
761+ }
762+
763+ function test_context_menu_new_tab() {
764+ var baseUrl = "http://localhost/"
765+ tabs.appendTab(baseUrl + "1", "tab 1", "")
766+ tabs.appendTab(baseUrl + "2", "tab 2", "")
767+ var menu = popupMenuOnTab(0)
768+ var item = getMenuItemForAction(menu, "new_tab")
769+ clickItem(item)
770+ compare(newTabRequestSpy.count, 1)
771+ compare(newTabRequestSpy.signalArguments[0][0], 1)
772+ compare(tabsModel.count, 3)
773+ compare(tabsModel.get(0).url, baseUrl + "1")
774+ compare(tabsModel.get(1).url, "")
775+ compare(tabsModel.get(2).url, baseUrl + "2")
776+ }
777 }
778 }
779
780=== modified file 'tests/unittests/tabs-model/tst_TabsModelTests.cpp'
781--- tests/unittests/tabs-model/tst_TabsModelTests.cpp 2015-09-02 16:48:01 +0000
782+++ tests/unittests/tabs-model/tst_TabsModelTests.cpp 2015-09-28 08:10:23 +0000
783@@ -17,6 +17,7 @@
784 */
785
786 // Qt
787+#include <QtCore/QStringList>
788 #include <QtQml/QQmlComponent>
789 #include <QtQml/QQmlEngine>
790 #include <QtQml/QQmlProperty>
791@@ -47,6 +48,20 @@
792 return item;
793 }
794
795+ QQuickItem* createTabWithTitle(const QString& title) {
796+ QQuickItem* tab = createTab();
797+ tab->setProperty("title", title);
798+ return tab;
799+ }
800+
801+ void verifyTabsOrder(QStringList orderedTitles) {
802+ QCOMPARE(model->rowCount(), orderedTitles.count());
803+ int i = 0;
804+ Q_FOREACH(QString title, orderedTitles) {
805+ QCOMPARE(model->get(i++)->property("title").toString(), title);
806+ }
807+ }
808+
809 private Q_SLOTS:
810 void init()
811 {
812@@ -113,6 +128,51 @@
813 QCOMPARE(model->rowCount(), 1);
814 }
815
816+ void shouldNotInsertNullTab()
817+ {
818+ QCOMPARE(model->insert(0, 0), -1);
819+ QCOMPARE(model->rowCount(), 0);
820+ }
821+
822+ void shouldReturnIndexWhenInsertingTab()
823+ {
824+ for(int i = 0; i < 3; ++i) {
825+ model->add(createTab());
826+ }
827+ for(int i = 2; i >= 0; --i) {
828+ QCOMPARE(model->insert(createTab(), i), i);
829+ }
830+ }
831+
832+ void shouldUpdateCountWhenInsertingTab()
833+ {
834+ QSignalSpy spy(model, SIGNAL(countChanged()));
835+ model->insert(createTab(), 0);
836+ QCOMPARE(spy.count(), 1);
837+ QCOMPARE(model->rowCount(), 1);
838+ }
839+
840+ void shouldInsertAtCorrectIndex()
841+ {
842+ model->insert(createTabWithTitle("B"), 0);
843+ model->insert(createTabWithTitle("A"), 0);
844+ verifyTabsOrder(QStringList({"A", "B"}));
845+ model->insert(createTabWithTitle("X"), 1);
846+ verifyTabsOrder(QStringList({"A", "X", "B"}));
847+ model->insert(createTabWithTitle("C"), 3);
848+ verifyTabsOrder(QStringList({"A", "X", "B", "C"}));
849+ }
850+
851+ void shouldClampIndexWhenInsertingTabOutOfBounds()
852+ {
853+ model->add(createTabWithTitle("A"));
854+ model->add(createTabWithTitle("B"));
855+ model->insert(createTabWithTitle("C"), 3);
856+ verifyTabsOrder(QStringList({"A", "B", "C"}));
857+ model->insert(createTabWithTitle("X"), -1);
858+ verifyTabsOrder(QStringList({"X", "A", "B", "C"}));
859+ }
860+
861 void shouldUpdateCountWhenRemovingTab()
862 {
863 model->add(createTab());
864@@ -138,19 +198,6 @@
865 delete removed;
866 }
867
868- void shouldNotChangeCurrentTabWhenAddingUnlessModelWasEmpty()
869- {
870- QSignalSpy spy(model, SIGNAL(currentTabChanged()));
871- QQuickItem* tab = createTab();
872- model->add(tab);
873- QCOMPARE(spy.count(), 1);
874- QCOMPARE(model->currentTab(), tab);
875- spy.clear();
876- model->add(createTab());
877- QVERIFY(spy.isEmpty());
878- QCOMPARE(model->currentTab(), tab);
879- }
880-
881 void shouldNotDeleteTabWhenRemoving()
882 {
883 QQuickItem* tab = createTab();
884@@ -244,64 +291,128 @@
885 QCOMPARE(model->currentTab(), tab2);
886 }
887
888- void shouldUpdateCurrentTabWhenRemoving()
889- {
890- QSignalSpy tabSpy(model, SIGNAL(currentTabChanged()));
891- QSignalSpy indexSpy(model, SIGNAL(currentIndexChanged()));
892-
893- // Adding a tab to an empty model should update the current tab.
894- // Removing the last tab from the model should update it too.
895- model->add(createTab());
896- tabSpy.clear();
897- indexSpy.clear();
898+ void shouldSetCurrentTabWhenAddingFirstTab()
899+ {
900+ // Adding a tab to an empty model should update the current tab
901+ // to that tab
902+ QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
903+ QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
904+ QCOMPARE(model->currentIndex(), -1);
905+ QCOMPARE(model->currentTab(), (QObject*) nullptr);
906+
907+ QQuickItem* tab1 = createTab();
908+ model->add(tab1);
909+
910+ QCOMPARE(spytab.count(), 1);
911+ QCOMPARE(spyindex.count(), 1);
912+ QCOMPARE(model->currentIndex(), 0);
913+ QCOMPARE(model->currentTab(), tab1);
914+
915+ // But adding further items should keep the index where it was
916+ model->add(createTab());
917+ model->add(createTab());
918+
919+ QCOMPARE(spytab.count(), 1);
920+ QCOMPARE(spyindex.count(), 1);
921+ QCOMPARE(model->currentIndex(), 0);
922+ QCOMPARE(model->currentTab(), tab1);
923+ }
924+
925+ void shouldSetCurrentTabWhenInsertingFirstTab()
926+ {
927+ // Inserting a tab to an empty model should update the current tab
928+ // to that tab
929+ QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
930+ QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
931+ QCOMPARE(model->currentIndex(), -1);
932+ QCOMPARE(model->currentTab(), (QObject*) nullptr);
933+
934+ QQuickItem* tab1 = createTab();
935+ model->insert(tab1, 0);
936+
937+ QCOMPARE(spytab.count(), 1);
938+ QCOMPARE(spyindex.count(), 1);
939+ QCOMPARE(model->currentIndex(), 0);
940+ QCOMPARE(model->currentTab(), tab1);
941+ }
942+
943+ void shouldSetInvalidIndexWhenRemovingLastTab()
944+ {
945+ // Removing the last item should also set the current index to -1
946+ // and the current tab to null
947+ model->add(createTab());
948+
949+ QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
950+ QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
951 delete model->remove(0);
952- QCOMPARE(tabSpy.count(), 1);
953- QCOMPARE(indexSpy.count(), 1);
954+ QCOMPARE(spytab.count(), 1);
955+ QCOMPARE(spyindex.count(), 1);
956+ QCOMPARE(model->currentIndex(), -1);
957+ QCOMPARE(model->currentTab(), (QObject*) nullptr);
958+ }
959
960- // When removing a tab after the current one, neither the
961- // current tab nor the current index should change.
962+ void shouldNotChangeIndexWhenRemovingAfterCurrent()
963+ {
964+ // When removing a tab after the current one,
965+ // the current tab shouldn’t change.
966 QQuickItem* tab1 = createTab();
967 model->add(tab1);
968 model->add(createTab());
969- tabSpy.clear();
970- indexSpy.clear();
971+
972+ QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
973+ QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
974 delete model->remove(1);
975 QCOMPARE(model->currentTab(), tab1);
976- QVERIFY(tabSpy.isEmpty());
977- QVERIFY(indexSpy.isEmpty());
978+ QVERIFY(spytab.isEmpty());
979+ QVERIFY(spyindex.isEmpty());
980+ }
981
982- // When removing the current tab, if there is a tab after it, it
983- // becomes the current one, and thus the current index doesn’t change.
984+ void shouldUpdateIndexWhenRemovingCurrent()
985+ {
986+ // When removing the current tab, if there is a tab after it,
987+ // it becomes the current one.
988+ QQuickItem* tab1 = createTab();
989 QQuickItem* tab2 = createTab();
990- model->add(tab2);
991- tabSpy.clear();
992- indexSpy.clear();
993- delete model->remove(0);
994- QCOMPARE(tabSpy.count(), 1);
995- QVERIFY(indexSpy.isEmpty());
996- QCOMPARE(model->currentTab(), tab2);
997-
998- // When removing a tab before the current one, the current
999- // tab doesn’t change but the current index is updated.
1000 QQuickItem* tab3 = createTab();
1001+ model->add(tab1);
1002+ model->add(tab2);
1003 model->add(tab3);
1004 model->setCurrentIndex(1);
1005- tabSpy.clear();
1006- indexSpy.clear();
1007- delete model->remove(0);
1008- QVERIFY(tabSpy.isEmpty());
1009- QCOMPARE(indexSpy.count(), 1);
1010+ QCOMPARE(model->currentIndex(), 1);
1011+ QCOMPARE(model->currentTab(), tab2);
1012+
1013+ QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
1014+ QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
1015+ delete model->remove(1);
1016+ QCOMPARE(spyindex.count(), 0);
1017+ QCOMPARE(spytab.count(), 1);
1018+ QCOMPARE(model->currentTab(), tab3);
1019+
1020+ // If there is no tab after it but one before, that one becomes current
1021+ delete model->remove(1);
1022+ QCOMPARE(spyindex.count(), 1);
1023+ QCOMPARE(spytab.count(), 2);
1024 QCOMPARE(model->currentIndex(), 0);
1025-
1026- // When removing the current tab, if it was the last one, the
1027- // current tab should be reset to 0.
1028- tabSpy.clear();
1029- indexSpy.clear();
1030- delete model->remove(0);
1031- QCOMPARE(tabSpy.count(), 1);
1032- QCOMPARE(indexSpy.count(), 1);
1033- QCOMPARE(model->currentTab(), (QObject*) nullptr);
1034- QCOMPARE(model->currentIndex(), -1);
1035+ QCOMPARE(model->currentTab(), tab1);
1036+ }
1037+
1038+ void shouldDecreaseIndexWhenRemovingBeforeCurrent()
1039+ {
1040+ // When removing a tab before the current tab, the current index
1041+ // should decrease to match.
1042+ model->add(createTab());
1043+ model->add(createTab());
1044+ QQuickItem* tab = createTab();
1045+ model->add(tab);
1046+ model->setCurrentIndex(2);
1047+
1048+ QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
1049+ QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
1050+ delete model->remove(1);
1051+ QVERIFY(spytab.isEmpty());
1052+ QCOMPARE(spyindex.count(), 1);
1053+ QCOMPARE(model->currentIndex(), 1);
1054+ QCOMPARE(model->currentTab(), tab);
1055 }
1056
1057 void shouldReturnData()

Subscribers

People subscribed via source and target branches

to status/vote changes: