1) ubuntu-ui-extras is being added to the platform snap (see https://code.launchpad.net/~osomon/ubuntu-app-platform/+git/ubuntu-app-platform/+merge/316684), so you can remove the extra logic from the packaging.

Removed those changes

2) Can you please update the copyright year of all changed files to 2017 ?

TODO

3) file://…/src/app/webbrowser/TabsBar.qml:122: TypeError: Property 'removeMovingTab' of object TabsBar_QMLTYPE_198(0x2f79320) is not a function

FIXED

4) When I right-click on a tab and choose "New Tab", the new tab is not getting focus. This doesn't appear to be a regression, but would you mind fixing this while you're at it?

FIXED

5) What is this module needed for? I’m not seeing any explicit import of QtQml.Models in the diff.

Removed, and adding to ui-extras https://code.launchpad.net/~ahayzen/ubuntu-ui-extras/add-missing-model-depends/+merge/317090

6) Please revert changes to the pot file.

REVERTED

7) ubuntu-ui-extras is being added to the platform snap (see https://code.launchpad.net/~osomon/ubuntu-app-platform/+git/ubuntu-app-platform/+merge/316684), so those changes should be reverted.

Removed those changes

8) Please remove the extra blank line.

Removed

9) Please make it a readonly property.

Made it readonly

10) Can you please remove the extra blank line?

Removed

11) Cosmetics: can this import be grouped together with the other Ubuntu.Components imports?

Moved

12) Does faviconFactory really need to be a property? Can't it simply be a nested Component?

Yup, changed to a nested Component

13) I wonder how this actually works. The incubator may instantiate the component asynchronously, so by the time the function returns its status might not be Component.Ready, and the icon source will be empty.
Also, why make the FaviconFetcher a child of tabsBar.parent, and not of tabsBar itself?

Change the parent to be the tabsBar.
This seems to work like a binding, although it should be tested on low powered machines to ensure it is actually working and not always working as the incubator happens to load in time.

14) Cosmetics: can you please add whitespaces around the equal sign?

Added

15) So this is like the changes in https://code.launchpad.net/~ahayzen/webbrowser-app/fix-1630211-drag-tabs-quickly-incorrect-position/+merge/307709 (sorry for not letting that one aside for so long), except that the indexes in the first beginMoveRows(…) are different. Why is that so?

I found that the old branch was actually wrong, and in the case where you move in a positive direction it would only allow you to move one position. Now with the changed logic it works correctly.

Also see the Qt documentation http://doc.qt.io/qt-5/qabstractitemmodel.html#beginMoveRows
It should be beginMoveRows(const QModelIndex &sourceParent, int sourceFirst, int sourceLast, const QModelIndex &destinationParent, int destinationChild)

I now have beginMoveRows(QmodelIndex(), i, i, QModelIndex(), i + 2), where i is the sourceFirst and sourceLast as it is the only index moving. And i + 2 is the destinationChild as the row is moving past the index after the current.

16) Is this really needed? There's already a 'tabsModel' property (line below).

This is required as the ui-extras windowFactory expects window.model to be the model of the tabs.

17) Can this debug statement be removed?

Removed

18) Why do you need to override the onContextMenu handler and define a new contextualOptionsComponent? Wouldn't this work with the stock one provided by the TabsBar component?

Yup, this isn't needed anymore

19) Cosmetics: can you please add whitespaces around the equal sign, and remove the one before the ++ operator?

Fixed


Please let me know if I have missed anything, or any of the above are not clear.