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

[Olivier Tilloy]

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.

[Olivier Tilloy (osomon)]

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.

[PS Jenkins bot (ps-jenkins)]

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)

[Riccardo Padovani (rpadovani)]

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

[PS Jenkins bot (ps-jenkins)]

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)

[Olivier Tilloy (osomon)]

> 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.

[PS Jenkins bot (ps-jenkins)]

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)

[Riccardo Padovani (rpadovani)]

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)]

Now is perfect, thanks so much!

review: Approve

[Ugo Riboni (uriboni)]

All looks good

review: Approve

=== 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 @@
                 onRemoved: if (chrome.bookmarked && (url === chrome.webview.url)) chrome.bookmarked = false
             }
 
-            onRequestNewTab: browser.openUrlInNewTab("", true)
+            onRequestNewTab: browser.openUrlInNewTab("", makeCurrent, true, index)
 
             onFindInPageModeChanged: if (!chrome.findInPageMode) internal.resetFocus()
 
@@ -1247,8 +1247,9 @@
             if (share) share.shareText(text)
         }
 
-        function addTab(tab, setCurrent) {
-            var index = tabsModel.add(tab)
+        function addTab(tab, setCurrent, index) {
+            if (index === undefined) index = tabsModel.add(tab)
+            else index = tabsModel.insert(tab, index)
             if (setCurrent) {
                 tabsModel.currentIndex = index
                 chrome.requestedUrl = tab.initialUrl
@@ -1351,10 +1352,10 @@
         }
     }
 
-    function openUrlInNewTab(url, setCurrent, load) {
+    function openUrlInNewTab(url, setCurrent, load, index) {
         load = typeof load !== 'undefined' ? load : true
         var tab = tabComponent.createObject(tabContainer, {"initialUrl": url, 'incognito': browser.incognito})
-        internal.addTab(tab, setCurrent)
+        internal.addTab(tab, setCurrent, index)
         if (load) {
             tabsModel.currentTab.load()
         }
 
=== 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 @@
         }
     }
 
+    function reload() {
+        if (webview) {
+            webview.reload()
+        } else {
+            load()
+        }
+    }
+
     function close() {
         unload()
         if (preview && preview.toString()) {
 
=== 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 @@
     property alias showFaviconInAddressBar: navigationBar.showFaviconInAddressBar
     readonly property alias bookmarkTogglePlaceHolder: navigationBar.bookmarkTogglePlaceHolder
 
-    signal requestNewTab()
+    signal requestNewTab(int index, bool makeCurrent)
 
     backgroundColor: incognito ? UbuntuColors.darkGrey : "#bcbcbc"
 
@@ -67,7 +67,7 @@
             sourceComponent: TabsBar {
                 model: tabsModel
                 incognito: chrome.incognito
-                onRequestNewTab: chrome.requestNewTab()
+                onRequestNewTab: chrome.requestNewTab(index, makeCurrent)
             }
 
             anchors {
 
=== 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 @@
     property bool incognito: false
     property bool active: false
     property bool hoverable: true
-    property int rightMargin: tabImage.anchors.rightMargin
+    property real rightMargin: 0
 
     property alias title: label.text
     property alias icon: favicon.source
 
+    property real dragMin: 0
+    property real dragMax: 0
+    readonly property bool dragging: mouseArea.drag.active
+
     signal selected()
     signal closed()
+    signal contextMenu()
 
     BorderImage {
         id: tabImage
@@ -83,14 +88,33 @@
         }
 
         MouseArea {
+            id: hoverArea
+            anchors.fill: parent
+            hoverEnabled: !tabItem.active && tabItem.hoverable
+        }
+
+        MouseArea {
+            id: mouseArea
             anchors.left: parent.left
             anchors.top: parent.top
             anchors.bottom: parent.bottom
             anchors.right: closeButton.left
-            onClicked: tabItem.selected()
+            acceptedButtons: Qt.AllButtons
+            onPressed: {
+                if (mouse.button === Qt.LeftButton) {
+                    tabItem.selected()
+                } else if (mouse.button === Qt.RightButton) {
+                    tabItem.contextMenu()
+                }
+            }
+            onClicked: {
+                if ((mouse.buttons === 0) && (mouse.button === Qt.MiddleButton)) {
+                    tabItem.closed()
+                }
+            }
         }
 
-        AbstractButton {
+        MouseArea {
             id: closeButton
             objectName: "closeButton"
 
@@ -108,17 +132,7 @@
                 name: "close"
             }
 
-            onTriggered: closed()
-        }
-
-        MouseArea {
-            id: hoverArea
-            anchors.fill: parent
-            hoverEnabled: !tabItem.active && tabItem.hoverable
-            propagateComposedEvents: true
-            acceptedButtons: Qt.MiddleButton
-            onClicked: tabItem.closed()
+            onClicked: closed()
         }
     }
 }
-
 
=== 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 @@
 
 import QtQuick 2.4
 import Ubuntu.Components 1.3
+import Ubuntu.Components.Popups 1.3
 import ".."
 
 Item {
@@ -31,7 +32,7 @@
 
     property bool incognito: false
 
-    signal requestNewTab()
+    signal requestNewTab(int index, bool makeCurrent)
 
     MouseArea {
         anchors.fill: parent
@@ -67,7 +68,36 @@
             color: incognito ? "white" : UbuntuColors.darkGrey
         }
 
-        onClicked: root.requestNewTab()
+        onClicked: root.requestNewTab(root.model.count, true)
+    }
+
+    Component {
+        id: contextualOptionsComponent
+        ActionSelectionPopover {
+            id: menu
+            objectName: "tabContextualActions"
+            property int targetIndex
+            readonly property var tab: root.model.get(targetIndex)
+
+            actions: ActionList {
+                Action {
+                    objectName: "tab_action_new_tab"
+                    text: i18n.tr("New Tab")
+                    onTriggered: root.requestNewTab(menu.targetIndex + 1, false)
+                }
+                Action {
+                    objectName: "tab_action_reload"
+                    text: i18n.tr("Reload")
+                    enabled: menu.tab.url.toString().length > 0
+                    onTriggered: menu.tab.reload()
+                }
+                Action {
+                    objectName: "tab_action_close_tab"
+                    text: i18n.tr("Close Tab")
+                    onTriggered: internal.closeTab(menu.targetIndex)
+                }
+            }
+        }
     }
 
     Item {
@@ -88,54 +118,58 @@
 
             property bool reordering: false
 
-            delegate: TabItem {
+            delegate: MouseArea {
                 id: tabDelegate
                 objectName: "tabDelegate"
+
                 readonly property int tabIndex: index
 
-                active: index === root.model.currentIndex
-                hoverable: true
-                incognito: root.incognito
-                title: model.title ? model.title : (model.url.toString() ? model.url : i18n.tr("New tab"))
-                icon: model.icon
-
                 anchors.top: tabsContainer.top
+                property real rightMargin: units.dp(1)
                 width: tabWidth + rightMargin
                 height: tabsContainer.height
-                rightMargin: units.dp(1)
-
-                onClosed: internal.closeTab(index)
-                onSelected: root.model.currentIndex = index
-
-                MouseArea {
-                    id: mouseArea
+
+                acceptedButtons: Qt.LeftButton | Qt.MiddleButton | Qt.RightButton
+                readonly property bool dragging: drag.active
+                drag {
+                    target: (pressedButtons === Qt.LeftButton) ? tabDelegate : null
+                    axis: Drag.XAxis
+                    minimumX: 0
+                    maximumX: root.width - tabDelegate.width
+                    filterChildren: true
+                }
+
+                TabItem {
                     anchors.fill: parent
-                    // XXX: should not start a drag when middle button was pressed
-                    drag {
-                        target: tabDelegate
-                        axis: Drag.XAxis
-                        minimumX: 0
-                        maximumX: root.width - tabDelegate.width
-                    }
-                    onReleased: root.model.currentIndex = index
-                    propagateComposedEvents: true
+
+                    active: tabIndex === root.model.currentIndex
+                    hoverable: true
+                    incognito: root.incognito
+                    title: model.title ? model.title : (model.url.toString() ? model.url : i18n.tr("New tab"))
+                    icon: model.icon
+
+                    rightMargin: tabDelegate.rightMargin
+
+                    onClosed: internal.closeTab(index)
+                    onSelected: root.model.currentIndex = index
+                    onContextMenu: PopupUtils.open(contextualOptionsComponent, tabDelegate, {"targetIndex": index})
                 }
 
                 Binding {
                     target: repeater
                     property: "reordering"
-                    value: mouseArea.drag.active
+                    value: dragging
                 }
 
                 Binding on x {
-                    when: !mouseArea.drag.active
+                    when: !dragging
                     value: index * width
                 }
 
                 Behavior on x { NumberAnimation { duration: 250 } }
 
                 onXChanged: {
-                    if (!mouseArea.drag.active) return
+                    if (!dragging) return
                     if (x < (index * width - width / 2)) {
                         root.model.move(index, index - 1)
                     } else if ((x > (index * width + width / 2)) && (index < (root.model.count - 1))) {
 
=== 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 @@
 // Qt
 #include <QtCore/QDebug>
 #include <QtCore/QObject>
+#include <QtCore/QtGlobal>
 
 /*!
     \class TabsModel
@@ -101,35 +102,58 @@
 
 QObject* TabsModel::currentTab() const
 {
-    if (m_tabs.isEmpty()) {
+    if (m_tabs.isEmpty() || !checkValidTabIndex(m_currentIndex)) {
         return nullptr;
     }
     return m_tabs.at(m_currentIndex);
 }
 
 /*!
-    Add a tab to the model and return the corresponding index in the model.
+    Append a tab to the model and return the corresponding index in the model.
 
     It is the responsibility of the caller to instantiate the corresponding
     Tab beforehand.
 */
 int TabsModel::add(QObject* tab)
 {
+    return insert(tab, m_tabs.count());
+}
+
+/*!
+    Add a tab to the model at the specified index, and return the index itself,
+    or -1 if the operation failed.
+
+    It is the responsibility of the caller to instantiate the corresponding
+    Tab beforehand.
+*/
+int TabsModel::insert(QObject* tab, int index)
+{
     if (tab == nullptr) {
         qWarning() << "Invalid Tab";
         return -1;
     }
-    int index = m_tabs.count();
+    index = qMax(qMin(index, m_tabs.count()), 0);
     beginInsertRows(QModelIndex(), index, index);
-    m_tabs.append(tab);
+    m_tabs.insert(index, tab);
     connect(tab, SIGNAL(urlChanged()), SLOT(onUrlChanged()));
     connect(tab, SIGNAL(titleChanged()), SLOT(onTitleChanged()));
     connect(tab, SIGNAL(iconChanged()), SLOT(onIconChanged()));
     endInsertRows();
     Q_EMIT countChanged();
-    if (index == 0) {
-        setCurrentIndex(0);
+
+    if (m_currentIndex == -1) {
+        // Set the index to zero if this is the first item that gets added to the
+        // model, as it should not be possible to have items in the model but no
+        // current tab.
+        m_currentIndex = 0;
+        Q_EMIT currentIndexChanged();
+        Q_EMIT currentTabChanged();
+    } else if (index <= m_currentIndex) {
+        // Increment the index if we are inserting items before the current index.
+        m_currentIndex++;
+        Q_EMIT currentIndexChanged();
     }
+
     return index;
 }
 
@@ -150,15 +174,21 @@
     tab->disconnect(this);
     endRemoveRows();
     Q_EMIT countChanged();
-    if (index == m_currentIndex) {
-        if (!checkValidTabIndex(index)) {
-            m_currentIndex = m_tabs.count() - 1;
+
+    if (index < m_currentIndex) {
+        // If we removed any tab before the current one, decrease the
+        // current index to match.
+        m_currentIndex--;
+        Q_EMIT currentIndexChanged();
+    } else if (index == m_currentIndex) {
+        // If the current tab was removed, the following one (if any) is made
+        // current. If it was the last tab in the model, the current index needs
+        // to be decreased.
+        if (m_currentIndex == m_tabs.count()) {
+            m_currentIndex--;
             Q_EMIT currentIndexChanged();
         }
         Q_EMIT currentTabChanged();
-    } else if (m_currentIndex > index) {
-        m_currentIndex -= 1;
-        Q_EMIT currentIndexChanged();
     }
     return tab;
 }
 
=== 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 @@
     QObject* currentTab() const;
 
     Q_INVOKABLE int add(QObject* tab);
+    Q_INVOKABLE int insert(QObject* tab, int index);
     Q_INVOKABLE QObject* remove(int index);
     Q_INVOKABLE QObject* get(int index) const;
     Q_INVOKABLE void move(int from, int to);
@@ -76,6 +77,7 @@
     int m_currentIndex;
 
     bool checkValidTabIndex(int index) const;
+    void setCurrentIndexNoCheck(int index);
     void onDataChanged(QObject* tab, int role);
 };
 
 
=== 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 @@
         self.pointing_device.click_object(button)
 
     def get_tabs(self):
-        return self.select_many("QQuickItem", objectName="tabDelegate")
+        return self.select_many("QQuickMouseArea", objectName="tabDelegate")
 
     def get_tab(self, index):
-        return self.select_single("QQuickItem", objectName="tabDelegate",
+        return self.select_single("QQuickMouseArea", objectName="tabDelegate",
                                   tabIndex=index)
 
     @autopilot.logging.log_action(logger.info)
@@ -342,7 +342,7 @@
     @autopilot.logging.log_action(logger.info)
     def close_tab(self, index):
         tab = self.get_tab(index)
-        close_button = tab.select_single("Icon", objectName="closeButton")
+        close_button = tab.select_single(objectName="closeButton")
         self.pointing_device.click_object(close_button)
 
 
 
=== 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 @@
+/*
+ * Copyright 2015 Canonical Ltd.
+ *
+ * This file is part of webbrowser-app.
+ *
+ * webbrowser-app is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * webbrowser-app is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+// Given the naming convention in QML for class names, we should
+// never have a class name with underscores in it, so the following
+// should be a safe way to remove the rest of the extra metatype
+// information produced by converting QML objects to strings.
+function qmlType(item) {
+    return String(item).split("_")[0]
+}
+
+function findDescendantsByType(item, type, list) {
+    list = list || []
+    if (qmlType(item) === type) list.push(item)
+    for (var i in item.children) {
+        findDescendantsByType(item.children[i], type, list)
+    }
+    return list
+}
+
+function findAncestorByType(item, type) {
+    while (item.parent) {
+        if (qmlType(item.parent) === type) return item.parent
+        item = item.parent
+    }
+    return null
+}
 
=== 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 @@
                 property url icon
                 property var request
                 property string currentState
+
+                property int reloaded: 0
+                function reload() { reloaded++ }
             }
             readonly property bool webviewPresent: webview
         }
@@ -76,6 +79,20 @@
             tab.destroy()
         }
 
+        function test_reload() {
+            var tab = tabComponent.createObject(root)
+            verify(!tab.webviewPresent)
+
+            tab.initialUrl = "http://example.org"
+            tab.reload()
+            tryCompare(tab, 'webviewPresent', true)
+            compare(tab.webview.reloaded, 0)
+
+            tab.reload()
+            verify(tab.webviewPresent)
+            compare(tab.webview.reloaded, 1)
+        }
+
         function test_create_with_request() {
             var tab = tabComponent.createObject(root, {'request': "foobar"})
             tryCompare(tab, 'webviewPresent', true)
 
=== 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 @@
 import Ubuntu.Test 1.0
 import "../../../src/app/webbrowser"
 import webbrowserapp.private 0.1
+import "qml_tree_helpers.js" as Tree
 
 Item {
     id: root
 
     width: 600
-    height: 50
+    height: 200
+    signal reload(string url)
 
     TabsModel {
         id: tabsModel
@@ -35,22 +37,35 @@
     Component {
         id: tabComponent
         QtObject {
+            id: tab
             property url url
             property string title
             property url icon
             function close() { destroy() }
+            function reload() { root.reload(tab.url) }
         }
     }
 
     TabsBar {
         id: tabs
-        anchors.fill: parent
+
+        // Make the tabs bar smaller than the window and aligned in the middle
+        // to leave room for the context menu to pop up and have all its items
+        // visible within the screen.
+        anchors.left: parent.left
+        anchors.right: parent.right
+        anchors.verticalCenter: parent.verticalCenter
+        height: 50
+
         model: tabsModel
-        onRequestNewTab: appendTab("", "", "")
+        onRequestNewTab: insertTab("", "", "", index)
         function appendTab(url, title, icon) {
+            insertTab(url, title, icon, model.count)
+            model.currentIndex = model.count - 1
+        }
+        function insertTab(url, title, icon, index) {
             var tab = tabComponent.createObject(root, {"url": url, "title": title, "icon": icon})
-            model.add(tab)
-            model.currentIndex = model.count - 1
+            model.insert(tab, index)
         }
     }
 
@@ -60,13 +75,46 @@
         signalName: "requestNewTab"
     }
 
+    SignalSpy {
+        id: reloadSpy
+        target: root
+        signalName: "reload"
+    }
+
+    // Ideally we would get menu items by their objectName, however they are
+    // created dynamically by the ActionSelectionPopover and the objectName
+    // of the source action is not copied to the generated item.
+    // So we first select the source Action by objectName then look for an item
+    // with the same text as the action. This is not ideal but it will work
+    // since we don't have items with the same text.
+    // https://launchpad.net/bugs/1205144 tracks the issue, and as of 2015-09-23
+    // is fixed in the vivid overlay PPA but not in wily yet.
+    function getMenuItemForAction(menu, actionName) {
+        actionName = "tab_action_" + actionName
+        var text = ""
+        var actions = menu.actions.actions
+        for (var i = 0; i < actions.length; i++) {
+            if (actions[i].objectName === actionName) {
+                text = actions[i].text
+                break
+            }
+        }
+        if (text === "") return null
+
+        var menuItems = Tree.findDescendantsByType(menu, "Label")
+        var matching = menuItems.filter(function(item) { return item.text === text })
+        if (matching.length === 0) return null
+        else return Tree.findAncestorByType(matching[0], "Empty")
+    }
+
     UbuntuTestCase {
         name: "TabsBar"
         when: windowShown
 
-        function clickItem(item) {
+        function clickItem(item, button) {
+            if (button === undefined) button = Qt.LeftButton
             var center = centerOf(item)
-            mouseClick(item, center.x, center.y)
+            mouseClick(item, center.x, center.y, button)
         }
 
         function getTabDelegate(index) {
@@ -80,11 +128,22 @@
             return null
         }
 
+        function popupMenuOnTab(index) {
+            var tab = getTabDelegate(index)
+            if (tab) {
+                clickItem(tab, Qt.RightButton)
+                var menu = findChild(root, "tabContextualActions")
+                waitForRendering(menu)
+                return menu
+            } else return null
+        }
+
         function cleanup() {
             while (tabsModel.count > 0) {
                 tabsModel.remove(0).destroy()
             }
             newTabRequestSpy.clear()
+            reloadSpy.clear()
         }
 
         function populateTabs() {
@@ -101,6 +160,9 @@
                 clickItem(newTabButton)
             }
             compare(newTabRequestSpy.count, 3)
+            compare(newTabRequestSpy.signalArguments[0][0], 0)
+            compare(newTabRequestSpy.signalArguments[1][0], 1)
+            compare(newTabRequestSpy.signalArguments[2][0], 2)
         }
 
         function test_mouse_left_click() {
@@ -117,11 +179,19 @@
             populateTabs()
             for (var i = 2; i >= 0; --i) {
                 var tab0 = getTabDelegate(0)
-                mouseClick(tab0, centerOf(tab0).x, centerOf(tab0).y, Qt.MiddleButton)
+                clickItem(tab0, Qt.MiddleButton)
                 compare(tabsModel.count, i)
             }
         }
 
+        function test_mouse_right_click() {
+            // Right click pops up the contextual actions menu
+            populateTabs()
+            var menu = popupMenuOnTab(0)
+            verify(menu)
+            verify(menu.visible)
+        }
+
         function test_mouse_wheel() {
             // Wheel events cycle through open tabs
             populateTabs()
@@ -141,6 +211,16 @@
             compare(tabsModel.currentIndex, 1)
         }
 
+        function test_close_tabs() {
+            populateTabs()
+            for (var i = 2; i >= 0; --i) {
+                var tab0 = getTabDelegate(0)
+                var closeButton = findChild(tab0, "closeButton")
+                clickItem(closeButton, Qt.LeftButton)
+                compare(tabsModel.count, i)
+            }
+        }
+
         function test_drag_tab() {
             populateTabs()
 
@@ -169,5 +249,63 @@
             tab = getTabDelegate(1)
             dragTab(tab, -tab.width * 2, 0)
         }
+
+        function test_menu_states_on_new_tab() {
+            populateTabs()
+            var menu = popupMenuOnTab(0)
+            var item = getMenuItemForAction(menu, "new_tab")
+            verify(item.enabled)
+            item = getMenuItemForAction(menu, "reload")
+            verify(!item.enabled)
+            item = getMenuItemForAction(menu, "close_tab")
+            verify(item.enabled)
+        }
+
+        function test_menu_states_on_page() {
+            tabs.appendTab("http://localhost/", "tab", "")
+            var menu = popupMenuOnTab(0)
+            var item = getMenuItemForAction(menu, "new_tab")
+            verify(item.enabled)
+            item = getMenuItemForAction(menu, "reload")
+            verify(item.enabled)
+            item = getMenuItemForAction(menu, "close_tab")
+            verify(item.enabled)
+        }
+
+        function test_context_menu_close() {
+            populateTabs()
+            var menu = popupMenuOnTab(1)
+            var item = getMenuItemForAction(menu, "close_tab")
+            clickItem(item)
+            compare(tabsModel.count, 2)
+            compare(tabsModel.get(0).title, "tab 0")
+            compare(tabsModel.get(1).title, "tab 2")
+        }
+
+        function test_context_menu_reload() {
+            var baseUrl = "http://localhost/"
+            tabs.appendTab(baseUrl + "1", "tab 1", "")
+            tabs.appendTab(baseUrl + "2", "tab 2", "")
+            var menu = popupMenuOnTab(1)
+            var item = getMenuItemForAction(menu, "reload")
+            clickItem(item)
+            compare(reloadSpy.count, 1)
+            compare(reloadSpy.signalArguments[0][0], baseUrl + "2")
+        }
+
+        function test_context_menu_new_tab() {
+            var baseUrl = "http://localhost/"
+            tabs.appendTab(baseUrl + "1", "tab 1", "")
+            tabs.appendTab(baseUrl + "2", "tab 2", "")
+            var menu = popupMenuOnTab(0)
+            var item = getMenuItemForAction(menu, "new_tab")
+            clickItem(item)
+            compare(newTabRequestSpy.count, 1)
+            compare(newTabRequestSpy.signalArguments[0][0], 1)
+            compare(tabsModel.count, 3)
+            compare(tabsModel.get(0).url, baseUrl + "1")
+            compare(tabsModel.get(1).url, "")
+            compare(tabsModel.get(2).url, baseUrl + "2")
+        }
     }
 }
 
=== 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 @@
  */
 
 // Qt
+#include <QtCore/QStringList>
 #include <QtQml/QQmlComponent>
 #include <QtQml/QQmlEngine>
 #include <QtQml/QQmlProperty>
@@ -47,6 +48,20 @@
         return item;
     }
 
+    QQuickItem* createTabWithTitle(const QString& title) {
+        QQuickItem* tab = createTab();
+        tab->setProperty("title", title);
+        return tab;
+    }
+
+    void verifyTabsOrder(QStringList orderedTitles) {
+        QCOMPARE(model->rowCount(), orderedTitles.count());
+        int i = 0;
+        Q_FOREACH(QString title, orderedTitles) {
+            QCOMPARE(model->get(i++)->property("title").toString(), title);
+        }
+    }
+
 private Q_SLOTS:
     void init()
     {
@@ -113,6 +128,51 @@
         QCOMPARE(model->rowCount(), 1);
     }
 
+    void shouldNotInsertNullTab()
+    {
+        QCOMPARE(model->insert(0, 0), -1);
+        QCOMPARE(model->rowCount(), 0);
+    }
+
+    void shouldReturnIndexWhenInsertingTab()
+    {
+        for(int i = 0; i < 3; ++i) {
+            model->add(createTab());
+        }
+        for(int i = 2; i >= 0; --i) {
+            QCOMPARE(model->insert(createTab(), i), i);
+        }
+    }
+
+    void shouldUpdateCountWhenInsertingTab()
+    {
+        QSignalSpy spy(model, SIGNAL(countChanged()));
+        model->insert(createTab(), 0);
+        QCOMPARE(spy.count(), 1);
+        QCOMPARE(model->rowCount(), 1);
+    }
+
+    void shouldInsertAtCorrectIndex()
+    {
+        model->insert(createTabWithTitle("B"), 0);
+        model->insert(createTabWithTitle("A"), 0);
+        verifyTabsOrder(QStringList({"A", "B"}));
+        model->insert(createTabWithTitle("X"), 1);
+        verifyTabsOrder(QStringList({"A", "X", "B"}));
+        model->insert(createTabWithTitle("C"), 3);
+        verifyTabsOrder(QStringList({"A", "X", "B", "C"}));
+    }
+
+    void shouldClampIndexWhenInsertingTabOutOfBounds()
+    {
+        model->add(createTabWithTitle("A"));
+        model->add(createTabWithTitle("B"));
+        model->insert(createTabWithTitle("C"), 3);
+        verifyTabsOrder(QStringList({"A", "B", "C"}));
+        model->insert(createTabWithTitle("X"), -1);
+        verifyTabsOrder(QStringList({"X", "A", "B", "C"}));
+    }
+
     void shouldUpdateCountWhenRemovingTab()
     {
         model->add(createTab());
@@ -138,19 +198,6 @@
         delete removed;
     }
 
-    void shouldNotChangeCurrentTabWhenAddingUnlessModelWasEmpty()
-    {
-        QSignalSpy spy(model, SIGNAL(currentTabChanged()));
-        QQuickItem* tab = createTab();
-        model->add(tab);
-        QCOMPARE(spy.count(), 1);
-        QCOMPARE(model->currentTab(), tab);
-        spy.clear();
-        model->add(createTab());
-        QVERIFY(spy.isEmpty());
-        QCOMPARE(model->currentTab(), tab);
-    }
-
     void shouldNotDeleteTabWhenRemoving()
     {
         QQuickItem* tab = createTab();
@@ -244,64 +291,128 @@
         QCOMPARE(model->currentTab(), tab2);
     }
 
-    void shouldUpdateCurrentTabWhenRemoving()
-    {
-        QSignalSpy tabSpy(model, SIGNAL(currentTabChanged()));
-        QSignalSpy indexSpy(model, SIGNAL(currentIndexChanged()));
-
-        // Adding a tab to an empty model should update the current tab.
-        // Removing the last tab from the model should update it too.
-        model->add(createTab());
-        tabSpy.clear();
-        indexSpy.clear();
+    void shouldSetCurrentTabWhenAddingFirstTab()
+    {
+        // Adding a tab to an empty model should update the current tab
+        // to that tab
+        QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
+        QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
+        QCOMPARE(model->currentIndex(), -1);
+        QCOMPARE(model->currentTab(), (QObject*) nullptr);
+
+        QQuickItem* tab1 = createTab();
+        model->add(tab1);
+
+        QCOMPARE(spytab.count(), 1);
+        QCOMPARE(spyindex.count(), 1);
+        QCOMPARE(model->currentIndex(), 0);
+        QCOMPARE(model->currentTab(), tab1);
+
+        // But adding further items should keep the index where it was
+        model->add(createTab());
+        model->add(createTab());
+
+        QCOMPARE(spytab.count(), 1);
+        QCOMPARE(spyindex.count(), 1);
+        QCOMPARE(model->currentIndex(), 0);
+        QCOMPARE(model->currentTab(), tab1);
+    }
+
+    void shouldSetCurrentTabWhenInsertingFirstTab()
+    {
+        // Inserting a tab to an empty model should update the current tab
+        // to that tab
+        QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
+        QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
+        QCOMPARE(model->currentIndex(), -1);
+        QCOMPARE(model->currentTab(), (QObject*) nullptr);
+
+        QQuickItem* tab1 = createTab();
+        model->insert(tab1, 0);
+
+        QCOMPARE(spytab.count(), 1);
+        QCOMPARE(spyindex.count(), 1);
+        QCOMPARE(model->currentIndex(), 0);
+        QCOMPARE(model->currentTab(), tab1);
+    }
+
+    void shouldSetInvalidIndexWhenRemovingLastTab()
+    {
+        // Removing the last item should also set the current index to -1
+        // and the current tab to null
+        model->add(createTab());
+
+        QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
+        QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
         delete model->remove(0);
-        QCOMPARE(tabSpy.count(), 1);
-        QCOMPARE(indexSpy.count(), 1);
+        QCOMPARE(spytab.count(), 1);
+        QCOMPARE(spyindex.count(), 1);
+        QCOMPARE(model->currentIndex(), -1);
+        QCOMPARE(model->currentTab(), (QObject*) nullptr);
+    }
 
-        // When removing a tab after the current one, neither the
-        // current tab nor the current index should change.
+    void shouldNotChangeIndexWhenRemovingAfterCurrent()
+    {
+        // When removing a tab after the current one,
+        // the current tab shouldn’t change.
         QQuickItem* tab1 = createTab();
         model->add(tab1);
         model->add(createTab());
-        tabSpy.clear();
-        indexSpy.clear();
+
+        QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
+        QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
         delete model->remove(1);
         QCOMPARE(model->currentTab(), tab1);
-        QVERIFY(tabSpy.isEmpty());
-        QVERIFY(indexSpy.isEmpty());
+        QVERIFY(spytab.isEmpty());
+        QVERIFY(spyindex.isEmpty());
+    }
 
-        // When removing the current tab, if there is a tab after it, it
-        // becomes the current one, and thus the current index doesn’t change.
+    void shouldUpdateIndexWhenRemovingCurrent()
+    {
+        // When removing the current tab, if there is a tab after it,
+        // it becomes the current one.
+        QQuickItem* tab1 = createTab();
         QQuickItem* tab2 = createTab();
-        model->add(tab2);
-        tabSpy.clear();
-        indexSpy.clear();
-        delete model->remove(0);
-        QCOMPARE(tabSpy.count(), 1);
-        QVERIFY(indexSpy.isEmpty());
-        QCOMPARE(model->currentTab(), tab2);
-
-        // When removing a tab before the current one, the current
-        // tab doesn’t change but the current index is updated.
         QQuickItem* tab3 = createTab();
+        model->add(tab1);
+        model->add(tab2);
         model->add(tab3);
         model->setCurrentIndex(1);
-        tabSpy.clear();
-        indexSpy.clear();
-        delete model->remove(0);
-        QVERIFY(tabSpy.isEmpty());
-        QCOMPARE(indexSpy.count(), 1);
+        QCOMPARE(model->currentIndex(), 1);
+        QCOMPARE(model->currentTab(), tab2);
+
+        QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
+        QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
+        delete model->remove(1);
+        QCOMPARE(spyindex.count(), 0);
+        QCOMPARE(spytab.count(), 1);
+        QCOMPARE(model->currentTab(), tab3);
+
+        // If there is no tab after it but one before, that one becomes current
+        delete model->remove(1);
+        QCOMPARE(spyindex.count(), 1);
+        QCOMPARE(spytab.count(), 2);
         QCOMPARE(model->currentIndex(), 0);
-
-        // When removing the current tab, if it was the last one, the
-        // current tab should be reset to 0.
-        tabSpy.clear();
-        indexSpy.clear();
-        delete model->remove(0);
-        QCOMPARE(tabSpy.count(), 1);
-        QCOMPARE(indexSpy.count(), 1);
-        QCOMPARE(model->currentTab(), (QObject*) nullptr);
-        QCOMPARE(model->currentIndex(), -1);
+        QCOMPARE(model->currentTab(), tab1);
+    }
+
+    void shouldDecreaseIndexWhenRemovingBeforeCurrent()
+    {
+        // When removing a tab before the current tab, the current index
+        // should decrease to match.
+        model->add(createTab());
+        model->add(createTab());
+        QQuickItem* tab = createTab();
+        model->add(tab);
+        model->setCurrentIndex(2);
+
+        QSignalSpy spytab(model, SIGNAL(currentTabChanged()));
+        QSignalSpy spyindex(model, SIGNAL(currentIndexChanged()));
+        delete model->remove(1);
+        QVERIFY(spytab.isEmpty());
+        QCOMPARE(spyindex.count(), 1);
+        QCOMPARE(model->currentIndex(), 1);
+        QCOMPARE(model->currentTab(), tab);
     }
 
     void shouldReturnData()