[Merge] lp:~ahayzen/webbrowser-app/migrate-to-tabs-component into lp:webbrowser-app/staging

[Olivier Tilloy]

Mostly good to go, see two comments inline.

Diff comments:

> 
> === added file 'src/app/webbrowser/TabsBar.qml'
> --- src/app/webbrowser/TabsBar.qml	1970-01-01 00:00:00 +0000
> +++ src/app/webbrowser/TabsBar.qml	2017-02-14 10:13:19 +0000
> @@ -0,0 +1,133 @@
> +/*
> + * Copyright 2013-2017 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/>.
> + */
> +
> +import QtQuick 2.4
> +import Ubuntu.Components 1.3
> +import Ubuntu.Components.Extras 0.3 as Extras
> +import Ubuntu.Components.Popups 1.3
> +import "."
> +import ".."
> +import webbrowsercommon.private 0.1
> +
> +Extras.TabsBar {
> +    id: tabsBar
> +    color: "#D9D9D9"  // FIXME: not in palette hardcode for now
> +    dragAndDrop {
> +        enabled: __platformName != "ubuntumirclient"
> +        maxYDiff: height / 12
> +        mimeType: "webbrowser/tab-" + (incognito ? "incognito" : "public")
> +        previewUrlFromIndex: function(index) {
> +            if (tabsBar.model.get(index)) {
> +                return PreviewManager.previewPathFromUrl(tabsBar.model.get(index).url)
> +            } else {
> +                return "";
> +            }
> +        }
> +    }
> +    fallbackIcon: "stock_website"
> +    windowFactoryProperties: {
> +        "incognito": tabsBar.incognito,
> +        "height": window.height,
> +        "width": window.width,
> +    }
> +
> +    property bool incognito
> +
> +    signal requestNewTab(int index, bool makeCurrent)
> +    signal tabClosed(int index, bool moving)
> +
> +    onContextMenu: PopupUtils.open(contextualOptionsComponent, tabDelegate, {"targetIndex": index})
> +
> +    Component {
> +        id: faviconFactory
> +        FaviconFetcher {
> +
> +        }
> +    }
> +
> +    function iconSourceFromModelItem(modelData, index) {
> +        var incubator = faviconFactory.incubateObject(
> +            tabsBar,
> +            {
> +                "shouldCache": Qt.binding(function() { return !incognito; }),
> +                "url": Qt.binding(function() { return modelData.icon || ""; })
> +            }
> +        );
> +        return incubator.status == Component.Ready ? incubator.object.localUrl || "" : "";

See documentation: https://doc.qt.io/qt-5/qml-qtqml-component.html#incubateObject-method.
There is no guarantee that the incubator will be ready by the time of the return statement. In fact if it works here it’s by accident. It seems to me the only clean solution here is to use faviconFactory.createObject(…).

One side question: is iconSourceFromModelItem() potentially called more than once for every tab? If so, the instantiated FaviconFetcher should be associated to the tab and re-used for subsequent calls. If we are guaranteed that it won’t be reused, then it should be deleted before the return statement.

> +    }
> +
> +    function titleFromModelItem(modelItem) {
> +        return modelItem.title ? modelItem.title : (modelItem.url.toString() ? modelItem.url : i18n.tr("New tab"))
> +    }
> +
> +    actions: [
> +        Action {
> +            // FIXME: icon from theme is fuzzy at many GUs
> +//                     iconSource: Qt.resolvedUrl("Tabs/tab_add.png")
> +            iconName: "add"
> +            objectName: "newTabButton"
> +            onTriggered: tabsBar.model.addTab()
> +        }
> +    ]
> +
> +    Component {
> +        id: contextualOptionsComponent
> +        ActionSelectionPopover {
> +            id: menu
> +            objectName: "tabContextualActions"
> +            property int targetIndex
> +            readonly property var tab: tabsBar.model.get(targetIndex)
> +
> +            actions: ActionList {
> +                Action {
> +                    objectName: "tab_action_new_tab"
> +                    text: i18n.tr("New Tab")
> +                    onTriggered: tabsBar.requestNewTab(menu.targetIndex + 1, true)
> +                }
> +                Action {
> +                    objectName: "tab_action_reload"
> +                    text: i18n.tr("Reload")
> +                    enabled: menu.tab.url.toString().length > 0
> +                    onTriggered: menu.tab.reload()
> +                }
> +                Action {
> +                    objectName: "tab_action_move_to_new_window"
> +                    text: i18n.tr("Move to New Window")
> +                    onTriggered: {
> +                        // callback function only removes from model
> +                        // and not destroy as webview is in new window
> +                        // Create new window and add existing tab
> +                        var window = tabsBar.windowFactory.createObject(null, windowFactoryProperties);
> +                        window.model.addExistingTab(menu.tab);
> +                        window.model.selectTab(window.model.count - 1);
> +                        window.show();
> +
> +                        // Just remove from model and do not destroy
> +                        // as webview is used in other window
> +                        tabsBar.model.removeTabWithoutDestroying(menu.targetIndex);
> +                    }
> +                }
> +                Action {
> +                    objectName: "tab_action_close_tab"
> +                    text: i18n.tr("Close Tab")
> +                    onTriggered: tabsBar.tabClosed(menu.targetIndex, false)
> +                }
> +            }
> +        }
> +    }
> +}
> 
> === modified file 'tests/unittests/qml/tst_TabsBar.qml'
> --- tests/unittests/qml/tst_TabsBar.qml	2016-11-18 15:10:47 +0000
> +++ tests/unittests/qml/tst_TabsBar.qml	2017-02-14 10:13:19 +0000
> @@ -19,6 +19,8 @@
>  import QtQuick 2.4
>  import QtTest 1.0
>  import "../../../src/app/webbrowser"
> +import Ubuntu.Components 1.3
> +import Ubuntu.Components.Popups 1.3

Those two imports don't appear to be needed.

>  import webbrowserapp.private 0.1
>  
>  Item {


-- 
https://code.launchpad.net/~ahayzen/webbrowser-app/migrate-to-tabs-component/+merge/312340
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app/staging.