[Merge] lp:~fboucault/webbrowser-app/background_open_tabs_adjacent into lp:webbrowser-app

Olivier Tilloy olivier.tilloy at canonical.com
Thu Feb 2 12:11:49 UTC 2017


Review: Needs Fixing

Can you please re-target the merge request to lp:webbrowser-app/staging?

The "Open link in new tab" and "Open link in new background tab" context menu entries should also open the new tab next to the caller (and there should be autopilot tests for those use cases).

Also, see one minor comment inline.

Diff comments:

> 
> === modified file 'tests/unittests/tabs-model/tst_TabsModelTests.cpp'
> --- tests/unittests/tabs-model/tst_TabsModelTests.cpp	2016-04-04 10:17:33 +0000
> +++ tests/unittests/tabs-model/tst_TabsModelTests.cpp	2017-02-01 16:41:48 +0000
> @@ -486,6 +486,18 @@
>          QCOMPARE(model->get(3), (QObject*) nullptr);
>      }
>  
> +    void shouldReturnTabIndex()
> +    {
> +        QQuickItem* tab1 = createTab();
> +        model->add(tab1);
> +        QQuickItem* tab2 = createTab();
> +        model->add(tab2);
> +        QQuickItem* nonAddedTab = createTab();
> +        QCOMPARE(model->indexOf(tab1), 0);
> +        QCOMPARE(model->indexOf(tab2), 1);
> +        QCOMPARE(model->indexOf(nonAddedTab), -1);

Please delete nonAddedTab to avoid leaking it.

> +    }
> +
>  private:
>      void moveTabs(int from, int to, bool moved, bool indexChanged, int newIndex)
>      {


-- 
https://code.launchpad.net/~fboucault/webbrowser-app/background_open_tabs_adjacent/+merge/316138
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list